[Django] #22058: Add `Http405` exception class and `handler405` view (simillar to 404, 403 and 500)
Django
and 500)
-----------------------------+--------------------
Reporter: debanshuk | Owner: nobody
Type: New feature | Status: new
Component: Core (URLs) | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+--------------------
With class based views it's not straight forward to to raise a `Http404`
error if the view doesn't support HTTP method (i.e. a 405 error), with
function based views it was. So, in such cases clients (users) see a blank
page with 405 response code, but with no message to tell them where to go
from there. I guess Django should raise and handle `Http405` exception and
use a default `handler405` view (which of course can be overridden) to
return response to user in such a case, as it does in case of 404, 403 and
500 errors.
--
Ticket URL: <https://code.djangoproject.com/ticket/22058>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
and 500)
Reporter: debanshuk | Owner: nobody
Type: New feature | Status: new
Component: Core (URLs) | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
405 errors are an order of magnitude less common than 404 and 403.
I don't think it's a good idea to keep adding exception classes and
hardcoding handlerXXX in URLconfs. We should start thinking about a more
generic mechanism.
For instance we could have a view to deal with client errors (4XX), a view
for server errors (5XX), and pass adequate arguments to these views. But
that would create lots of code churn in existing projects :/
--
Ticket URL: <https://code.djangoproject.com/ticket/22058#comment:1>
Django
and 500)
Reporter: debanshuk | Owner: nobody
Type: New feature | Status: new
Component: Core (URLs) | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by anubhav9042):
I am interested in this especially I want this to incorporate in GSoC
proposal.
As suggested we can just have two views which can be matched with regex
4XX and 5XX, and accordingly lot of code will be reduced.
Also as this ticket suggests, we can add some more cases including 405
like 401, etc
Any suggestions??
--
Ticket URL: <https://code.djangoproject.com/ticket/22058#comment:2>
Django
and 500)
Reporter: debanshuk | Owner: anubhav9042
Type: New feature | Status: assigned
Component: Core (URLs) | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by anubhav9042):
* status: new => assigned
* owner: nobody => anubhav9042
* cc: anubhav9042@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/22058#comment:3>
Django
-----------------------------+---------------------------------------
Reporter: debanshuk | Owner: anubhav9042
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/22058#comment:4>
Django
-----------------------------+---------------------------------------
Reporter: debanshuk | Owner: anubhav9042
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by anubhav9042):
I think the best solution is to refactor the code creating only two
handlers `handler4XX` & `handler5XX` and two views. And in order to
prevent any backwards incompatibility, we can let the existing exceptions
as it is and create two generic ones. This way in future people will be
able to deal with any kind of errors along with the previous ones provided
for 403,404,500. I mean this way whatever Aymeric suggests can be done
without any "code churn in existing projects".
--
Ticket URL: <https://code.djangoproject.com/ticket/22058#comment:5>
Django
-----------------------------+---------------------------------------
Reporter: debanshuk | Owner: anubhav9042
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timo):
I'm not sure about that suggestion. Having two handlers to catch all
errors could encourage gigantic functions. It could be useful to look into
how other frameworks deal with this. Flask,
[http://flask.pocoo.org/docs/patterns/errorpages/#error-handlers for
example], allows you to decorate functions with the error code which seems
nice:
{{{
@app.errorhandler(404)
def page_not_found(e):
...
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22058#comment:6>
Django
-----------------------------+---------------------------------------
Reporter: debanshuk | Owner: anubhav9042
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
If memory serves, that's also how Flask's url dispatcher works in general.
One decorates view functions to declare the URL they're available at. So
this mechanism is consistent with Flask's design but not with Django's.
For better or worse, Django's contract is to provide view information
under conventional names in the ROOT_URLCONF module: `urlpatterns`,
`handler403/404/500`. I think we should keep this declaration mechanism
because there's no strong reason to change it.
The open question is — what conventional name(s) do we add to ROOT_URLCONF
and with what semantics?
Ideas:
- `handler` view function for all errors.
- `handler4xx` / `handler5xx` view functions — it's a smaller change and
it decreases the chances of having a dysfunctional 5xx view. That's why I
prefer to deal with 4xx and 5xx separately.
- `handlers` dict mapping error codes to view functions, possibly a
`defaultdict` to provide a fallback for unknown errors
- `handlers4xx` / `handlers5xx` dicts, which allows providing two
`defaultdict`s with different defaults, for the same reason as above
- etc.
--
Ticket URL: <https://code.djangoproject.com/ticket/22058#comment:7>
Django
-----------------------------+---------------------------------------
Reporter: debanshuk | Owner: anubhav9042
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by anubhav9042):
I have done some work on `handler4xx` and `handler5xx` and I think it
works just fine.
Have a look: https://github.com/coder9042/django/compare/gsoc_%2322058
--
Ticket URL: <https://code.djangoproject.com/ticket/22058#comment:8>
Django
-----------------------------+---------------------------------------
Reporter: debanshuk | Owner: anubhav9042
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by anubhav9042):
I have updated the above work with fix for #21668 as commented there.
--
Ticket URL: <https://code.djangoproject.com/ticket/22058#comment:9>
Django
-----------------------------+---------------------------------------
Reporter: debanshuk | Owner: anubhav9042
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timo):
Consolidating the views looks good (perhaps name them something like
`4xx_error` and `5xx_error`). Adding `'handler4xx'` and `'handler5xx'`
with the ability to define a specific version like `'handler404'` also
looks good. Could you try to put together a minimal patch with just these
concepts and not all the other refactorings like in the resolver and the
new exception classes (we'll do those separately as it's best to separate
new features from internal cleanups)?
Also, we need to keep the old `django.views'django.views.defaults.*` views
and deprecate them as they are documented.
--
Ticket URL: <https://code.djangoproject.com/ticket/22058#comment:10>
Django
-----------------------------+---------------------------------------
Reporter: debanshuk | Owner: anubhav9042
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by anubhav9042):
* has_patch: 0 => 1
Comment:
https://github.com/django/django/pull/2828
--
Ticket URL: <https://code.djangoproject.com/ticket/22058#comment:11>
Django
-----------------------------+---------------------------------------
Reporter: debanshuk | Owner: anubhav9042
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timo):
From IRC: "apollo13 doesn't see why we'd need a generic handler for every
http status code out there."
Absent some additional arguments, I guess I'd agree. I'd still like to do
some of the code cleanup in resolvers that the existing PR implements, but
I'm okay if we "won't fix" this ticket in general.
In the case of the problem described by the ticket, you can override the
[https://docs.djangoproject.com/en/1.7/ref/class-based-
views/base/#django.views.generic.base.View.http_method_not_allowed
View.http_method_not_allowed()] method if you want to customize the
behavior of class-based views.
--
Ticket URL: <https://code.djangoproject.com/ticket/22058#comment:12>
Django
-----------------------------+---------------------------------------
Reporter: debanshuk | Owner: anubhav9042
Component: Core (URLs) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* status: assigned => closed
* resolution: => wontfix
--
Ticket URL: <https://code.djangoproject.com/ticket/22058#comment:13>