diff options
author | Rémy Coutable <remy@rymai.me> | 2017-01-12 13:12:50 -0500 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2017-01-12 23:15:25 -0500 |
commit | 892ff3a3ae640272f8712fb190242f2b1fe010a0 (patch) | |
tree | c62ce2cfb477839e94ce3899e061b96e7776bd93 | |
parent | 707570ac7de4867a62ad4c94c95e43e8ca0afbcb (diff) | |
download | gitlab-ce-892ff3a3ae640272f8712fb190242f2b1fe010a0.tar.gz |
Check for env[Grape::Env::GRAPE_ROUTING_ARGS] instead of endpoint.route26587-metrics-middleware-endpoint-is-nil
`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 <remy@rymai.me>
-rw-r--r-- | changelogs/unreleased/26587-metrics-middleware-endpoint-is-nil.yml | 4 | ||||
-rw-r--r-- | lib/gitlab/metrics/rack_middleware.rb | 15 | ||||
-rw-r--r-- | spec/lib/gitlab/metrics/rack_middleware_spec.rb | 11 |
3 files changed, 26 insertions, 4 deletions
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 |