Skip to content

Commit

Permalink
Flask apps only signal an exception on real server errors (#1611)
Browse files Browse the repository at this point in the history
Fixes the problem of non-exceptional "exceptions" being recorded by
telemetry systems as a serious error. This expands on the changes made
in #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 committed Dec 7, 2022
1 parent 670bee9 commit a829536
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 @@ -63,7 +63,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,
Expand All @@ -85,6 +84,9 @@ def common_error_handler(self, exception):
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 a829536

Please sign in to comment.