From 9c82e61eaa89f080e3e17f9f107b744ab3dc7800 Mon Sep 17 00:00:00 2001 From: David McDonald Date: Wed, 1 Jul 2020 13:52:04 +0100 Subject: [PATCH] Move init of gds metrics before csrf This solves two problems - it makes our response times more accurate as it means we start measuring the response time earlier (otherwise we aren't recording the time spent by `csrf` and `login_manager`s `before_request` functions - is a temporary fix for a bug in the gds python metrics library as explained below. Currently, when a request comes in it goes through various `before_request` functions. Currently it goes through the function introduced by the csrf client and then the one introduced by the metrics client. If an exception is thrown by the csrf.before_request function then we do not run the `metrics.before_request` function. This would happen in the case that a CSRF token is invalid and then the main body of the request would not process but then all `teardown_request` functions will run. When the `metrics.teardown_request` function runs it looks for `g._gds_metrics_start_time`, however this attribute is not availble on the flask global object as it was not created as the `metrics.before_request` function that creates it did not run. This then throws an `AttributeError` and results in a 500 for the user. The short term solution for this (initing metrics before csrf) means that `_gds_metrics_start_time` will be set before csrf is at risk of throwing an exception. A separate PR will be put into the gds metrics python library to remove the risk of an `AttributeError` and instead to log a warning instead of throwing an uncaught exception. --- app/__init__.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index daaaae04e..b596bdfdd 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -130,11 +130,13 @@ def create_app(application): init_jinja(application) for client in ( - # Gubbins + # Note, metrics purposefully first so we start measuring response times as early as possible before any + # other `app.before_request` handlers (introduced by any of these clients) are processed (which would + # otherwise mean we aren't measuring the full response time) + metrics, csrf, login_manager, - metrics, proxy_fix, request_helper,