bugsnag / bugsnag-ruby

BugSnag error monitoring & reporting software for rails, sinatra, rack and ruby
https://docs.bugsnag.com/platforms/ruby
MIT License
249 stars 174 forks source link

Bugsnag does not report JSON bodies that are arrays #790

Closed bethesque closed 1 year ago

bethesque commented 1 year ago

Hi Bugsnag [sic] team, from the Pactflow [sic] team!

Describe the bug

When there is an error for a request that has a top level array (eg. application/json-patch+json), the params are not reported to Bugsnag (ie. there are no params in the event screen)

Expected behaviour

Any successfully parsed JSON body should be shown in the params section of the event screen.

Steps to reproduce

Cause an error to happen in a request with a Content-Type of application/json-patch+json and a top level array as per https://jsonpatch.com/.

Environment

Example code snippet

The reason only hashes are reported is due to this line in the bugsnag rack_request class.

https://github.com/bugsnag/bugsnag-ruby/blob/50957b4/lib/bugsnag/middleware/rack_request.rb#L112

    def add_request_body(report, request, env)
      body = parsed_request_body(request, env)

      # this request may not have a body
      return unless body.is_a?(Hash) && !body.empty?

      report.add_metadata(:request, :body, body)
    end


Is there any technical reason why the request body cannot be returned if it is an object other than a Hash? I'm happy to submit a PR to change this logic, if so.

johnkiely1 commented 1 year ago

Hey @bethesque, what you are suggesting seems sensible. We'd be happy to accept a PR if you've got the time to make one.

bethesque commented 1 year ago

What would the UI do if a non-collection object was submitted? (eg. just a String or Integer). It would be odd, I know, but from 10 years of maintaining a contract testing library for JSON request/response bodies, I know people do odd things!

imjoehaines commented 1 year ago

What would the UI do if a non-collection object was submitted? (eg. just a String or Integer). It would be odd, I know, but from 10 years of maintaining a contract testing library for JSON request/response bodies, I know people do odd things!

The UI can handle any type of value that's representable by JSON so a bare int/string/etc… is fine 🙂

I think the only constraint we'd need is not to attach the body if parsed_request_body returns nil since that's what it returns when it fails

clr182 commented 1 year ago

Hi, As this issue hasn't seen any activity for some time I will be closing it out. Please feel free to reopen if there are any further questions.