search

segmentio / analytics-swift

The hassle-free way to add Segment analytics to your Swift app (iOS/tvOS/watchOS/macOS/Linux).
MIT License
94 stars 81 forks source link

Pluggable Networking Stacks #279

Closed brianmichel closed 2 months ago

brianmichel commented 8 months ago

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.

bsneed commented 8 months ago

@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 commented 8 months ago

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

brianmichel commented 8 months ago

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 commented 8 months ago

@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.