500 ISE errors when dealing with patches that use tabs to change the indentation of existing lines

RESOLVED FIXED

Status

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: fauweh, Assigned: glob)

Tracking

Details

Attachments

(1 attachment)

Reporter

Description

3 years ago
Reported by :jaws in #moc.

<jaws> hey #moc, i'm getting HTTP 500 errors on mozreview when trying to publish review feedback
<jaws> i just get a message box with "HTTP 500 INTERNAL SERVER ERROR"
<jaws> yeah i get a 500 when i click the Publish button on this review, https://reviewboard.mozilla.org/r/90168/diff/1#index_header

On reviewboard2.webapp.scl3.mozilla.com:/var/log/reviewboard/reviewboard.log I found the following, followed by some python errors:

4523:2016-11-04 22:55:54,731 - INFO -  - Publishing review for user: jaws review id: 90536 review request id: 90168
4539:2016-11-04 22:55:55,253 - ERROR -  - Exception thrown for user jaws at https://reviewboard.mozilla.org/api/review-requests/90168/reviews/90536/
4599:2016-11-04 22:56:26,912 - INFO -  - Publishing review for user: jaws review id: 90536 review request id: 90168
4615:2016-11-04 22:56:27,167 - ERROR -  - Exception thrown for user jaws at https://reviewboard.mozilla.org/api/review-requests/90168/reviews/90536/
4686:2016-11-04 22:59:21,931 - INFO -  - Publishing review for user: jaws review id: 90536 review request id: 90168
4702:2016-11-04 22:59:22,054 - ERROR -  - Exception thrown for user jaws at https://reviewboard.mozilla.org/api/review-requests/90168/reviews/90536/
4757:2016-11-04 23:08:17,597 - INFO -  - Publishing review for user: jaws review id: 90536 review request id: 90168
4773:2016-11-04 23:08:17,785 - ERROR -  - Exception thrown for user jaws at https://reviewboard.mozilla.org/api/review-requests/90168/reviews/90536/

2016-11-04 22:56:27,162 - ERROR -  - Error when calling <function on_review_publishing at 0x7f44dedf3aa0> from SignalHook: pop from empty list
Reporter

Updated

3 years ago
Summary: 500 ISE errors when posting review → 500 ISE errors when posting review to reviewboard.m.o
I'm still getting this error today on the same review. It won't let me publish it. Any work-arounds?
Flags: needinfo?(kferrando)
Group: infra → mozilla-employee-confidential
Product: Developer Services → MozReview
Two tracebacks for the prices of one:

Traceback (most recent call last):
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/djblets/extensions/hooks.py", line 196, in _wrap_callback
    self.callback(extension=self.extension, **kwargs)
  File "/data/www/reviewboard.mozilla.org/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 "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/mozreview-0.1.2a0-py2.6.egg/mozreview/signal_handlers.py", line 652, in on_review_publishing
    {"user": user})
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/mozreview-0.1.2a0-py2.6.egg/mozreview/diffs.py", line 337, in build_plaintext_review
    review.is_reply()))
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/mozreview-0.1.2a0-py2.6.egg/mozreview/diffs.py", line 310, in render_comment_plain
    lines.extend(render_equal_chunk(chunk, parser))
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/mozreview-0.1.2a0-py2.6.egg/mozreview/diffs.py", line 191, in render_equal_chunk
    replace_chars.pop()
IndexError: pop from empty list

... and ...

Traceback (most recent call last):
  File "/data/www/reviewboard.mozilla.org/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 "/usr/lib64/python2.6/site-packages/newrelic-2.44.0.36/newrelic/hooks/framework_django.py", line 497, in wrapper
    return wrapped(*args, **kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/django/views/decorators/cache.py", line 52, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/django/views/decorators/vary.py", line 19, in inner_func
    response = func(*args, **kwargs)
  File "/data/www/reviewboard.mozilla.org/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 "/data/www/reviewboard.mozilla.org/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 "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/djblets/webapi/resources/base.py", line 464, in put
    return self.update(request, *args, **kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/djblets/webapi/decorators.py", line 122, in _call
    return view_func(*args, **kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/ReviewBoard-2.5.4.moz-py2.6.egg/reviewboard/webapi/decorators.py", line 139, in _check
    return view_func(*args, **kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/djblets/webapi/decorators.py", line 122, in _call
    return view_func(*args, **kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/djblets/webapi/decorators.py", line 143, in _checklogin
    return view_func(*args, **kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/djblets/webapi/decorators.py", line 122, in _call
    return view_func(*args, **kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/djblets/webapi/decorators.py", line 122, in _call
    return view_func(*args, **kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/djblets/webapi/decorators.py", line 307, in _validate
    return view_func(*args, **new_kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/ReviewBoard-2.5.4.moz-py2.6.egg/reviewboard/webapi/resources/base_review.py", line 256, in update
    return self._update_review(request, review, *args, **kwargs)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/ReviewBoard-2.5.4.moz-py2.6.egg/reviewboard/webapi/resources/base_review.py", line 303, in _update_review
    review.publish(user=request.user)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/ReviewBoard-2.5.4.moz-py2.6.egg/reviewboard/reviews/models/review.py", line 222, in publish
    review=self)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/django/dispatch/dispatcher.py", line 185, in send
    response = receiver(signal=self, sender=sender, **named)
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/djblets/extensions/hooks.py", line 196, in _wrap_callback
    self.callback(extension=self.extension, **kwargs)
  File "/data/www/reviewboard.mozilla.org/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 "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/mozreview-0.1.2a0-py2.6.egg/mozreview/signal_handlers.py", line 652, in on_review_publishing
    {"user": user})
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/mozreview-0.1.2a0-py2.6.egg/mozreview/diffs.py", line 337, in build_plaintext_review
    review.is_reply()))
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/mozreview-0.1.2a0-py2.6.egg/mozreview/diffs.py", line 310, in render_comment_plain
    lines.extend(render_equal_chunk(chunk, parser))
  File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/mozreview-0.1.2a0-py2.6.egg/mozreview/diffs.py", line 191, in render_equal_chunk
    replace_chars.pop()
IndexError: pop from empty list
Flags: needinfo?(kferrando)
Assignee

Comment 3

3 years ago
looks like something about that patch is breaking the handling of the indentation.
that patch is unusual because it contains tabs and some lines are CRLF delimited.
Assignee

Comment 4

3 years ago
i can reproduce this issue in my dev environment; working on it.
Assignee: nobody → glob
Group: mozilla-employee-confidential
Summary: 500 ISE errors when posting review to reviewboard.m.o → 500 ISE errors when dealing with patches that use tabs to change the indentation of existing lines
Comment hidden (mozreview-request)

Comment 6

3 years ago
mozreview-review
Comment on attachment 8808483 [details]
mozreview: fix handling of tab-indented lines (bug 1315395);

https://reviewboard.mozilla.org/r/91316/#review91590

Thanks for fixing this up, sooo much nicer now.

::: pylib/mozreview/mozreview/diffs.py:188
(Diff revision 1)
> -            elif chars[index] == "&" and chars[index + 1] == "m":  # "&mdash;".
> -                # One of the spaces we translated before
> -                # was actually part of this tab we've
> +                # Skip &gt; prefix.
> +                if chars[index:index + 4] == "&gt;":
> +                    index += 4

I had thought these characters were actually part of the tabs length, should we not be incrementing the `current_width`?

::: pylib/mozreview/mozreview/diffs.py:197
(Diff revision 1)
> +                # Skip &gt; suffix.
> +                if chars[index:index + 4] == "&gt;":
> +                    index += 4

again, should we not increment the `current_width`?

::: pylib/mozreview/mozreview/diffs.py:216
(Diff revision 1)
> +                if index > 0:
> -                replace_chars.pop()
> +                    replace_chars.pop()

Is there really a case where this triggers? or are you just being defensive?
Attachment #8808483 - Flags: review?(smacleod) → review+
Assignee

Comment 7

3 years ago
mozreview-review-reply
Comment on attachment 8808483 [details]
mozreview: fix handling of tab-indented lines (bug 1315395);

https://reviewboard.mozilla.org/r/91316/#review91590

> Is there really a case where this triggers? or are you just being defensive?

defensive - should the rb formatting change again i'd prefer to output the wrong number of spaces in bugzilla than crash.
Comment hidden (mozreview-request)

Comment 9

3 years ago
Pushed by bjones@mozilla.com:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/ba836bfdca9a
mozreview: fix handling of tab-indented lines ; r=smacleod
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.