flask: close the transaction in the exception handler (#635)

The `request_finished` signal isn't sent if an unhandled exception
occurs, and other signals (e.g. `request_teardown` don't provide the
`response` object.

fixes #629

Author: beniwohli

CHANGELOG.md

@@ -8,6 +8,10 @@
  * added instrumentation for mysql-connector and pymysql (#603)
  * implemented stack_trace_limit configuration option (#623)
  * autoinsert tracing middleware in django settings (#625)
+ 
+### Bugfixes
+
+ * fixed issue with transactions not being captured when errors occur in Flask (#635)
 
 ## v5.2.3
 [Check the diff](https://github.com/elastic/apm-agent-python/compare/v5.2.2...v5.2.3)

elasticapm/contrib/flask/__init__.py

@@ -120,6 +120,12 @@ def handle_exception(self, *args, **kwargs):
             custom={"app": self.app},
             handled=False,
         )
+        # End the transaction here, as `request_finished` won't be called when an
+        # unhandled exception occurs.
+        #
+        # Unfortunately, that also means that we can't capture any response data,
+        # as the response isn't ready at this point in time.
+        self.client.end_transaction(result="HTTP 5xx")
 
     def init_app(self, app, **defaults):
         self.app = app
@@ -182,11 +188,6 @@ def request_started(self, app):
             else:
                 trace_parent = None
             self.client.begin_transaction("request", trace_parent=trace_parent)
-
-    def request_finished(self, app, response):
-        if not self.app.debug or self.client.config.debug:
-            rule = request.url_rule.rule if request.url_rule is not None else ""
-            rule = build_name_with_http_method_prefix(rule, request)
             elasticapm.set_context(
                 lambda: get_data_from_request(
                     request,
@@ -195,14 +196,19 @@ def request_finished(self, app, response):
                 ),
                 "request",
             )
+            rule = request.url_rule.rule if request.url_rule is not None else ""
+            rule = build_name_with_http_method_prefix(rule, request)
+            elasticapm.set_transaction_name(rule, override=False)
+
+    def request_finished(self, app, response):
+        if not self.app.debug or self.client.config.debug:
             elasticapm.set_context(
                 lambda: get_data_from_response(response, capture_headers=self.client.config.capture_headers), "response"
             )
             if response.status_code:
                 result = "HTTP {}xx".format(response.status_code // 100)
             else:
                 result = response.status
-            elasticapm.set_transaction_name(rule, override=False)
             elasticapm.set_transaction_result(result, override=False)
             # Instead of calling end_transaction here, we defer the call until the response is closed.
             # This ensures that we capture things that happen until the WSGI server closes the response.

tests/contrib/flask/flask_tests.py

@@ -51,7 +51,7 @@ def test_error_handler(flask_apm_client):
     client = flask_apm_client.app.test_client()
     response = client.get("/an-error/")
     assert response.status_code == 500
-    assert len(flask_apm_client.client.events) == 1
+    assert len(flask_apm_client.client.events[ERROR]) == 1
 
     event = flask_apm_client.client.events[ERROR][0]
 
@@ -62,12 +62,16 @@ def test_error_handler(flask_apm_client):
     assert exc["handled"] is False
     assert event["culprit"] == "tests.contrib.flask.fixtures.an_error"
 
+    transaction = flask_apm_client.client.events[TRANSACTION][0]
+    assert transaction["result"] == "HTTP 5xx"
+    assert transaction["name"] == "GET /an-error/"
+
 
 def test_get(flask_apm_client):
     client = flask_apm_client.app.test_client()
     response = client.get("/an-error/?foo=bar")
     assert response.status_code == 500
-    assert len(flask_apm_client.client.events) == 1
+    assert len(flask_apm_client.client.events[ERROR]) == 1
 
     event = flask_apm_client.client.events[ERROR][0]
 
@@ -104,7 +108,7 @@ def test_get_debug_elasticapm(flask_apm_client):
     flask_apm_client.client.config.debug = True
     with pytest.raises(ValueError):
         app.test_client().get("/an-error/?foo=bar")
-    assert len(flask_apm_client.client.events) == 1
+    assert len(flask_apm_client.client.events[ERROR]) == 1
 
 
 @pytest.mark.parametrize(
@@ -267,7 +271,7 @@ def test_post_files(flask_apm_client):
             },
         )
     assert response.status_code == 500
-    assert len(flask_apm_client.client.events) == 1
+    assert len(flask_apm_client.client.events[ERROR]) == 1
 
     event = flask_apm_client.client.events[ERROR][0]
     if flask_apm_client.client.config.capture_body in ("errors", "all"):