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

Handling HTTP 405 on the blueprint level #1494

Closed
jacobsvante opened this issue Jun 11, 2015 · 7 comments
Closed

Handling HTTP 405 on the blueprint level #1494

jacobsvante opened this issue Jun 11, 2015 · 7 comments

Comments

@jacobsvante
Copy link

I really like being able to override the error templates for each blueprint using their errorhandler methods. All error codes work great for me. Except one. 405's are AFAIK not catchable by the blueprint. This is a bit unfortunate when you have a JSON REST API where you want your consumers to be able to parse the data that they get back.

This line in werkzeug.routing is where the exception is raised. I feel a bit lost about the whole flow of a request in werkzeug/flask and don't really have any idea what I could do to be able to catch it in the Blueprint. Is this a bug? A known "feature"? Or am I doing something wrong?

Here's a test case which shows this behavior:

import unittest
from flask import Flask, Blueprint, request

app = Flask(__name__)
myblueprint = Blueprint('myblueprint', __name__)


@myblueprint.route('/', methods=['GET'])
def hello():
    return 'hello world!'

myblueprint.errorhandler(405)(lambda e: ('myblueprint 405', 405))
app.register_blueprint(myblueprint)
app.errorhandler(405)(lambda e: ('app 405', 405))


class BlueprintOrAppTestCase(unittest.TestCase):

    def setUp(self):
        self.client = app.test_client()

    def test_200(self):
        resp = self.client.get('/')
        self.assertEqual(resp.status_code, 200)

    def test_405(self):
        with app.test_client() as client:
            resp = client.post('/?http405')
            self.assertEqual(resp.status_code, 405)
            self.assertEqual(resp.get_data(True), 'myblueprint 405')
            self.assertEqual(request.blueprint, 'myblueprint')


if __name__ == '__main__':
    # app.run(use_reloader=True)
    unittest.main()

Running this gives:

.F
======================================================================
FAIL: test_405 (__main__.BlueprintOrAppTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test.py", line 30, in test_405
    self.assertEqual(resp.get_data(True), 'myblueprint 405')
AssertionError: 'app 405' != 'myblueprint 405'
- app 405
+ myblueprint 405


----------------------------------------------------------------------
Ran 2 tests in 0.013s

FAILED (failures=1)

A version which tests more error codes can be found in this gist.

@untitaker
Copy link
Contributor

It certainly seems like a rough edge, similar to #1498

@jacobsvante
Copy link
Author

Kind of, but at least there we know of a workaround.

@svieira
Copy link
Contributor

svieira commented Jun 23, 2015

@jmagnusson - the way to work around this (just like in #1498) is to compose distinct Flask applications, rather than trying to make a blueprint handle 405s.

@untitaker - I think one way to help people for whom this is an issue (for 1.x or 1.0 if that ship hasn't sailed yet) would be to add a new registration method to flask.Flask that does app composition from another app or a blueprint. Something like app.mount_app(bp_or_app, url_prefix=None, subdomain=None, **config_args) Doing something like this could also help with issues like #1507 as well.

@jacobsvante
Copy link
Author

@svieira Interesting idea about using app composition. However, don't we miss out on features like linking between them with url_for?

@svieira
Copy link
Contributor

svieira commented Jun 23, 2015

You would need to add to the set of Flask.url_build_error_handlers so that the parent app (at least) could use url_for to route to the children. You could also update the children so the children could route to the parents, but that's probably not a good idea in the general case (too much magic to avoid the infinite recursion problem + the children shouldn't be aware of the parents if they really are stand-alone applications that just happen to be mounted together for convenience)

@jacobsvante
Copy link
Author

Sounds somewhat messy. I kind of like the simplicity we have today where we just add Blueprints to our app to add more HTTP endpoints.

Couldn't there be some other way to make 405 catchable? An idea I have is that we make Flask create "fake routes" for each method in the url_map. This would make it work a bit like your workaround for 404s not being catched by the blueprints.

@svieira
Copy link
Contributor

svieira commented Jun 23, 2015

You're absolutely right, you could iterate over all the defined routes in a blueprint and add a route handler for the url + (all_possible_http_methods - route_accepted_methods). However, as it currently stands Flask does not make this easy. The routes in a Blueprint are stored as a closure that will just call BlueprintSetupState.add_url_rule when the Blueprint is registered on the application ... and the add_url_rule method of BlueprintSetupState basically just forwards on its arguments to Flask.add_url_rule.

So you are left with either:

  • Reflecting the nonlocals of the lambda function of each entry in Blueprint.deferred_functions
  • Overriding add_url_rule and register for your Blueprint sub-class to store the extra meta-data and register the 405 method handlers.

@davidism davidism closed this as completed Jun 1, 2017
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants