aws / aws-xray-sdk-python

AWS X-Ray SDK for the Python programming language
Apache License 2.0
327 stars 143 forks source link

Flask middleware errors when an earlier Flask extension throws an exception in a before_request method #405

Open SamStephens opened 12 months ago

SamStephens commented 12 months ago

If the Flask middleware is configured after another extension, and that extension throws an exception in a before_request method, the Flask middleware throws an exception in its after_request method.

  File "/home/sam/.pyenv/versions/3.9.7/envs/flask-test-2-3.9.7/lib/python3.9/site-packages/flask/app.py", line 1508, in finalize_request
    response = self.process_response(response)
  File "/home/sam/.pyenv/versions/3.9.7/envs/flask-test-2-3.9.7/lib/python3.9/site-packages/flask/app.py", line 2002, in process_response
    response = self.ensure_sync(func)(response)
  File "/home/sam/.pyenv/versions/3.9.7/envs/flask-test-2-3.9.7/lib/python3.9/site-packages/aws_xray_sdk/ext/flask/middleware.py", line 74, in _after_request
    segment.put_http_meta(http.STATUS, response.status_code)
AttributeError: 'NoneType' object has no attribute 'put_http_meta'
cannot find the current segment/subsegment, please make sure you have a segment open

This does not happen if the Flask middleware is configured before other extensions.

The root cause appears to be that when an extension throws an exception in a before_request method, processing of subsequent before request methods is suppressed, but after_request methods are still executed.

I think there's two things needed to do to resolve this:

Note that even if the after_request handler doesn't raise an exception, having other middleware prevent the Flask middleware's before_request method from executing is a problem, because it means that you will not get X-Ray traces when this happens. I suggest that the after_request handler should log a warning or error explaining the problem if the current segment or subsegment is None.

I've attached a reproduction. This relies on the fact that Flask-WTF throws an exception to indicate a bad request when a CSRF token is missing from a request that requires one. To reproduce the bug:

You can see that ordering matters by swapping the following two lines in app.py:

csrf = CSRFProtect(app)
XRayMiddleware(app, xray_recorder)

If the XRayMiddleware is initialised first, the request will succeed, and you'll see the Bad Request error that you should (because the request does not have a CSRF token).

srprash commented 10 months ago

Thanks for raising this issue and posting the workaround too.

I think there's two things needed to do to resolve this:

-Make it clear in https://docs.aws.amazon.com/xray-sdk-for-python/latest/reference/frameworks.html#flask that Flask should be the first extension configured. -In the after_request handler, handle the current segment or subsegment being None without raising an exception.

For the second point, a better approach will be to just log the exception instead of raising it by default - like we did in X-Ray Java SDK: https://github.com/aws/aws-xray-sdk-java/pull/353