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

Capture event for HTTP requests resulted in server error #38

Closed
12 tasks done
bruno-garcia opened this issue Jul 20, 2022 · 10 comments
Closed
12 tasks done

Capture event for HTTP requests resulted in server error #38

bruno-garcia opened this issue Jul 20, 2022 · 10 comments

Comments

@bruno-garcia
Copy link
Member

bruno-garcia commented Jul 20, 2022

An HTTP request can 'succeed' from the point of view of code. There wasn't a DNS failure, or a network timeout. No Exception was thrown.

Currently, in many SDKs, that means Sentry doesn't capture any events. The thinking behind it was: It's the user code that is responsible in this case. They might want to do a retry after a few seconds. If that retry works, then the first failure might not be considered an error at all.
The might want to capture the first event with level info (as opposed to error) and only once all retries are exhausted, the error level event is sent.

On the flip side, the assumption that the user code will "do the right thing" is flawed. We need to have sensible defaults, that will give value to our users out of the box. If there's some outage caused by the backend, a Mobile developer would want to be aware of that. It might come in through a series of failed HTTP requests, but the spike in events will trigger and alert, and the relevant teams can get involved to try fix the issue.

For that reason we're designing a new feature to be built-in to SDKs that have HTTP integrations. Some thoughts:

  • The feature must be opt-in at first. If we turn this on by default, it's possible that a large number of errors will deplete the users quota.
  • If this is a new installation of the SDK, the feature could be "on by default". We tackle that by adding the opt-in flag to the project onboarding wizard in Sentry. Same way we do 'enable performance' in SDKs.
  • What status codes are we going to capture automatically? All 500-599 range? What about 4xx?
  • What type of configuration will this require? What type of callbacks?

The target of this feature is any SDK with HTTP integrations. For example:

  • Android OkHttp
  • Java/Andrid OpenFeign
  • .NET/Unity HttpMessageHandler
  • UnityWebRequest
  • NSURLSession
  • XHR/fetch (JavaScript/React Native)
  • Flutter dio
  • etc..

Additionally from the event captured, we could optionally attach the request and/or response body. This is tracked here:

Relevant PRs in Flutter

Internal customer request issue: https://getsentry.atlassian.net/browse/FEEDBACK-1492

Docs

Preview Give feedback

SDKs

Preview Give feedback
  1. Effort: Small Impact: Medium Platform: Dart
    denrase
  2. Effort: Small Feature Impact: Large Platform: .NET
    jamescrosswell

Backend

Preview Give feedback
  1. olksdr
  2. Scope: Backend Scope: Frontend
@marandaneto
Copy link

marandaneto commented Jul 21, 2022

Adding the response metadata/body as part of the Event context would be very useful along with this feature.
Eg getsentry/sentry-dart#934
Right Now SDKs only have the built-in request metadata, but the HTTP response code sometimes might not be enough and the offending bits might be in the response metadata/body.

@bruno-garcia
Copy link
Member Author

@marandaneto great point, we raised #41 and I added to the description

@marandaneto
Copy link

Removed the label for Apple because, with Swizzling, it should work for every integration.
Keeping the one for Android, it's done for OkHttp but it can be done for other integrations such as GraphQL, etc.

@marandaneto marandaneto moved this from In Progress to Backlog in Mobile & Cross Platform SDK Nov 8, 2022
@marandaneto
Copy link

Pending for Dart/Flutter getsentry/sentry-dart#1115

@marandaneto
Copy link

@krystofwoldrich this has been implemented on the JS SDK, can we confirm it works for RN as well and mark the checkbox?

@krystofwoldrich
Copy link
Member

krystofwoldrich commented Jan 20, 2023

JS SDK implementation works on RN native. Just needs to be added to the RN docs.

@bruno-garcia
Copy link
Member Author

Capacitor will also pickup the JS implementation.
Only .NET left? cc @mattjohnsonpint

@romtsn
Copy link
Member

romtsn commented Apr 5, 2023

Android/Java tracking here getsentry/sentry-java#2639

@marandaneto marandaneto removed their assignment Apr 18, 2023
@mattjohnsonpint
Copy link

I think .NET was the last one - Just released today in 3.31.0. Closing this as completed. Thanks all!

@mattjohnsonpint
Copy link

Only Cocoa has good docs for this. We need to make that page common so we can all add them.
See getsentry/sentry-docs#6984

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Status: In Progress 📈
Development

No branches or pull requests

6 participants