-
Notifications
You must be signed in to change notification settings - Fork 842
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
Support socket.io 3 + starscream 4 #1309
Conversation
445488a
to
6cd8f79
Compare
If anyone wants to test things out. I have published |
Pushed |
@darrachequesne just so you're aware, this is coming up soon |
@nuclearace like, already? That's awesome! Please ping me if you encounter any issue with the migration. |
@darrachequesne I do have one question regarding the encoding of binary engine messages. Documentation stats that
gets encoded to It would match the breakdown given below the string in the documentation, where it only mentions
Might want to update that if so. Also I noticed the change log mentions |
That's correct 👍 I've updated the protocol documentation: socketio/engine.io-protocol@567231e
Yes again. In the documentation, the WebSocket transport deals with individual "packets", while the HTTP long-polling transport deals with "payloads" (concatenated packets) |
Hey guys, thanks for the great work on progressing this! |
@nuclearace hi Erik, I hope you are fine. Is there something I can do to help on this? For other readers, please note that since version 3.1.0, the Socket.IO server now supports v2 clients: const io = require("socket.io")({
allowEIO3: true // false by default
}); Which should help, until the work on the swift client is completed. |
@darrachequesne Yes I plan to get back to this this week. Apologies all. I had several family members struck by covid after Thanksgiving continuing through Christmas. All survived with varying levels of lasting issues. This caused friction with an also busy release time at work. The remaining work is to make this backwards compatible with v2, where the biggest hurdle will just be switching ping mechanisms. But this shouldn't be too hard. |
@darrachequesne Unfortunately I just discovered that the swift client was not sending the Edit: If they manually send the EIO through connectParams, that should work. |
@darrachequesne I'm running into some weird behavior on 3.1.0 when using Server:
Client:
|
@darrachequesne it only seems to happen when there's an upgrade happening. Going straight to WS seems to handle ping correctly. Also it seems to occasionally work fine, could there be some race in the upgrade code on the server side? Edit: Responding to the ping the server sends just causes a transport error... so something weird is going on
|
|
There was two issues with this behavior: - v3 clients (with allowEIO3: true) were also receiving a "ping" after a successful upgrade, which is incorrect (in v3, it's the client that sends the "ping", and the server answers with a "pong") - the ping timer is not reset after upgrade on the client-side, so an upgrade which took longer than the `pingTimeout` duration could lead to a "ping timeout" error on the client-side I think the latter issue is present since the initial implementation. Related: socketio/socket.io-client-swift#1309 (comment)
|
Thanks for pointing out. I’ll fix that. It would be good to use the typed versions though, the NSdict one was mostly just for compatibility with objc |
There was two issues with this behavior: - v3 clients (with allowEIO3: true) were also receiving a "ping" after a successful upgrade, which is incorrect (in v3, it's the client that sends the "ping", and the server answers with a "pong") - the ping timer is not reset after upgrade on the client-side, so an upgrade which took longer than the `pingTimeout` duration could lead to a "ping timeout" error on the client-side I think the latter issue is present since the initial implementation. Related: socketio/socket.io-client-swift#1309 (comment) Backported from ff2b8ab
No description provided.