From 892ff3a3ae640272f8712fb190242f2b1fe010a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 12 Jan 2017 13:12:50 -0500 Subject: Check for env[Grape::Env::GRAPE_ROUTING_ARGS] instead of endpoint.route MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `endpoint.route` is calling `env[Grape::Env::GRAPE_ROUTING_ARGS][:route_info]` but `env[Grape::Env::GRAPE_ROUTING_ARGS]` is `nil` in the case of a 405 response Signed-off-by: Rémy Coutable --- .../26587-metrics-middleware-endpoint-is-nil.yml | 4 ++++ lib/gitlab/metrics/rack_middleware.rb | 15 +++++++++++---- spec/lib/gitlab/metrics/rack_middleware_spec.rb | 11 +++++++++++ 3 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 changelogs/unreleased/26587-metrics-middleware-endpoint-is-nil.yml diff --git a/changelogs/unreleased/26587-metrics-middleware-endpoint-is-nil.yml b/changelogs/unreleased/26587-metrics-middleware-endpoint-is-nil.yml new file mode 100644 index 00000000000..5891a5ef6e8 --- /dev/null +++ b/changelogs/unreleased/26587-metrics-middleware-endpoint-is-nil.yml @@ -0,0 +1,4 @@ +--- +title: Check for env[Grape::Env::GRAPE_ROUTING_ARGS] instead of endpoint.route +merge_request: 8544 +author: diff --git a/lib/gitlab/metrics/rack_middleware.rb b/lib/gitlab/metrics/rack_middleware.rb index d01d47a6a7a..47f88727fc8 100644 --- a/lib/gitlab/metrics/rack_middleware.rb +++ b/lib/gitlab/metrics/rack_middleware.rb @@ -71,10 +71,17 @@ module Gitlab def tag_endpoint(trans, env) endpoint = env[ENDPOINT_KEY] - # endpoint.route is nil in the case of a 405 response - if endpoint.route - path = endpoint_paths_cache[endpoint.route.request_method][endpoint.route.path] - trans.action = "Grape##{endpoint.route.request_method} #{path}" + begin + route = endpoint.route + rescue + # endpoint.route is calling env[Grape::Env::GRAPE_ROUTING_ARGS][:route_info] + # but env[Grape::Env::GRAPE_ROUTING_ARGS] is nil in the case of a 405 response + # so we're rescuing exceptions and bailing out + end + + if route + path = endpoint_paths_cache[route.request_method][route.path] + trans.action = "Grape##{route.request_method} #{path}" end end diff --git a/spec/lib/gitlab/metrics/rack_middleware_spec.rb b/spec/lib/gitlab/metrics/rack_middleware_spec.rb index 7371b578a48..fb470ea7568 100644 --- a/spec/lib/gitlab/metrics/rack_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/rack_middleware_spec.rb @@ -126,5 +126,16 @@ describe Gitlab::Metrics::RackMiddleware do expect(transaction.action).to eq('Grape#GET /projects/:id/archive') end + + it 'does not tag a transaction if route infos are missing' do + endpoint = double(:endpoint) + allow(endpoint).to receive(:route).and_raise + + env['api.endpoint'] = endpoint + + middleware.tag_endpoint(transaction, env) + + expect(transaction.action).to be_nil + end end end -- cgit v1.2.1