[Django] #30752: Allow using ExceptionReporter subclass in django.views.debug.technical_500_response
Django
django.views.debug.technical_500_response
-------------------------------------------+------------------------
Reporter: Carlton Gibson | Owner: (none)
Type: New feature | Status: new
Component: Error reporting | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------------+------------------------
#29714 allows using an ExceptionReporter subclass with AdminEmailHandler.
Ideally we'd make the similar available for the 500 debug error view.
Currently the use of `ExceptionReporter` is hardcoded.
* Move this to a parameter
* Provide an example of using, e.g., `functools.partial` to configure a
subclass with specifying `handler500`.
--
Ticket URL: <https://code.djangoproject.com/ticket/30752>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
django.views.debug.technical_500_response
Reporter: Carlton Gibson | Owner: (none)
Type: New feature | Status: new
Component: Error reporting | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Description changed by Carlton Gibson:
Old description:
> #29714 allows using an ExceptionReporter subclass with AdminEmailHandler.
>
> Ideally we'd make the similar available for the 500 debug error view.
>
> Currently the use of `ExceptionReporter` is hardcoded.
>
> * Move this to a parameter
> * Provide an example of using, e.g., `functools.partial` to configure a
> subclass with specifying `handler500`.
New description:
#29714 allows using an ExceptionReporter subclass with AdminEmailHandler.
Ideally we'd make the similar available for the 500 debug error view.
Currently the use of `ExceptionReporter` is hardcoded.]
* Move this to a parameter
* Provide an example of using, e.g., `functools.partial` to configure a
subclass with specifying `handler500`.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/30752#comment:1>
Django
django.views.debug.technical_500_response
Reporter: Carlton Gibson | Owner: (none)
Type: New feature | Status: new
Component: Error reporting | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by felixxm):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/30752#comment:2>
Django
django.views.debug.technical_500_response
Reporter: Carlton Gibson | Owner: (none)
Type: New feature | Status: new
Component: Error reporting | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Description changed by Carlton Gibson:
Old description:
> #29714 allows using an ExceptionReporter subclass with AdminEmailHandler.
>
> Ideally we'd make the similar available for the 500 debug error view.
>
> Currently the use of `ExceptionReporter` is hardcoded.]
>
> * Move this to a parameter
> * Provide an example of using, e.g., `functools.partial` to configure a
> subclass with specifying `handler500`.
New description:
#29714 allows using an ExceptionReporter subclass with AdminEmailHandler.
Ideally we'd make the similar available for the 500 debug error view.
Currently the use of `ExceptionReporter` is hardcoded.]
* Move this to a parameter
* Provide an example of using, e.g., `functools.partial` to configure a
subclass when specifying `handler500`.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/30752#comment:3>
Django
django.views.debug.technical_500_response
Reporter: Carlton Gibson | Owner: Pavel Lysak
Type: New feature | Status: assigned
Component: Error reporting | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Pavel Lysak):
* cc: Pavel Lysak (added)
* owner: (none) => Pavel Lysak
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/30752#comment:4>
Django
django.views.debug.technical_500_response
Reporter: Carlton Gibson | Owner: Pavel Lysak
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Pavel Lysak):
Hi!
But as i see handler500 and technical_500_response are different things.
The first is view which we can set in settings and which runs when DEBUG =
False.
The second one runs when DEBUG = True. And it called from
response_for_exception and handle_uncaught_exception. So i don't think
that we can use functools.partial when providing the handler500 in the
URLConf.
Am i wrong?
--
Ticket URL: <https://code.djangoproject.com/ticket/30752#comment:5>
Django
django.views.debug.technical_500_response
Reporter: Carlton Gibson | Owner: Pavel Lysak
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Pavel Lysak):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/30752#comment:6>
Django
django.views.debug.technical_500_response
Reporter: Carlton Gibson | Owner: Pavel Lysak
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Carlton Gibson):
* needs_docs: 0 => 1
Old description:
> #29714 allows using an ExceptionReporter subclass with AdminEmailHandler.
>
> Ideally we'd make the similar available for the 500 debug error view.
>
> Currently the use of `ExceptionReporter` is hardcoded.]
>
> * Move this to a parameter
> * Provide an example of using, e.g., `functools.partial` to configure a
> subclass when specifying `handler500`.
New description:
#29714 allows using an ExceptionReporter subclass with AdminEmailHandler.
Ideally we'd make the similar available for the 500 debug error view.
[https://github.com/django/django/blob/52e9c753659ffeab67149fcf26b95c10cf137c40/django/views/debug.py#L84-L95
Currently the use of `ExceptionReporter` is hardcoded.]
~~* Move this to a parameter~~
~~* Provide an example of using, e.g., `functools.partial` to configure a
subclass when specifying `handler500`.~~
**Updated** for [https://code.djangoproject.com/ticket/30752#comment:5
comment:5]
* Add `ExceptionReporter` to the documentation, explaining the relation
between the exception reporter class and the filter it uses, and showing a
simple override of `get_traceback_data()`.
* Add a `DEFAULT_EXCEPTION_REPORTER` setting to allow specifying this.
(At that point we could review deprecating
`DEFAULT_EXCEPTION_REPORTER_FILTER`, as was discussed on #25167 — but
maybe that's unnecessary. Thoughts?)
--
Comment:
Hi Pavel, thanks for your input.
You're absolutely correct re the `handler500` suggestion. That's not
sufficient at all. (I'm not sure quite what was in my head thinking about
this.)
I've adjusted the description appropriately.
Looking at it, there's no nice way to pass down the exception reporter
class so, I agree, a setting will be the easiest way forward.
Your PR looks more or less right, but I think we need to say more in the
`Custom error reports` section. Specifically we should probably begin
stating that the reports are created by the `ExceptionReporter` class, and
that it uses an `ExceptionReporterFilter` to control the filtering. Then
then existing filtering examples are OK — that's the first port of call ‚
before giving an override example for `ExceptionReporter`...
{{{
#!python
class CustomExceptionReporter(ExceptionReporter):
def get_traceback_data(self):
data = super().get_traceback_data()
# ... remove/add something here...
return data
}}}
That should be about it. (I'm not sure there's any demand to customise
anything else on `ExceptionReporter`.)
Thanks again, super stuff.
--
Ticket URL: <https://code.djangoproject.com/ticket/30752#comment:7>
Django
django.views.debug.technical_500_response
Reporter: Carlton Gibson | Owner: Pavel Lysak
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 1 | UI/UX: 0
Comment (by Pavel Lysak):
HI, thank you for review.
I added some more to the docs. Also considered what you mentioned in PR on
github.
Also i thought about deprecating `DEFAULT_EXCEPTION_REPORTER_FILTER`. I
like that idea because it may be confusing to have these similar settings.
I think it would not be a problem to use something like this if we only
want to change filtering behavior.
{{{
#!python
class ExceptionReporter:
filter_class = SafeExceptionReporterFilter
def __init__(self, request, exc_type, exc_value, tb, is_email=False):
self.filter = self.filter_class(self.request)
...
class CustomExceptionReporter(ExceptionReporter):
filter_class = CustomExceptionReporterFilter
}}}
This will allow us to get rid of `DEFAULT_EXCEPTION_REPORTER_FILTER `
--
Ticket URL: <https://code.djangoproject.com/ticket/30752#comment:8>
Django
django.views.debug.technical_500_response
Reporter: Carlton Gibson | Owner: Pavel Lysak
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Pavel Lysak):
* needs_docs: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/30752#comment:9>
Django
django.views.debug.technical_500_response
Reporter: Carlton Gibson | Owner: Pavel
| Lysak
Type: New feature | Status: assigned
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Carlton Gibson):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/30752#comment:10>
Django
django.views.debug.technical_500_response
Reporter: Carlton Gibson | Owner: Pavel
| Lysak
Component: Error reporting | Version: master
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Carlton Gibson <carlton@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"13e4abf83e5129b44c771b2204809792087abda4" 13e4abf8]:
{{{
#!CommitTicketReference repository=""
revision="13e4abf83e5129b44c771b2204809792087abda4"
Fixed #30752 -- Allowed using ExceptionReporter subclasses in error
reports.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30752#comment:11>
Django
django.views.debug.technical_500_response
Reporter: Carlton Gibson | Owner: Pavel
| Lysak
Type: New feature | Status: closed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"56071826bbb741b6ca04aa517a26acd335fc24d7" 56071826]:
{{{
#!CommitTicketReference repository=""
revision="56071826bbb741b6ca04aa517a26acd335fc24d7"
Refs #30752 -- Doc'd error reporting related optional request attributes.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30752#comment:12>