Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not handle RoutingExceptions with app error handlers #2986

Merged

Conversation

@taion
Copy link
Contributor

commented Nov 1, 2018

Fixes #2984

@davidism

This comment has been minimized.

Copy link
Member

commented Nov 1, 2018

Yeah, this makes sense now.

@taion

This comment was marked as off-topic.

Copy link
Contributor Author

commented Jan 3, 2019

Any thoughts here?

@davidism

This comment has been minimized.

Copy link
Member

commented Jan 7, 2019

I did a little more research just to make sure I understood what changed from 0.12 to 1.0, in case we need to address this again later.

Prior to 1.0, error handlers for HTTPException classes were only looked up by code. So if there wasn't a specific handler registered, it wouldn't fall back to a more general handler, which people found confusing. But it was possible to add a handler specifically for the 301 code and have different redirect behavior. Why you would do that, I don't know, but it was possible. And since RequestRedirect wasn't part of the werkzeug.exceptions.default_exceptions map, a handler for 301 wouldn't have matched it anyway.

I'm OK with saying that anyone who really wants to catch RequestRedirect should override handle_http_exception or url_map to do something different. Or if they want to redirect using exceptions, they can create their own class Found(HTTPException): code = 302 and add a handler for that.

@davidism davidism added this to the 1.0.3 milestone Jan 7, 2019

taion and others added some commits Nov 1, 2018

@davidism davidism changed the base branch from master to 1.0-maintenance Jan 7, 2019

@davidism davidism force-pushed the taion:do-not-handle-routing-exception branch from f9f6e78 to 662ce21 Jan 7, 2019

@davidism davidism changed the title [RFC] Do not handle RoutingExceptions with app error handlers Do not handle RoutingExceptions with app error handlers Jan 7, 2019

@davidism davidism merged commit ded7cbe into pallets:1.0-maintenance Jan 7, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@taion taion deleted the taion:do-not-handle-routing-exception branch Jan 7, 2019

@taion

This comment has been minimized.

Copy link
Contributor Author

commented Jan 7, 2019

Or if they want to redirect using exceptions, they can create their own class Found(HTTPException): code = 302 and add a handler for that.

One small comment on the above – if someone does want to redirect using exceptions in a view, they wouldn't use RequestRedirect anyway. It's documented as a thing to use inside Werkzeug routing.

Because RoutingException is not actually part of werkzeug.exceptions.default_exceptions, installing an exception handler for 302 (well, now it's 308) in 0.12 would not have caught RoutingExceptions anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.