Make HttpTransport public (for other SDKs)

[vaind]

This is required to be able to reuse the logic in HttpTransport while implementing a custom HttpClient.

Required for sentry-unity

#skip-changelog

[codecov-commenter]

Codecov Report

Merging #1553 (a677b87) into main (e3ca414) will increase coverage by 0.01%.
The diff coverage is 88.23%.

@@            Coverage Diff             @@
##             main    #1553      +/-   ##
==========================================
+ Coverage   82.78%   82.80%   +0.01%     
==========================================
  Files         219      219              
  Lines        7419     7421       +2     
  Branches     1417     1418       +1     
==========================================
+ Hits         6142     6145       +3     
- Misses        852      854       +2     
+ Partials      425      422       -3     

Impacted Files	Coverage Δ
src/Sentry/Internal/Http/HttpTransport.cs	94.28% <88.23%> (-0.65%)	⬇️
...gnosticSource/SentryOptionsDiagnosticExtensions.cs	33.33% <0.00%> (-33.34%)	⬇️
src/Sentry/Exceptions/SentryStackFrame.cs	78.02% <0.00%> (+3.29%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e3ca414...a677b87. Read the comment docs.

[SimonCropp]

SentryOptions exposes a SentryHttpClientFactory property which allows you to use a custom HttpClient

public interface ISentryHttpClientFactory
{
    /// <summary>
    /// Creates an HttpClient using the specified options.
    /// </summary>
    /// <param name="options">The options.</param>
    /// <returns><see cref="HttpClient"/>.</returns>
    HttpClient Create(SentryOptions options);
}

does this meet your requirements?

[vaind]

Unfortunately not. I've tried that (see approaches in the linked sentry-unity PR description), but it doesn't work in Unity webgl because the HttpTransport still uses async/await which requires multithreading and gets stuck, because there's none. What I had to so was override the async methods with unity's goroutine implementation and reuse the logic from the base class.

[bruno-garcia]

@mattjohnsonpint should we move this to Sentry.Http namespace (or other) or really open the door to have public types under the Internal namespace to 'signify' this won't follow semver.

ASP.NET Core has public types under Internal namespaces, I know as I've relied on them without noticying in the past :D

[mattjohnsonpint]

Looks like this log message was removed?

[mattjohnsonpint]

I have some concerns.

It seems that while this would make some of the functionality in HttpTransport available to be used by UnityWebRequestTransport (in the linked Unity PR), that class would no longer be accurately implementing the ITransport interface.

I can also see that we would then end up with two places to remember to add features and fix things. For example, I'm currently working on adding client reports (see draft PR #1556), which is adding more business logic to the background worker and the transport. These wouldn't carry over to the Unity implementation so would have to be implemented there also.

I think rather than exposing HttpClient public, we'd likely be better off moving the common functionality to an abstract base class for them both to share, and making that public. At least that way it's not implied that UnityWebTransport implements ITransport. I also think we may need something similar for the background workers. There may be a couple of new interfaces needed as well.

In short, exposing internals smells like we're missing some interfaces or shared base classes. I'd rather refactor for that now and built on top of a stronger contract.

[mattjohnsonpint]

@vaind - FYI, I'm working on an approach that should address my concerns and still provide what you need for Unity. I'll have a PR out shortly. Blocking this PR for now. Thanks.

[mattjohnsonpint]

Closing in favor of #1560 instead.

[SimonCropp]

@mattjohnsonpint can this branch be deleted?