ably-cocoa
ably-cocoa copied to clipboard
ably
Basic NSURLSessionWebSocketTask functionality
Because this will replace core/critical functionality, it is probably a good idea to introduce it gradually via something like clientOptions.useNativeWebSocket and explicitly ask some clients to give it a try. WDYT @lawrence-forooghian @QuintinWillison
I'll probably need to take some time to get some understanding of WebSockets in general (perhaps reading the RFC) before I can give this a proper review. I'll see if I can find some time this week for that, but can't guarantee that I'll be able to.
Because this will replace core/critical functionality, it is probably a good idea to introduce it gradually via something like
clientOptions.useNativeWebSocketand explicitly ask some clients to give it a try. WDYT @lawrence-forooghian @QuintinWillison
It's definitely worth thinking about ways to get confidence in this change, yeah.
I don't think that adding a client option to change core runtime behaviour is going to do anything to explain this to time poor customers, nor drive them to use it. My preference would be to use the tools already available to us to make this change happen (versioning). So, perhaps, we should be discussing this as part of scope for version 2 of this SDK? (i.e. a major version bump) -- this should count as an enhancement from a customer's perspective, as well as from ours as maintainers. FYI, @mikelee638
@QuintinWillison:
So, perhaps, we should be discussing this as part of scope for version 2 of this SDK? (i.e. a
majorversion bump) -- this should count as an enhancement from a customer's perspective, as well as from ours as maintainers. FYI, @mikelee638
I'm not sure I understand how versioning would help us here – this change is meant to be a backwards-compatible enhancement so it wouldn't warrant a major version bump. If we did want to use versioning to provide customers with a way to opt in to this new code (which, as you say, I'm not sure many would be interested in doing), then it would probably make more sense to use a pre-release version.
Thanks for your comments, @lawrence-forooghian. You're right regarding versioning, though perhaps it might be a version 2 scope change if we're going to significantly increment minimum OS runtime version(s). 🤔 You also commented:
I also think that making the change that's proposed here makes sense mainly if we’re intending to drop support for iOS < 13 at some point. If not, then we're just going to end up having to maintain two WebSocket implementations instead of one.
Maintaining two WebSocket implementations is not ideal, both in terms of maintainability of the codebase and how it is tested. So my preference would be to drop support for the older iOS runtimes as soon as we can.
Taking another look at this, in the context of @umair-ably’s RFC for dropping support for iOS < 14. @maratal, do you remember whether your work here concluded that NSURLSessionWebSocketTask would be an adequate replacement for SocketRocket? Also, I noticed that the Network framework provides WebSocket functionality through NWProtocolWebSocket, also introduced in iOS 13; do you know if/how this differs to NSURLSessionWebSocketTask?
I noticed that the
Networkframework provides WebSocket functionality throughNWProtocolWebSocket, also introduced in iOS 13; do you know if/how this differs toNSURLSessionWebSocketTask?
Looks like it's swift only. And NSURLSessionWebSocketTask is an objective-c wrapper around it. That's my conclusion from the first glance.
Ah, OK. There is also a C version, though. Apple’s guidance is to favour the Network framework:
Unless you have a specific reason to use URLSession, use Network framework for new WebSocket code.
Ah, OK. There is also a C version, though. Apple’s guidance is to favour the Network framework:
Unless you have a specific reason to use URLSession, use Network framework for new WebSocket code.
That's interesting, will check in depth the effort needed to move to c-version, thanks @lawrence-forooghian
If we drop support for iOS < 14, as is being proposed, we can then start using Swift inside the codebase anyway