Skip to content

Commit

Permalink
fix: Prevent error page recursion
Browse files Browse the repository at this point in the history
We sometimes see rendering errors in the error page itself, which then
cause another attempt at rendering the error page. By catching all errors
raised during error-page render and substituting in a hardcoded string, we
can reduce server resources, avoid logging massive sequences of recursive
stack traces, and still give the user *some* indication that yes, there was
a problem.

This should help address #35151

At least one of these rendering errors is known to be due to a translation
error. There's a separate issue for restoring translation quality so that
we avoid those issues in the future (openedx/openedx-translations#549)
but in general we should catch all rendering errors, including unknown
ones.

Testing:

- In `lms/envs/devstack.py` change `DEBUG` to `False` to ensure that the
  usual error page is displayed (rather than the debug error page).
- Add line `1/0` to the top of the `student_dashboard` function in
 `common/djangoapps/student/views/dashboard.py` to make that view error.
- In `lms/templates/static_templates/server-error.html` replace
  `static.get_platform_name()` with `None * 7` to make the error template
  itself produce an error.
- Visit <http://localhost:18000/dashboard>.

Without the fix, the response takes 10 seconds and produces a 6 MB, 85k
line set of stack traces and the page displays "A server error occurred.
Please contact the administrator."

Without the fix, the response takes less than a second and produces three
stack traces (one of which contains the error page's rendering error).
  • Loading branch information
timmc-edx committed Jul 31, 2024
1 parent 46c972e commit 035677c
Showing 1 changed file with 18 additions and 2 deletions.
20 changes: 18 additions & 2 deletions lms/djangoapps/static_template_view/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
# List of valid templates is explicitly managed for (short-term)
# security reasons.


import logging
import mimetypes

from django.conf import settings
Expand All @@ -23,6 +23,8 @@
from common.djangoapps.util.views import fix_crum_request
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers

log = logging.getLogger(__name__)

valid_templates = []

if settings.STATIC_GRAB:
Expand Down Expand Up @@ -122,4 +124,18 @@ def render_429(request, exception=None): # lint-amnesty, pylint: disable=unused

@fix_crum_request
def render_500(request):
return HttpResponseServerError(render_to_string('static_templates/server-error.html', {}, request=request))
try:
return HttpResponseServerError(render_to_string('static_templates/server-error.html', {}, request=request))
except BaseException as e:
# If we can't render the error page, ensure we don't raise another
# exception -- because if we do, we'll probably just end up back
# at the same rendering error.
#
# This is an attempt at working around the recursive error handling issues
# observed in <https://github.com/openedx/edx-platform/issues/35151>, which
# were triggered by Mako and translation errors.

log.error("Encountered error while rendering error page.", exc_info=True)
# This message is intentionally hardcoded and does not involve
# any translation, templating, etc. Do not translate.
return HttpResponseServerError("Encountered error while rendering error page.")

0 comments on commit 035677c

Please sign in to comment.