Opened 11 years ago
Closed 11 years ago
#22058 closed New feature (wontfix)
Consider a more general method to replace handlerXXX
Reported by: | Debanshu Kundu | Owned by: | ANUBHAV JOSHI |
---|---|---|---|
Component: | Core (URLs) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | anubhav9042@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
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.
Change History (13)
comment:1 by , 11 years ago
comment:2 by , 11 years ago
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??
comment:3 by , 11 years ago
Cc: | added |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:4 by , 11 years ago
Summary: | Add `Http405` exception class and `handler405` view (simillar to 404, 403 and 500) → Consider a more general method to replace handlerXXX |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:5 by , 11 years ago
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".
comment:6 by , 11 years ago
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, for example, allows you to decorate functions with the error code which seems nice:
@app.errorhandler(404) def page_not_found(e): ...
comment:7 by , 11 years ago
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 adefaultdict
to provide a fallback for unknown errorshandlers4xx
/handlers5xx
dicts, which allows providing twodefaultdict
s with different defaults, for the same reason as above- etc.
comment:8 by , 11 years ago
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
As far as the functionality is concerned, I think it works fine, but there might some design decisions involved for which some things might have to be changed.
comment:10 by , 11 years ago
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.
comment:12 by , 11 years ago
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 View.http_method_not_allowed() method if you want to customize the behavior of class-based views.
comment:13 by , 11 years ago
Resolution: | → wontfix |
---|---|
Status: | assigned → closed |
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 :/