Fix error e-mails sent to admins in response to expected HTTP 500 codes.

Review Request #12417 — Created June 27, 2022 and submitted

Information

Review Board
release-5.0.x

Reviewers

We hit a regression when deploying Review Board 5 to production. API
responses that returned a HTTP 500 (which we use to indicate an issue
usually with a service Review Board is talking to) were resulting in
e-mails sent to the administrations indicating a problem. These were
false-positives.

The problem has to do with a behavioral change in Django, which is less
than ideal. In older versions, uncaught exceptions would result in an
error log message, which in turn triggered a handler to send an e-mail.
If a HttpResponse instance was instead returned, it would be returned
as-is.

With the modern behavior, any HttpResponse with a HTTP 500+ would
trigger this e-mail behavior.

Fortunately, as part of the logging message, there's an exception
traceback in the uncaught exception case, but not in the standard
response case. We can use this to disable this behavior.

This requires setting an explicit LOGGING dictionary in settings.py.
The settings are documented as being based on Django 3.2's settings,
with our specific changes listed.

In all tests conducted, this brings us back to the behavior we've been
used to with prior versions of Django.

Ran without DEBUG.

Accessed APIs (e.g., /api/repository/1/info/) that returned HTTP 500+
responses. Didn't see a logged e-mail, but did see the logged HTTP line,
as expected.

Triggered an exception in a view. Saw a logged e-mail, as expected.

Summary ID
Fix error e-mails sent to admins in response to expected HTTP 500 codes.
We hit a regression when deploying Review Board 5 to production. API responses that returned a HTTP 500 (which we use to indicate an issue usually with a service Review Board is talking to) were resulting in e-mails sent to the administrations indicating a problem. These were false-positives. The problem has to do with a behavioral change in Django, which is less than ideal. In older versions, uncaught exceptions would result in an error log message, which in turn triggered a handler to send an e-mail. If a `HttpResponse` instance was instead returned, it would be returned as-is. With the modern behavior, any `HttpResponse` with a HTTP 500+ would trigger this e-mail behavior. Fortunately, as part of the logging message, there's an exception traceback in the uncaught exception case, but not in the standard response case. We can use this to disable this behavior. This requires setting an explicit `LOGGING` dictionary in `settings.py`. The settings are documented as being based on Django 3.2's settings, with our specific changes listed. In all tests conducted, this brings us back to the behavior we've been used to with prior versions of Django.
01aebbf467c8b7b6c3e875844b2534714db53b89
Description From Last Updated

require_exceptions -> require_exception

daviddavid
david
  1. 
      
  2. reviewboard/settings.py (Diff revision 1)
     
     
    Show all issues

    require_exceptions -> require_exception

  3. 
      
maubin
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-5.0.x (472c21e)
Loading...