Create a Sentry event for failed HTTP requests

[bruno-garcia]

Problem Statement

Failed HTTP requests don't result in Sentry events

Solution Brainstorm

• Spec: https://develop.sentry.dev/sdk/features/#http-client-errors

Main issue with description

• Capture event for HTTP requests resulted in server error team-mobile#38

[mattjohnsonpint]

Applies to HttpClient - we already have SentryHttpMessageHandler, so this should be fairly trivial to add there.

We should think about what stacktrace (if any) belongs with the error.

[mattjohnsonpint]

This would be part of our SentryHttpMessageHandler

[jamescrosswell]

I'm looking into this now and in particular the docs regarding FailedRequestTargets.

We already have the TracePropagationTarget class, which appears to do exactly what we want. About the only thing wrong with it is the name. If it wasn't public I'd suggest we simply rename it to PropagationTarget and reuse it for SentryOptions.FailedRequestTargets as well.

Two alternatives I can think of then (and there may be more):

1. Refactor key functionality to a new PropagationTarget class that TracePropagationTarget inherits from...
2. Refactor into a Utils/Helper class, similar to the PropogationTargetsUtils class in the Java SDK.
3. Something else entirely.

Are there any strong opinions about which approach to take? I was leaning towards option 1 personally... but it may have drawbacks I haven't considered.

[mattjohnsonpint]

Agreed, the name is too specific. Even PropogationTarget is too specific, IMHO. What do you think of SubstringOrRegexPattern?

[mattjohnsonpint]

To retain compatibility, we can create a new type for this, move most of the functionality, and make it the base type of the existing one. In the next major, we can remove the current one and replace it with the new one. (Until then, we'll just leave a comment. We don't need to obsolete it.)

[jamescrosswell]

SubstringOrRegexPattern works... quite literal ;-)

[jamescrosswell]

So I think I'm in the home stretch on this. All I have to do now is actually send the event data!

I think I'll be able to take a lot of inspiration from SentryOkHttpInterceptor.captureEvent in the Java SDK. I have a few questions though.

The Java SDK contains a SentryHttpClientException and an ExceptionMechanismException decorator, which is what gets passed to the SentryEvent constructor.

I see the .NET SDK includes a SentryException class (which is not derived from System.Exception) that has a Mechanism property on it. And the SentryEvent class in the .NET SDK has a SentryExceptions property as well as an Exception property.

I'm trying to work out, in .NET, how we should be recording all the appropriate information. My best guess is:

1. Create a new SentryHttpClientException
2. Leverage SentryExceptionExtensions.SetSentryMechanism to record details of the mechanism in that exception
3. Create a new SentryEvent, passing our newly created Exception in via the constructor

I suspect (but haven't gone all the way down the rabbit hole to verify) that the SentryEvent.SentryExceptions property would then get set when Sentry processes the event (in the MainExceptionProcessor). So I don't have to manually populate the SentryEvent.SentryExceptions right?

Is that the general flow? Maybe there's a good example of how exception events are generated somewhere else in the .NET SDK code that would be good to refer to?

[mattjohnsonpint]

I see the .NET SDK includes a SentryException class (which is not derived from System.Exception) ...

Correct. SentryException is the model class for passing exceptions to Sentry, within a SentryEvent. It's not an actual .NET Exception.

1. Create a new SentryHttpClientException ...

We don't need to create any custom exception here, because .NET already has HttpRequestException designed for this. We can also just let .NET throw the exception for us, so we don't have to create a custom message. We should still set the mechanism though.

try
{
    response.EnsureSuccessStatusCode();
}
catch (HttpRequestException exception)
{
    exception.SetSentryMechanism(MechanismType);
    _hub.CaptureException(exception);
}

I suspect ... that the SentryEvent.SentryExceptions property would then get set when Sentry processes the event. ... So I don't have to manually populate the SentryEvent.SentryExceptions right?

Correct. We don't need to do anything special here. Just capture the exception and let the existing SDK components do the rest.

[mattjohnsonpint]

I'll add some comments inline the draft PR also. Thanks. 😃