Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pluggable Networking Stacks #279

Closed
brianmichel opened this issue Nov 28, 2023 · 4 comments · Fixed by #344
Closed

Pluggable Networking Stacks #279

brianmichel opened this issue Nov 28, 2023 · 4 comments · Fixed by #344
Assignees
Labels

Comments

@brianmichel
Copy link
Contributor

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, swiftlang/swift-corelibs-foundation#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
Copy link
Contributor

bsneed commented Nov 28, 2023

@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
Copy link
Contributor

bsneed commented Nov 28, 2023

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
Copy link
Contributor Author

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 thebrowsercompany#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
Copy link
Contributor Author

@bsneed here's a rough sketch on the pluggable networking stacks thebrowsercompany#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants