Skip to content

Commit

Permalink
Flask apps only signal an exception on real server errors (spec-first…
Browse files Browse the repository at this point in the history
…#1611)

Fixes the problem of non-exceptional "exceptions" being recorded by
telemetry systems as a serious error. This expands on the changes made
in spec-first#1326. The intention of that other change seems to be making
telemetry systems like Sentry record serious errors. Diving into the
Sentry implementation even shows that the signalled exception is
recorded at "level" = "error".

The root of the problem is that exceptions are being used for control
flow, which is not ideal - convenient for app writers but not always for
library maintainers. The connexion BadRequestProblem and
NotFoundProblem, for example, should not be recorded as an error in a
telemetry system. In my case
[elastic-apm-python](https://github.com/elastic/apm-agent-python) is
receiving these signals and recording 4xx events as serious errors.

This pull request only propagates an exception signal to flask if it's a
serious error.

Aiohttp applications have a similar problem with exceptions being used
for control flow - the problems middleware will convert the exception
into an appropriate problem response but things like the elastic apm
python telemetry middleware will see that exception and record it as a
serious error. Interestingly aiohttp also uses exceptions for control
flow and the elastic apm agent was patched to specifically ignore aio
web exceptions below the 5xx status response range. Elastic apm and
sentry cannot be expected to be aware of non-serious control flow
exceptions used by various libraries though. So, a solution for Aiohttp
applications is a separate problem.

Co-authored-by: Enerqi <>
  • Loading branch information
enerqi authored and RobbeSneyders committed Jan 24, 2023
1 parent 7193804 commit e840840
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion connexion/apps/flask_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ def common_error_handler(self, exception):
"""
:type exception: Exception
"""
signals.got_request_exception.send(self.app, exception=exception)
if isinstance(exception, ProblemException):
response = problem(
status=exception.status, title=exception.title, detail=exception.detail,
Expand All @@ -66,6 +65,9 @@ def common_error_handler(self, exception):
status=exception.code,
headers=exception.get_headers())

if response.status_code >= 500:
signals.got_request_exception.send(self.app, exception=exception)

return FlaskApi.get_response(response)

def add_api(self, specification, **kwargs):
Expand Down

0 comments on commit e840840

Please sign in to comment.