Sentry-dart: SentryHttpClient captures errored requests and set the 'request' field accordingly

[ueman]

The SentryHttpClient should add the last request to crash reports. This should work for all supported platform.

I feel like this should be an easy change because requests are already recorded as breadcrumbs.

[marandaneto]

@ueman do you mean the request field? https://develop.sentry.dev/sdk/event-payloads/request/

request is not part of the Scope, and after reading this:

The Request interface contains information on a HTTP request related to the event

so adding the last request to crash reports would cause false positives IMO and only the breadcrumb would be enough-right approach.

my suggestion would be, capturing events automatically out of the HTTP requests as we do for example with Spring.

for example:

try {
  final response = await _client.send(request);
  // add breadcrumb as we already do
  return response;
} catch (error) {
  // capture event with proper `request` field.
  rethrow;
}

is that what you meant?

[ueman]

Yep, that is what I meant 👍

[marandaneto]

Yep, that is what I meant 👍

nice, just changed the title a bit so it's easier to understand.

[marandaneto]

this should be an opt-in feature, some people prefer to capture themselves, maybe a ctor param on SentryHttpClient which tells us either to capture events or not, and if so, maybe we could also define when to capture eg >= error code 400 or >= error code 500 maybe or only if an exception has happened, maybe we check how the other SDKs do it, this really depends on how people do their APIs so it should not be enabled by default and some level of configuration would be nice

[ueman]

This is twofolds:

• Requests which are aborted or did not finish. So requests which do not even get a response. For example when the user lost access to the internet while doing a request.
• Requests which return a failure status code, like 500 or 400.

Also I think we should probably think about having multiple clients.
One for breadcrumbs, one for errored/aborted requests and one for reading the performance tracing headers. For the user it could still be SentryHttpClient which then combines all of the clients.

[ueman]

This should be opt in at first. There should be an enum for request size which tells us if it should be included.

[marandaneto]

related to payload size -> https://docs.sentry.io/platforms/dotnet/guides/aspnetcore/configuration/options/#max-request-body-size