Ensure no exception is raised when Raven tries to get the current user in API context38571-fix-exception-in-raven-report

Signed-off-by: Rémy Coutable <remy@rymai.me>

3 files changed, 32 insertions, 3 deletions

diff --git a/changelogs/unreleased/38571-fix-exception-in-raven-report.yml b/changelogs/unreleased/38571-fix-exception-in-raven-report.yml
new file mode 100644
index 00000000000..62e3b8d304c
--- /dev/null
+++ b/changelogs/unreleased/38571-fix-exception-in-raven-report.yml
@@ -0,0 +1,6 @@
+---
+title: Ensure no exception is raised when Raven tries to get the current user in API
+  context
+merge_request: 14580
+author:
+type: fixed
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 1e8475ba3ec..4964a76bef6 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -464,10 +464,12 @@ module API
       header(*Gitlab::Workhorse.send_artifacts_entry(build, entry))
     end
 
-    # The Grape Error Middleware only has access to env but no params. We workaround this by
-    # defining a method that returns the right value.
+    # The Grape Error Middleware only has access to `env` but not `params` nor
+    # `request`. We workaround this by defining methods that returns the right
+    # values.
     def define_params_for_grape_middleware
-      self.define_singleton_method(:params) { Rack::Request.new(env).params.symbolize_keys }
+      self.define_singleton_method(:request) { Rack::Request.new(env) }
+      self.define_singleton_method(:params) { request.params.symbolize_keys }
     end
 
     # We could get a Grape or a standard Ruby exception. We should only report anything that
diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb
index 98c49d3364c..060c8902471 100644
--- a/spec/requests/api/helpers_spec.rb
+++ b/spec/requests/api/helpers_spec.rb
@@ -480,6 +480,27 @@ describe API::Helpers do
 
       handle_api_exception(exception)
     end
+
+    context 'with a personal access token given' do
+      let(:token) { create(:personal_access_token, scopes: ['api'], user: user) }
+
+      # Regression test for https://gitlab.com/gitlab-org/gitlab-ce/issues/38571
+      it 'does not raise an additional exception because of missing `request`' do
+        # We need to stub at a lower level than #sentry_enabled? otherwise
+        # Sentry is not enabled when the request below is made, and the test
+        # would pass even without the fix
+        expect(Gitlab::Sentry).to receive(:enabled?).twice.and_return(true)
+        expect(ProjectsFinder).to receive(:new).and_raise('Runtime Error!')
+
+        get api('/projects', personal_access_token: token)
+
+        # The 500 status is expected as we're testing a case where an exception
+        # is raised, but Grape shouldn't raise an additional exception
+        expect(response).to have_gitlab_http_status(500)
+        expect(json_response['message']).not_to include("undefined local variable or method `request'")
+        expect(json_response['message']).to start_with("\nRuntimeError (Runtime Error!):")
+      end
+    end
   end
 
   describe '.authenticate_non_get!' do