500 error when publishing a draft review that is not the parent

RESOLVED INVALID

Status

RESOLVED INVALID
3 years ago
2 months ago

People

(Reporter: mars, Unassigned)

Tracking

Production

Details

(Reporter)

Description

3 years ago
Publishing a review that is not the summary review using the "Finish Review" button raises a HTTP 500.

I caused this error when trying to submit my first review request.  I didn't know that I had to visit the parent review page, so when looking at the child review page without an obvious "Publish" button, I clicked the button that looked like it would let me "finish" my draft: the "Finish Review" button.

Triggered in production, reproducible on dev.

STR:

1. create a draft review
2. log in to reviewboard as the owner of the new draft
3. visit the unpublished draft review's child review (not the summary/parent)
4. click "Finish Review"
5. click "Publish Review"
6. HTTP 500


Logs from dev:

2016-06-09 18:53:30,309 - INFO -  - Publishing review for user: default review id: 1 review request id: 3
2016-06-09 18:53:30,314 - ERROR -  - Error when calling <function on_review_publishing at 0x7f3492f65a28> from SignalHook: list in
dex out of range
Traceback (most recent call last):
  File "/venv/lib/python2.6/site-packages/djblets/extensions/hooks.py", line 196, in _wrap_callback
    self.callback(extension=self.extension, **kwargs)
  File "/venv/lib/python2.6/site-packages/mozreview-0.1.2a0-py2.6.egg/mozreview/bugzilla/errors.py", line 28, in _transform_errors
    return func(*args, **kwargs)
  File "/venv/lib/python2.6/site-packages/rbbz-0.2.9-py2.6.egg/rbbz/extension.py", line 115, in on_review_publishing
    bug_id = int(review_request.get_bug_list()[0])
IndexError: list index out of range
2016-06-09 18:53:30,316 - ERROR -  - Exception thrown for user default at http://192.168.33.33:55556/api/review-requests/3/reviews
/

list index out of range


Traceback (most recent call last):
  File "/venv/lib/python2.6/site-packages/django/core/handlers/base.py", line 112, in get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/venv/lib/python2.6/site-packages/django/views/decorators/cache.py", line 52, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "/venv/lib/python2.6/site-packages/django/views/decorators/vary.py", line 19, in inner_func
    response = func(*args, **kwargs)
  File "/venv/lib/python2.6/site-packages/djblets/webapi/resources/base.py", line 196, in __call__
    request, method, view, api_format=api_format, *args, **kwargs)
  File "/venv/lib/python2.6/site-packages/djblets/webapi/resources/mixins/api_tokens.py", line 65, in call_method_view
    return view(request, *args, **kwargs)
  File "/venv/lib/python2.6/site-packages/djblets/webapi/resources/base.py", line 449, in post
    return self.create(*args, **kwargs)
  File "/venv/lib/python2.6/site-packages/djblets/webapi/decorators.py", line 122, in _call
    return view_func(*args, **kwargs)
  File "/venv/lib/python2.6/site-packages/reviewboard/webapi/decorators.py", line 139, in _check
    return view_func(*args, **kwargs)
  File "/venv/lib/python2.6/site-packages/djblets/webapi/decorators.py", line 122, in _call
    return view_func(*args, **kwargs)
  File "/venv/lib/python2.6/site-packages/djblets/webapi/decorators.py", line 143, in _checklogin
    return view_func(*args, **kwargs)
  File "/venv/lib/python2.6/site-packages/djblets/webapi/decorators.py", line 122, in _call
    return view_func(*args, **kwargs)
  File "/venv/lib/python2.6/site-packages/djblets/webapi/decorators.py", line 122, in _call
    return view_func(*args, **kwargs)
  File "/venv/lib/python2.6/site-packages/djblets/webapi/decorators.py", line 307, in _validate
    return view_func(*args, **new_kwargs)
  File "/venv/lib/python2.6/site-packages/reviewboard/webapi/resources/base_review.py", line 224, in create
    result = self._update_review(request, review, *args, **kwargs)
  File "/venv/lib/python2.6/site-packages/reviewboard/webapi/resources/base_review.py", line 303, in _update_review
    review.publish(user=request.user)
  File "/venv/lib/python2.6/site-packages/reviewboard/reviews/models/review.py", line 222, in publish
    review=self)
  File "/venv/lib/python2.6/site-packages/django/dispatch/dispatcher.py", line 185, in send
    response = receiver(signal=self, sender=sender, **named)
  File "/venv/lib/python2.6/site-packages/djblets/extensions/hooks.py", line 196, in _wrap_callback
    self.callback(extension=self.extension, **kwargs)
  File "/venv/lib/python2.6/site-packages/mozreview-0.1.2a0-py2.6.egg/mozreview/bugzilla/errors.py", line 28, in _transform_errors
    return func(*args, **kwargs)
  File "/venv/lib/python2.6/site-packages/rbbz-0.2.9-py2.6.egg/rbbz/extension.py", line 115, in on_review_publishing
    bug_id = int(review_request.get_bug_list()[0])
IndexError: list index out of range
(Reporter)

Comment 1

3 years ago
Inspecting the reviews_reviewrequest database table and related code, it appears that the object being passed into on_review_publishing() is a ReviewRequest with an empty .bugs_closed property and other incomplete data.  The empty bugs_closed property would cause the IndexError.

When I check the reviews_reviewrequestdraft table, I see ReviewRequestDraft object data with the correct review ID and correct value for .bugs_closed.

I'm guessing we aren't supposed to call "Finish Review" or "Publish Review" with an review in the draft state, because the code behind these buttons then instantiates a busted ReviewRequest object.
(In reply to Māris Fogels [:mars] from comment #1)
> I'm guessing we aren't supposed to call "Finish Review" or "Publish Review"
> with an review in the draft state, because the code behind these buttons
> then instantiates a busted ReviewRequest object.

Correct, I had thought RB prevented you from even starting a review on a draft review request... Do you think you could check if upstream has a bug that's allowing this or it's something we're causing with our extensions?

Comment 3

2 months ago
MozReview is now obsolete. Please use Phabricator instead. Closing this bug.
Status: NEW → RESOLVED
Last Resolved: 2 months ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.