analytics-swift icon indicating copy to clipboard operation
analytics-swift copied to clipboard

Pluggable Networking Stacks

Open brianmichel opened this issue 2 years ago • 4 comments

When using Swift on Windows and Linux the built-in URLSession code has a pretty nasty bug which can result in a crash as outlined here, https://github.com/apple/swift-corelibs-foundation/issues/4791. Being able to plug in a networking stack, by perhaps implementing an interface and setting a value on the Segment SDK would be nice.

We will likely be taking on a version of this work since we need to use Segment on Windows, so I wanted to file this issue to see if this is something the maintainers would accept as as upstream patch perhaps.

Describe the solution you'd like I'm imagining a protocol the describe the current HTTPClient and perhaps some types to mask over the URLSession*Task types that get stored in various places.

Describe alternatives you've considered

  1. Using Segment without patching the underlying Swift issue risks application crashes so it's an option, but is risky as it could result in a crash app.
  2. We could consider running all Segment code in a different process so that if it's networking code results in a crash it just takes down that process and doesn't kill our application. This is a little challenging since there isn't a standardized way to do this across Darwin, Linux, and Windows, but it's likely possible.

Additional context Add any other context or screenshots about the feature request here.

brianmichel avatar Nov 28 '23 17:11 brianmichel

@brianmichel I think that's a great idea. As a matter of fact we're heading that route with a few other things like storage, user agent data, flush queue (see main), etc. Do you have an ETA for the PR?

bsneed avatar Nov 28 '23 17:11 bsneed

ps. If you have any change sets related to supporting Windows, we'd be interested in that as well. Looks like I'll have to get that running in a VM 💯 :D

bsneed avatar Nov 28 '23 17:11 bsneed

ps. If you have any change sets related to supporting Windows, we'd be interested in that as well. Looks like I'll have to get that running in a VM 💯 :D

Here's my PR for Windows support on our fork https://github.com/thebrowsercompany/analytics-swift/pull/7 there are a couple of tests that I'm trying to fix, but it does compile!

As far as the pluggable network stack I'm probably going to have something by EOW, i'll ping you on it and get your thoughts!

brianmichel avatar Nov 28 '23 20:11 brianmichel

@bsneed here's a rough sketch on the pluggable networking stacks https://github.com/thebrowsercompany/analytics-swift/pull/8. While I thought about putting this in the vendor system, it seemed to most make sense to provide this in the configuration since you want to ensure your Analytics instance is using the right stack from the get-go. Also I tried to map the built-in URLSession to the new protocols so by default the SDK should function exactly how it used to. LMK what you think about this sketch.

brianmichel avatar Nov 29 '23 17:11 brianmichel