Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exceptions captured with rescue_from :all still appear in Error Tracking when using Grape integration #3253

Open
llxff opened this issue Nov 10, 2023 · 2 comments · Fixed by #3370
Assignees
Labels
bug Involves a bug community Was opened by a community member

Comments

@llxff
Copy link

llxff commented Nov 10, 2023

Expected behavior

When using Grape's rescue_from :all to capture exceptions, I expect these exceptions to be handled internally and not to appear in Datadog's Error Tracking.

Actual behavior
Despite using rescue_from :all, exceptions are still being sent to Datadog's Error Tracking. It seems that the error is assigned to the span before the rescue_from block is called, so the error still appears in the Error Tracking dashboard.

Additional context

If Grape's instrumentation is removed, the context in rack spans is lost, e.g.:

with Grape's instrumentation:
Screenshot 2023-11-10 at 16 29 10

without Grape's instrumentation:
Screenshot 2023-11-10 at 16 30 03

@llxff llxff added bug Involves a bug community Was opened by a community member labels Nov 10, 2023
@marcotc
Copy link
Member

marcotc commented Nov 10, 2023

Hey @llxff, thank you for bringing this issue up.

I looked into the implementation of https://github.com/ruby-grape/grape-on-rack/blob/03e03b78c52752c17c07c2f08430d3fe1544da12/api/rescue_from.rb#L4C39-L4C42 and I see that it will return a status code 500 will be returned. I assume that's the behavior you are seeing in your application.

In that case, the Rack integration will consider a 500 response as an error response, and mark it as an error for Error Tracking.

How could the product best work for you here, ideally?
e.g. Would you like the grape and rack spans to be marked with error, but error tracking not being notified of it? Would you like either only one of rack or grape to be marked with error? Would like that none of the spans were marked as error?

@marcotc marcotc self-assigned this Nov 10, 2023
@llxff
Copy link
Author

llxff commented Nov 13, 2023

Hi Marco, thank you for checking that!

Generally, rescue_from is used for handling exceptions and preparing contextual responses for clients. The status code is not necessarily 500, it could be anything depending on context: e.g. 422 if the error is related to invalid parameters or 401 if the error is related to authentication.

Ideally, the span should not be marked with an error if the exception is handled. In the end, you can log the exception if you need an alert in Datadog, right?

@TonyCTHsu TonyCTHsu linked a pull request Jan 10, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Involves a bug community Was opened by a community member
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants