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 HTTP Response Body for SentryHttpClient #2220

Open
sumanthratna opened this issue Aug 8, 2024 · 9 comments · May be fixed by #2293
Open

Capture HTTP Response Body for SentryHttpClient #2220

sumanthratna opened this issue Aug 8, 2024 · 9 comments · May be fixed by #2293

Comments

@sumanthratna
Copy link

sumanthratna commented Aug 8, 2024

Problem Statement

I would like to automatically capture the following information for all outgoing requests made by SentryHttpClient:

  • request URL
  • request method
  • request headers
  • request body
  • response status code
  • response headers
  • response body

This should be opt-in, similar to networkDetailAllowUrls in the JavaScript SDK: https://docs.sentry.io/platforms/javascript/session-replay/configuration/#network-details

Solution Brainstorm

https://github.com/getsentry/rfcs/blob/main/text/0022-response-context.md

Are you willing to submit a PR?

Yes

@buenaflor
Copy link
Contributor

hey, what did you have in mind where you want this information to show up in the sentry product?

@sumanthratna
Copy link
Author

sumanthratna commented Aug 9, 2024

@buenaflor Here is what I'm thinking (based on the accepted RFC linked above):

Each request made through SentryHttpClient should add a transaction within the current trace. Each transaction has a "Request" context and a "Response" context, which hold the requested data (listed above in the bullet points). [For request and response bodies, attachments on the transaction may be more suitable due to size limitations with context.]

Our use-case is that when an HTTP request fails, we don't just want to know which request failed, but we want to see the entire trace of requests and responses that led up to the failing request.

Does this make sense? Thanks!

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Aug 9, 2024
@sumanthratna
Copy link
Author

Another possible way to capture + present this information could be Breadcrumbs (which I believe already support outgoing HTTP requests, but doesn't include request body or response data)

@kahest
Copy link
Member

kahest commented Aug 12, 2024

@sumanthratna if I understand correctly, you're looking for our Dio integration, which does exactly what you're looking for when using Dio. If you use a different HTTP library, we'd be interested to know.
For context - SentryHttpClient SentryClient (see below) is our SDK-internal HTTP client that performs the necessary HTTP requests from the SDK to Sentry and not something to be used by the host app.

@ueman
Copy link
Collaborator

ueman commented Aug 12, 2024

@kahest I think you're confusing the SentryClient and the SentryHttpClient, since SentryHttpClient is the same thing as the DIO integration but for the http package.

To get the most out of the http integrations, you have to set the following options:

These options work for both, the SentryHttpClient and the sentry_dio integration.

Please be aware, that this is essentially a privacy and security nightmare, since you would be able to see users PII and their username/password. Please also be aware, that this only works when the request actually fails.

@kahest
Copy link
Member

kahest commented Aug 12, 2024

@ueman ah yes, you're totally right, thanks :)

@sumanthratna
Copy link
Author

sumanthratna commented Aug 12, 2024

To get the most out of the http integrations, you have to set the following options:

These options work for both, the SentryHttpClient and the sentry_dio integration.

Please be aware, that this is essentially a privacy and security nightmare, since you would be able to see users PII and their username/password. Please also be aware, that this only works when the request actually fails.

Thanks! I set these options and I now see Request Body and Request Headers under my Sentry Issues. However, I don't see Response body. I skimmed through the Dart SDK source code and it looks like maxResponseBodySize is only used by the Dio integration. Does maxResponseBodySize work with SentryHttpClient?

edit: also, is there any mechanism to capture response headers, in the same way that SentryHttpClient captures request headers, request body, and response body? it seems like response headers are presented under Contexts

edit: shouldn't data be passed to SentryResponse if sendDefaultPii is true? (see below)

event.contexts.response = SentryResponse(
headers: _hub.options.sendDefaultPii ? response.headers : null,
bodySize: response.contentLength,
statusCode: response.statusCode,
);

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Aug 12, 2024
@sumanthratna
Copy link
Author

sumanthratna commented Aug 13, 2024

@kahest @ueman re: maxResponseBodySize, see #624 which proposes this feature and #1557 which implements it, only for Dio. I couldn't find a corresponding PR for the http integration.

Is my understanding correct that the Sentry Dart SDK should be updated to implement maxResponseBodySize for the http integration?

@ueman
Copy link
Collaborator

ueman commented Aug 13, 2024

Yeah, seems like you're correct.

A note for the future implementer:
Reading the body of a StreamedRequestfrom the http package hinders the body from being read again, meaning you need to deep clone it or something before returning it, in order to implement this feature. It's quite tricky to do correctly, so please be sure it works correctly before releasing it.

@sumanthratna sumanthratna changed the title Capture network details for SentryHttpClient Capture HTTP Response Body for SentryHttpClient Aug 13, 2024
@buenaflor buenaflor moved this from Needs Discussion to Backlog in Mobile & Cross Platform SDK Aug 29, 2024
@buenaflor buenaflor moved this from Backlog to In Progress in Mobile & Cross Platform SDK Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Status: In Progress
5 participants