Don't log client-side errors in Grape middleware (e.g. validation errors)

[padarom]

When using the Appsignal::Integrations::Grape middleware a lot of exceptions are logged to Appsignal that in our mind don't belong there. These include authorization errors, validation errors, or generally all kinds of client-side errors that would result in a 4xx HTTP status code.

We would rather have these errors logged for our client applications (such as frontends, or other APIs that happen to make incorrect API calls) than the service where the error is ultimately spotted.

appsignal-ruby/lib/appsignal/integrations/grape.rb

Lines 25 to 28 in 22b57fe

	rescue Exception => error # rubocop:disable Lint/RescueException
	# Do not set error if "grape.skip_appsignal_error" is set to `true`.
	transaction.set_error(error) unless env["grape.skip_appsignal_error"]
	raise error

There is a way to disable Grape error reporting, but I don't find it flexible enough, as I would have to know the error reason ahead of time, or perform some env reconfiguring when raising an error.

We have reimplemented our middleware for this use case something like this:

  rescue Exception => e # rubocop:disable Lint/RescueException
    statuscode = e.try(:status) || 500
    transaction.set_error(e) unless (400..499).include?(statuscode)

This makes use of the fact that Grape errors specifically all have an optional status property that defines the response status (e.g. 400 for input validation errors). Our custom API errors also implement this field.

Would it make sense to add something like this to your provided middleware or how else would you recommend working around this issue, without having to implement/monkeypatch our own middleware?

[shairyar]

Hey @padarom, we will discuss this and get back to you.

[tombruijn]

Sorry for the very, very late response to this issue. I'm currently reviewing our Grape instrumentation middleware to see if we can improve this. I don't think we want to add a response status code check or something like that to the middleware.

If the error reaches our middleware, no other exception handling is present in the app. I think apps should add their own exception handling to render specific error pages. AppSignal does not report other errors like Grape::Exceptions::InvalidVersionHeader or parameter filtering errors.

If you have any specific examples you'd like us to review, I'd be happy to reopen and improve this.