SentryHttpFailedRequestHandler disposes Content on .net framework about sentry-dotnet HOT 2 OPEN

Hi @Liklainy, thanks for getting in touch.

@bitsandfoxes we could fairly easily implement our own EnsureSuccessStatusCode for target frameworks below .net5.0. I know Matt was against this on principal, as it would mean we wouldn't get any fixes if the code we copied changed.

However, we've already got a workaround for the broken framework code, but at the moment that workaround only protects the proper functioning of our own code (but doesn't guarantee other stuff won't get broken).

In this particular case, I'd be OK to bend the "don't copy framework code into our solution" rule, as long as our code is only used on .netcore3.1 and below. Under the hood, all EnsureSuccessStatusCode does is throw an exception if the response status code isn't in the 200 range, so trivial to implement ourselves:

• https://github.com/microsoft/referencesource/blob/51cf7850defa8a17d815b4700b67116e3fa283c2/System/net/System/Net/Http/HttpResponseMessage.cs#L117-L120
• https://github.com/microsoft/referencesource/blob/51cf7850defa8a17d815b4700b67116e3fa283c2/System/net/System/Net/Http/HttpResponseMessage.cs#L142-L159

from sentry-dotnet.

Hi @jamescrosswell, thank you for quick reply!

I can suggest two ways to fix this issue:

1. As a simple and crude fix we can store Content stream locally, make it null so it will not be disposed, and restore it in finally
2. Throwing an exception just to immediately catch it doesn't make any sense. We only need to create an exception with new. But HttpRequestException has a different structure in different versions, for example, StatusCode was added as a separate property which probably should be initialized for compatibility, which leads to more #if workarounds. Maybe the best way will be to create a separate custom exception class

from sentry-dotnet.