Add request to Report/Event

CHANGELOG.md

@@ -22,6 +22,8 @@ Changelog
   | [#691](https://github.com/bugsnag/bugsnag-ruby/pull/691)
 * Add `original_error` to `Report`/`Event` containing the original Exception instance
   | [#692](https://github.com/bugsnag/bugsnag-ruby/pull/692)
+* Add `request` to `Report`/`Event` containing HTTP request metadata
+  | [#693](https://github.com/bugsnag/bugsnag-ruby/pull/693)
 
 ### Fixes
 
@@ -43,6 +45,7 @@ Changelog
     For example, `Bugsnag::Breadcrumbs::ERROR_BREADCRUMB_TYPE` is now available as `Bugsnag::BreadcrumbType::ERROR`
 * `Report#exceptions` has been deprecated in favour of the new `errors` property
 * `Report#raw_exceptions` has been deprecated in favour of the new `original_error` property
+* Accessing request data via `Report#metadata` has been deprecated in favour of using the new `request` property. Request data will be moved out of metadata in the next major version
 
 ## v6.22.1 (11 August 2021)
 

features/fixtures/docker-compose.yml

@@ -129,6 +129,7 @@ services:
       - SQL_ONLY_BREADCRUMBS
       - ADD_ON_ERROR
       - USE_DEFAULT_AUTO_CAPTURE_SESSIONS
+      - ADD_REQUEST_ON_ERROR
     restart: "no"
 
   rails4:
@@ -165,6 +166,7 @@ services:
       - SQL_ONLY_BREADCRUMBS
       - ADD_ON_ERROR
       - USE_DEFAULT_AUTO_CAPTURE_SESSIONS
+      - ADD_REQUEST_ON_ERROR
     restart: "no"
 
   rails5:
@@ -201,6 +203,7 @@ services:
       - SQL_ONLY_BREADCRUMBS
       - ADD_ON_ERROR
       - USE_DEFAULT_AUTO_CAPTURE_SESSIONS
+      - ADD_REQUEST_ON_ERROR
     restart: "no"
 
   rails6:
@@ -237,6 +240,7 @@ services:
       - SQL_ONLY_BREADCRUMBS
       - ADD_ON_ERROR
       - USE_DEFAULT_AUTO_CAPTURE_SESSIONS
+      - ADD_REQUEST_ON_ERROR
     restart: "no"
     networks:
       default:

features/fixtures/rails3/app/config/initializers/bugsnag.rb

@@ -25,4 +25,11 @@
       })
     end)
   end
+
+  if ENV["ADD_REQUEST_ON_ERROR"] == "true"
+    config.add_on_error(proc do |report|
+      report.request[:something] = "hello"
+      report.request[:params][:another_thing] = "hi"
+    end)
+  end
 end

features/fixtures/rails4/app/config/initializers/bugsnag.rb

@@ -25,4 +25,11 @@
       })
     end)
   end
+
+  if ENV["ADD_REQUEST_ON_ERROR"] == "true"
+    config.add_on_error(proc do |report|
+      report.request[:something] = "hello"
+      report.request[:params][:another_thing] = "hi"
+    end)
+  end
 end

features/fixtures/rails5/app/config/initializers/bugsnag.rb

@@ -25,4 +25,11 @@
       })
     end)
   end
+
+  if ENV["ADD_REQUEST_ON_ERROR"] == "true"
+    config.add_on_error(proc do |report|
+      report.request[:something] = "hello"
+      report.request[:params][:another_thing] = "hi"
+    end)
+  end
 end

features/fixtures/rails6/app/config/initializers/bugsnag.rb

@@ -25,4 +25,11 @@
       })
     end)
   end
+
+  if ENV["ADD_REQUEST_ON_ERROR"] == "true"
+    config.add_on_error(proc do |report|
+      report.request[:something] = "hello"
+      report.request[:params][:another_thing] = "hi"
+    end)
+  end
 end

features/rails_features/request.feature

@@ -0,0 +1,52 @@
+Feature: Request data
+
+@rails3 @rails4 @rails5 @rails6
+Scenario: Request data is collected automatically
+  Given I start the rails service
+  When I navigate to the route "/unhandled/error?a=123&b=456" on the rails app
+  And I wait to receive a request
+  Then the request is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier"
+  And the event "unhandled" is true
+  And the exception "errorClass" equals "NameError"
+  And the exception "message" starts with "undefined local variable or method `generate_unhandled_error' for #<UnhandledController"
+  And the event "app.type" equals "rails"
+  And the event "metaData.request.clientIp" is not null
+  And the event "metaData.request.headers.Host" is not null
+  And the event "metaData.request.headers.User-Agent" is not null
+  And the event "metaData.request.headers.Version" is not null
+  And the event "metaData.request.httpMethod" equals "GET"
+  And the event "metaData.request.params.action" equals "error"
+  And the event "metaData.request.params.controller" equals "unhandled"
+  And the event "metaData.request.params.a" equals "123"
+  And the event "metaData.request.params.b" equals "456"
+  And the event "metaData.request.railsAction" equals "unhandled#error"
+  And the event "metaData.request.referer" is null
+  And the event "metaData.request.requestId" is not null
+  And the event "metaData.request.url" ends with "/unhandled/error?a=123&b=456"
+
+@rails3 @rails4 @rails5 @rails6
+Scenario: Request data can be modified in callbacks
+  Given I set environment variable "ADD_REQUEST_ON_ERROR" to "true"
+  And I start the rails service
+  When I navigate to the route "/unhandled/error?a=123&b=456" on the rails app
+  And I wait to receive a request
+  Then the request is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier"
+  And the event "unhandled" is true
+  And the exception "errorClass" equals "NameError"
+  And the exception "message" starts with "undefined local variable or method `generate_unhandled_error' for #<UnhandledController"
+  And the event "app.type" equals "rails"
+  And the event "metaData.request.something" equals "hello"
+  And the event "metaData.request.params.another_thing" equals "hi"
+  And the event "metaData.request.clientIp" is not null
+  And the event "metaData.request.headers.Host" is not null
+  And the event "metaData.request.headers.User-Agent" is not null
+  And the event "metaData.request.headers.Version" is not null
+  And the event "metaData.request.httpMethod" equals "GET"
+  And the event "metaData.request.params.action" equals "error"
+  And the event "metaData.request.params.controller" equals "unhandled"
+  And the event "metaData.request.params.a" equals "123"
+  And the event "metaData.request.params.b" equals "456"
+  And the event "metaData.request.railsAction" equals "unhandled#error"
+  And the event "metaData.request.referer" is null
+  And the event "metaData.request.requestId" is not null
+  And the event "metaData.request.url" ends with "/unhandled/error?a=123&b=456"

lib/bugsnag/report.rb

@@ -308,6 +308,14 @@ def metadata=(metadata)
       @meta_data = metadata
     end
 
+    ##
+    # Data from the current HTTP request. May be nil if no data has been recorded
+    #
+    # @return [Hash, nil]
+    def request
+      @meta_data[:request]
+    end
+
     private
 
     def generate_exception_list

spec/integrations/rack_spec.rb

@@ -92,135 +92,114 @@ class Request
       end
     end
 
+    after do
+      Object.send(:remove_const, :Rack) if @mocked_rack
+    end
+
     it "correctly redacts from url and referer any value indicated by meta_data_filters" do
-      callback = double
       rack_env = {
-        :env => true,
-        :HTTP_REFERER => "https://bugsnag.com/about?email=hello@world.com&another_param=thing",
-        "rack.session" => {
-          :session => true
-        }
+        env: true,
+        HTTP_REFERER: "https://bugsnag.com/about?email=hello@world.com&another_param=thing",
+        "rack.session" => { session: true }
       }
 
       rack_request = double
-      rack_params = {
-        :param => 'test'
-      }
       allow(rack_request).to receive_messages(
-        :params => rack_params,
-        :ip => "rack_ip",
-        :request_method => "TEST",
-        :path => "/TEST_PATH",
-        :scheme => "http",
-        :host => "test_host",
-        :port => 80,
-        :referer => "https://bugsnag.com/about?email=hello@world.com&another_param=thing",
-        :fullpath => "/TEST_PATH?email=hello@world.com&another_param=thing"
+        params: { param: 'test' },
+        ip: "rack_ip",
+        request_method: "TEST",
+        path: "/TEST_PATH",
+        scheme: "http",
+        host: "test_host",
+        port: 80,
+        referer: "https://bugsnag.com/about?email=hello@world.com&another_param=thing",
+        fullpath: "/TEST_PATH?email=hello@world.com&another_param=thing"
       )
+
       expect(::Rack::Request).to receive(:new).with(rack_env).and_return(rack_request)
 
-      # modify rack_env to include redacted referer
-      report = double("Bugsnag::Report")
-      allow(report).to receive(:request_data).and_return({
-        :rack_env => rack_env
-      })
-      expect(report).to receive(:automatic_context=).with("TEST /TEST_PATH")
-      expect(report).to receive(:user).and_return({})
-
-      config = Bugsnag.configuration
-      config.send_environment = true
-      config.meta_data_filters = ['email']
-
-      allow(report).to receive(:configuration).and_return(config)
-      expect(report).to receive(:add_tab).once.with(:request, {
-        :url => "http://test_host/TEST_PATH?email=[FILTERED]&another_param=thing",
-        :httpMethod => "TEST",
-        :params => rack_params,
-        :referer => "https://bugsnag.com/about?email=[FILTERED]&another_param=thing",
-        :clientIp => "rack_ip",
-        :headers => {
-          "Referer" => "https://bugsnag.com/about?email=[FILTERED]&another_param=thing"
-        }
-      })
-      # rack_env["HTTP_REFERER"] = "https://bugsnag.com/about?email=[FILTERED]&another_param=thing"
-      expect(report).to receive(:add_tab).once.with(:environment, rack_env)
-      expect(report).to receive(:add_tab).once.with(:session, {
-        :session => true
-      })
+      Bugsnag.configure do |config|
+        config.send_environment = true
+        config.meta_data_filters = ['email']
+        config.request_data[:rack_env] = rack_env
+      end
 
+      report = Bugsnag::Report.new(RuntimeError.new('abc'), Bugsnag.configuration)
+
+      callback = double
       expect(callback).to receive(:call).with(report)
 
       middleware = Bugsnag::Middleware::RackRequest.new(callback)
       middleware.call(report)
+
+      expect(report.request).to eq({
+        url: "http://test_host/TEST_PATH?email=[FILTERED]&another_param=thing",
+        httpMethod: "TEST",
+        params: { param: 'test' },
+        referer: "https://bugsnag.com/about?email=[FILTERED]&another_param=thing",
+        clientIp: "rack_ip",
+        headers: {
+          "Referer" => "https://bugsnag.com/about?email=[FILTERED]&another_param=thing"
+        }
+      })
+
+      expect(report.metadata[:request]).to be(report.request)
+      expect(report.metadata[:environment]).to eq(rack_env)
+      expect(report.metadata[:session]).to eq({ session: true })
     end
 
     it "correctly extracts data from rack middleware" do
-      callback = double
       rack_env = {
-        :env => true,
-        :HTTP_test_key => "test_key",
-        "rack.session" => {
-          :session => true
-        }
+        env: true,
+        HTTP_test_key: "test_key",
+        "rack.session" => { session: true }
       }
 
       rack_request = double
-      rack_params = {
-        :param => 'test'
-      }
       allow(rack_request).to receive_messages(
-        :params => rack_params,
-        :ip => "rack_ip",
-        :request_method => "TEST",
-        :path => "/TEST_PATH",
-        :scheme => "http",
-        :host => "test_host",
-        :port => 80,
-        :referer => "referer",
-        :fullpath => "/TEST_PATH"
+        params: { param: 'test' },
+        ip: "rack_ip",
+        request_method: "TEST",
+        path: "/TEST_PATH",
+        scheme: "http",
+        host: "test_host",
+        port: 80,
+        referer: "referer",
+        fullpath: "/TEST_PATH"
       )
+
       expect(Rack::Request).to receive(:new).with(rack_env).and_return(rack_request)
 
-      report = double("Bugsnag::Report")
-      allow(report).to receive(:request_data).and_return({
-        :rack_env => rack_env
-      })
-      expect(report).to receive(:automatic_context=).with("TEST /TEST_PATH")
-      expect(report).to receive(:user).and_return({})
-
-      config = Bugsnag.configuration
-      config.send_environment = true
-      config.meta_data_filters = []
-
-      allow(report).to receive(:configuration).and_return(config)
-      expect(report).to receive(:add_tab).once.with(:environment, rack_env)
-      expect(report).to receive(:add_tab).once.with(:request, {
-        :url => "http://test_host/TEST_PATH",
-        :httpMethod => "TEST",
-        :params => rack_params,
-        :referer => "referer",
-        :clientIp => "rack_ip",
-        :headers => {
-          "Test-Key" => "test_key"
-        }
-      })
-      expect(report).to receive(:add_tab).once.with(:session, {
-        :session => true
-      })
+      Bugsnag.configure do |config|
+        config.send_environment = true
+        config.meta_data_filters = []
+        config.request_data[:rack_env] = rack_env
+      end
+
+      report = Bugsnag::Report.new(RuntimeError.new('abc'), Bugsnag.configuration)
 
+      callback = double
       expect(callback).to receive(:call).with(report)
 
       middleware = Bugsnag::Middleware::RackRequest.new(callback)
       middleware.call(report)
-    end
 
-    after do
-      Object.send(:remove_const, :Rack) if @mocked_rack
-    end
+      expect(report.request).to eq({
+        url: "http://test_host/TEST_PATH",
+        httpMethod: "TEST",
+        params: { param: 'test' },
+        referer: "referer",
+        clientIp: "rack_ip",
+        headers: { "Test-Key" => "test_key" }
+      })
 
+      expect(report.metadata[:request]).to be(report.request)
+      expect(report.metadata[:environment]).to eq(rack_env)
+      expect(report.metadata[:session]).to eq({ session: true })
+    end
   end
 
-  it "don't mess with middlewares list on each req" do
+  it "doesn't change the middleware list on each request" do
     app = lambda { |env| ['200', {}, ['']] }
 
     Bugsnag::Rack.new(app)