Repository navigation
Socket.IO Client for Swift 17: native URLSession transport, Swift 6.4, JavaScript client parity - #1527
Open
kaeferfreund wants to merge 401 commits into
Open
Socket.IO Client for Swift 17: native URLSession transport, Swift 6.4, JavaScript client parity#1527kaeferfreund wants to merge 401 commits into
kaeferfreund wants to merge 401 commits into
Conversation
…oot nsp Same shape as the phase-6 fix: SocketTimedEmitterE2ETest connects to the default '/' namespace, so the ping echo + never_ack handlers must live there.
JS reference (socket.io-client/lib/socket.ts) calls destroy() — which
clears `this.subs` (=> active = false) — from three paths:
1. user `socket.disconnect()` (already handled, line 446)
2. server-initiated DISCONNECT (was missing — added)
3. CONNECT_ERROR packet receipt (was missing — added)
Swift only flipped `active = false` in case (1), leaving server-driven
termination paths with `active == true` indefinitely. The CHANGELOG
framing ("Survives engine-close + reconnect cycles") accurately covered
engine-close but glossed over the two server-driven destroy() paths.
Add the flips at the SocketPacket dispatch site, BEFORE invoking
didDisconnect / handleEvent, so user listeners observe `active = false`
when their .disconnect / .error callback fires (matches JS where destroy()
runs before super.emit).
Update the property docstring to enumerate all three termination paths.
Add two regression tests asserting active == false after handlePacket
of a .disconnect packet and a .error (CONNECT_ERROR) packet respectively.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `fired: Bool` field was initialized false and never assigned true; five comments across SocketAckManager.swift and SocketTimedEmitter.swift described an atomic "checks fired -> flips fired = true -> removes entry -> invokes callback" sequence that did not match the code. Atomicity was actually upheld by `timedAcks[id] = nil` on the serial handleQueue — only the first path to remove the entry sees a non-nil result, every other path's lookup returns nil and short-circuits. Replace the field + truthy-guard pattern with `removeValue(forKey:)` on the four call sites (addTimedAck timer body, executeTimedAck, cancelTimedAck, clearTimedAcks). Rewrite the comments to describe the actual mechanism. No behavioral change — all 272 tests still pass, including the Phase 9 race-stress suite (200 iters cancel-vs-timer / cancel-vs-ack / timer-vs-ack). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Document v3-only gating, the socket.recovered flag, pid/offset capture (including the 256-byte cap and oversized-offset drop), CONNECT payload merge precedence, pre-ack replay buffering, and clearRecoveryState() semantics (in-memory-only scope, identity-swap usage, Swift-only timed-ack .disconnected side effect, subclass disconnect() ordering). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This fork has no CI history at all: the upstream workflow exists but never ran, so nothing here has ever been built or tested outside the author's machine. Before taking changes on top of that base we need a baseline run of the untouched code, and a way to re-run it on demand. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
v3 runs on a Node runtime GitHub has already retired; runners warn about it and will eventually stop scheduling it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Engine.IO requires the current transport to be paused before the upgrade packet is sent. This client only waited for the outstanding long-poll and ignored an in-flight POST, so the upgrade packet could overtake it. The server then answers that late POST with HTTP 400 (TRANSPORT_MISMATCH), the response is discarded because `polling` is already false, and the packet is never resent because `postWait` is cleared when the POST is created. The payload is lost without any error surfacing. An app that emits a request with an ack right after connecting sees this as a request that never gets acknowledged; if the lost packet is the namespace CONNECT, no `connect`/auth event ever arrives. The reference implementation waits for both the poll and the write to settle before upgrading (`pause()` in engine.io-client's polling transport, which resolves on `pollComplete` and `drain`). Mirror that: `canSendUpgradePacket` requires a pending upgrade with no outstanding poll and no outstanding POST, and whichever callback settles last carries out the deferred upgrade.
`upgradeTransport()` sets `fastUpgrade` and only then enqueues its noop, so `flushWaitingForPost()` marked the transport as writing while `doRequest` refused to start that request (its guard excludes an upgrading transport). Nothing could clear the flag, so the pause introduced in the previous commit never resolved: neither callback upgraded or restarted the poll, leaving the engine connected without an active transport. Stop attempting polling writes once an upgrade is pending, which is what the existing comment in `upgradeTransport()` already states. The queued packets still leave over the WebSocket via `doFastUpgrade`. Found by an automated correctness review of the previous commit.
A protocol-level script that drives a real Socket.IO 4.8 server the way the engine does, once with the upgrade packet overtaking an in-flight POST and once with the POST drained first. It fails unless the first ordering loses the event (HTTP 400, no ack) and the second delivers it. Node only, so it also runs where Starscream cannot be built, which is why it is a script rather than another XCTest case. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Swift job needs macOS because Starscream does not build elsewhere, so it cannot cover the server side of this fix. The proof script is plain Node and runs on Linux in seconds, which keeps the wire-level contract honest even if the engine.io server changes its behaviour. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The macOS job spawns the fixture server from PATH, so its E2E results silently depended on whichever Node the runner image shipped. Pin it, and move the proof job from 22 to the current release so we learn early if the fixture stops working on it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…efore-upgrade fix(engine): pause the polling transport before upgrading to WebSocket
engine.io v4 servers advertise a `maxPayload` in the handshake and enforce it: a POST above the limit is answered with HTTP 413 and every packet it carried is discarded. The session stays open, so from the application's side the events simply never happened. This client batched the whole queue into one POST regardless. The reference implementation cuts the batch at the limit and leaves the rest for the next POST (`getWritablePackets()` in engine.io-client); mirror that. A packet larger than the limit on its own is still sent, because it cannot be split and that is what the reference client does too. Byte length is what counts, not character count. engine.io v3 is untouched: those servers advertise no limit and use a different payload format. `resetEngine()` now drops whatever is still queued. Until now a POST always drained the queue completely, so a new session could not inherit packets from the old one; capping the batch can leave a remainder, and sending it under a fresh sid would carry ack ids that server never issued. Evidence: `Fixtures/max-payload-proof.mjs` shows a real Socket.IO 4.8 server answering the oversized batch with 413 and delivering nothing, while the split batch gets through and both events are acked. The batching rule itself is pinned by eight SocketEngineTest cases, and an E2E test asserts the advertised limit actually reaches the engine over a real handshake. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`SocketIOClient.manager` is weak, so the manager created inside the helper was deallocated the moment it returned: the engine could not be reached and no emit ever left. Both failures in the previous run came from that, not from the batching change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fix(engine): keep polling POSTs under the server's maxPayload
… them JS holds an emit made while the socket is not connected in `sendBuffer` and writes it on the next CONNECT. This client dropped it and reported "Tried emitting when not connected" — the event was gone, and the only way to not lose data was for every caller to implement its own queue. Three details decide whether the buffer is coherent rather than merely present: - The outgoing any-listeners fire when the packet is actually written, not when it is queued. That is where JS fires them (`emitBuffered()`), and it keeps the existing promise that they only ever see real sends. - An ack whose packet is still buffered survives the disconnect. JS `_clearAcks` skips exactly those: the packet never reached the server, so the ack is still owed once the socket is back. Failing it with `.disconnected` while the packet is still queued for delivery would be a lie. - A timed-out ack drops its packet from the buffer, as JS does in `_registerAckCallback`, so a reconnect does not deliver an emit whose caller already saw it fail. Ack responses (`emitAck`) are left as they were: JS does not buffer them either, and replaying one into a session the server no longer knows is meaningless. This is a breaking change for callers that used the `.error` as a signal, and it is in the CHANGELOG as one. The buffer has no upper bound, matching JS. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…veryState The previous commit edited the wrong one of the two identical `clearTimedAcks` call sites, so a disconnect still failed the acks of packets that had not been sent, and CI caught it. The two paths want opposite things. A disconnect keeps buffered emits, so their acks are still owed. `clearRecoveryState()` is the identity-swap path: there the buffer is dropped, because delivering the previous user's queued events into the successor session would leak them across identities — and their acks are failed along with them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fix(client): buffer emits made while disconnected instead of dropping them
Nine scenarios taken verbatim from socket.io-client 4.8.3's test suite, run against a fixture server extended to match `test/support/server.ts` (the `/no` and `/with-data` middleware namespaces, `/foo` and `/asd`, and the `echo` handler). The point is to stop deciding what "parity" means by reading the source. These tests encode the reference client's own expectations, so where this client differs, CI says so instead of us guessing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A reconnect fires .connect again on a handler that is still registered, and XCTest treats the second fulfill as an API violation — which aborts the whole test process, so the run reported one failure and skipped the rest. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`_engineDidOpen` re-sent CONNECT for every socket still in `.connecting`. A socket the server rejected stays in that state, so the client asked again on every engine open — a server refusing an expired token was re-asked forever. The rejoin now requires `socket.active`. That flag already goes false on CONNECT_ERROR and on a user disconnect, with a comment stating the manager would not auto-rejoin; nothing was reading it. JS destroys the socket in both cases and waits for an explicit `connect()`. Found by porting the JS client's own scenario, not by reading the source. That is the point of PARITY.md, added here: all 116 scenarios of socket.io-client 4.8.3's test suite with an honest status each, including the 42 that are still unassessed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The manager now only rejoins active sockets, and this test reaches .connecting through setTestStatus instead of connect(), which is what sets the flag. A setup artifact, not a behaviour change: production has no path to .connecting that skips connect(). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
test: measure parity against the JS client's own test suite
`testSocketIdChangesOnReconnection` went red on master right after it was green on the branch. The cause is the test, not the client: it dropped the engine with `engine.disconnect(reason:)`, which is a clean client-initiated shutdown and does not reliably lead to a reconnect. The JS tests use `socket.io.engine.close()`, but the drop they mean to simulate is an unexpected one, which here is `/admin/kill-transport` — what StateRecoveryE2ETest already uses. `testNoRejoinAfterAMiddlewareFailure` had the worse version of the same problem: it asserts that something does *not* happen, so an engine that never came back would have made it pass for the wrong reason. It now connects a second socket on an accepted namespace, kills that transport, and asserts that socket reconnected before checking the refused one was not rejoined. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nnects test(e2e): force reconnects from the server side, not the client
Nine more scenarios from socket.io-client 4.8.3: namespace joins after the connection is up and after it was closed, the CONNECT-frame count when the same namespace is fetched twice, socket ids on a custom namespace, a falsy payload surviving the round trip, an IPv6 host surviving URL construction, and the two that assert no error is reported — for a disconnect during open, and for a transport dropping under an established connection. The last one matters beyond parity: reporting a dropped transport as an error is what makes an app tell the user the server is unreachable while it is merely reconnecting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Unassessed drops from 42 to 23. Nine scenarios ported, five reclassified as n/a with a reason (the url.ts cases resolve against the browser's `location`, which Swift has no equivalent of), and five turned out to be gaps worth naming: no per-emit compress chain, no manager-level events, and Manager.socket() not reopening a cached inactive socket. What is left is almost entirely reconnection semantics. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
test: second batch of ported JS scenarios (42 → 23 unassessed)
Thirteen scenarios from socket.io-client 4.8.3, the reconnection group: manual and automatic reconnects, a disconnect immediately followed by a connect, stopping the loop on one namespace while another keeps going, attempt budgets against a port nothing listens on, query-parameter encoding, binary ordering, and the v2-server detection (a CONNECT without a payload). The fixture server gains `/valid`, `/abc` (emits the handshake) and the `abuff1`/`abuff2` ordering handlers, mirroring the JS support server. Two of these are written to the JS contract knowing this client may not meet it: the v2 detection, and exactly one `.disconnect` for a `disconnect(); connect()` bounce. CI decides; a failure there is a finding. Implementation by Muse Spark 1.3 via opencode, from a written brief; review, verification and merge decisions by the orchestrating agent. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
On a `.three` manager a CONNECT packet whose payload carries no `sid` used to connect the socket. The JS client recognises that shape as a Socket.IO v2 server, which a v3 client cannot talk to, and fires `connect_error` with a message pointing at the migration guide. Now this client fires `.error` with the same message and does not connect. `.two` managers are untouched: the v2 protocol carries no sid, and a test pins that exemption. Found by porting the JS client's own scenario. Nothing else in the suite injected a bare CONNECT, so no existing expectation changes. Also corrects the one ported test that had encoded a wrong assumption from the brief: Swift's `.reconnect` fires when reconnection *starts* (`setReconnecting`), not on success as JS's `reconnect` does. A successful reconnect here is another `.connect`, and the test now waits for that. Implementation by Muse Spark 1.3 via opencode, from a written brief; review, verification and merge decisions by the orchestrating agent. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Move release/parity internals one level down into Documentation/, lead with 30s -> 5min -> deep-dive structure, document modern ack/auth/volatile/recovery/transport APIs, drop Carthage/CocoaPods install paths from primary README.
Release 17.0.0: finalize documentation and validate SPM installation
- Remove Cartfile, podspec, Xcode project and workspace metadata - Rename CI framework-distribution job to native package platform builds - Point docs, changelog and parity notes at SPM as the only install method - Delete completed review-audit notes superseded by PARITY.md
Split all 24 substantive README sections into focused application/developer guides and retain the 43 original README anchors. Add documentation, API-source, scripts, contributor, release and evidence indexes. Preserve old Usage Docs byte-for-byte in a clearly marked archive and link already removed reports/plans to their original commit. Correct stale release/parity references, add issue/PR templates and EditorConfig, and ignore local Python outputs. Run existing Swift jobs on feature-branch pushes without removing any checks; add an offline documentation checker with 17 regressions and compile the marked README example as an independent Swift Package consumer. Source, Tests, Package.swift, LICENSE, CHANGELOG, contract/inventory data, existing scripts and all retained review evidence are unchanged. Verified the complete Git tree against local file hashes plus preserved baseline tree/blob IDs. No retired package manager or framework metadata restored.
Add a focused project header, navigation and branch-labeled badges. Document concrete differences from the inspected official Swift client, including migration trade-offs and the distinction from JavaScript parity. Align the getting-started guide and retain existing README anchors and its CI-compiled quick-start example. No library, test or license changes.
Recenter README and WhyThisFork on production bugs, the observed upstream and Starscream maintenance gaps, JavaScript behavior alignment, missing client features, and AI-assisted development backed by reproducible checks. Record the 2026-09-19 maintenance review with pinned commits and releases, include the reconnect semantics example, and preserve migration guidance, README anchors, the compiled quick start, and all code and license files.
Add the prepared swallow SVG as a local documentation asset and display it below the README title. Name and link daltoniam/Starscream consistently. Preserve all existing examples, anchors, source code and license files.
Restore FinalParityAssertions doc, clarify physical-device scope and deferred council review in Release17, drop stale RemainingClientParity references from backlog CSV, fix native distribution script macOS destination and scheme listing.
Put socketio/socket.io-client-swift and daltoniam/Starscream in matching left-hand README table cells with separate linked names and descriptions. Retain the locally restored final assertion report at its stable path, label its historical scope without changing its body, and reconcile the evidence and archive indexes. Preserve all changes from bf62914, including the release clarifications, backlog cleanup and native build fix. No source, test, script, image or license changes.
Pin all checkout, setup-node and upload-artifact v7 references to verified upstream commit SHAs without changing job settings or permissions. Exclude inline-code HTML from anchor collection while retaining heading text and source line numbers. Remove exponential backtracking from the Markdown destination pattern, preserving existing link and title handling. Add seven focused regressions, including a subprocess deadline for long unterminated destinations, and document all Python validator/test functions. All 24 documentation tests pass locally. Library code, README, assets, license, parity evidence and the existing native-build fix are unchanged.
SPM-only distribution
Encode connect parameter keys and values with encodeURIComponent semantics so Foundation accepts special characters in percentEncodedQuery. Cancel per-socket connection deadlines after success, disconnection or replacement so stale timers cannot interrupt reconnection. Derive defaultSocket's namespace from the manager URL path while retaining an independently configured transport endpoint. Support client-certificate credentials for polling, WebSocket and upgrades, scoped to the configured TLS origin and retaining server trust validation. Add regression tests, mutual TLS fixtures and usage documentation. Linux support is outside this change. Validation: Swift syntax parsing, strict parity-contract checks, 132 query encoding/round-trip cases and Node mTLS fixture checks passed locally. Swift XCTest execution remains pending on macOS; the local Linux build is blocked by existing Apple platform dependencies. Fixes socketio#1421 Fixes socketio#887 Fixes socketio#1297 Fixes socketio#857 Fixes socketio#936 Fixes socketio#1157
Add requestTimeout in seconds for the initial handshake, subsequent polling GETs, POSTs and retiring-session close requests. Apply explicit values to both request idle and session resource timeouts so a slow response cannot extend the configured total deadline by trickling bytes. Reject nonpositive/nonfinite values and invalid dictionary types before opening a transport. Preserve native defaults when omitted and leave WebSocket requests unchanged. Document seconds versus JavaScript's milliseconds and the native-default/disabled-timeout differences. Add request propagation and validation tests plus real HTTP fixtures for stalled handshake/GET/POST and trickling bounded/unbounded responses. Validation: Swift syntax parse, strict parity contracts, diff checks and isolated actual-source Foundation request configuration/timeout smoke test passed. Full XCTest execution is pending on macOS; Linux compilation is blocked by existing Apple dependencies. Fixes socketio#681
Align legacy URL assertions with encodeURIComponent, include the configuration-error prefix and model WSS client authentication as an HTTPS protection space. Production code and security checks remain unchanged. Refs socketio#1421, socketio#681, socketio#857, socketio#936, socketio#1157
docs: preserve upstream issue triage report and evaluation evidence
- Canonicalize client-cert challenge hosts via IDNA (case, Unicode/Punycode, trailing dot, IPv6 scopes) - Reset clientCertificate and requestTimeout when a reused engine gets new config - Emit disconnect on namespace connect timeout, keep buffered acks, let listener reconnects supersede the deadline
…timeout fix: harden mTLS credential host matching and per-socket connect timeout cleanup
Author
Member
|
Thank you! I'll review over the week |
|
Hi @kaeferfreund @nuclearace, thanks for this work! I really needed for an iOS app talking to a Socket.IO 4.8 server. One request before this lands upstream: could Package.swift declare swift-tools-version:6.2 instead of 6.4? |
Author
|
Hi @thekhubaib, thanks for trying it out and for the clear request. That is a fair point about Xcode 26.x and Intel Macs. I pushed a test branch that declares Only the tools version changed ( It would be great if you could take you up on the offer and check on Xcode 26.3: |
|
Thanks @kaeferfreund Tested compat/swift-tools-6.2 @ 803cc69 on Xcode 26.3 (17C529), Swift 6.2.4, Intel Mac (macOS 15): So the sources compile and pass unchanged on 6.2. It would be great to see the 6.2 manifest in the upstream PR. That way the official client keeps working for everyone on Xcode 26.x. Happy to re-test anything else. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR proposes the work from kaeferfreund/socket.io-client-swift (released there as 17.0.0 → 17.1.0) for upstream. It is a major, breaking release: 401 commits on top of
masterat 42da871, developed in 28 reviewed PRs on the fork.The aim was to fix production problems we hit with 16.x and to make the Swift client behave like the JavaScript client (
socket.io-client/engine.io-client). Behavior is checked against a pinned JS reference. Remaining API and platform differences are documented.Highlights
Transport and toolchain
URLSessionWebSocketTaskplusURLSessionlong-polling. There are no third-party Swift dependencies.tryAllTransportsandrememberUpgrade. When a transport fails to open, the client can try the next configured one.Client behavior aligned with JavaScript
reconnect,reconnect_attempt,reconnect_errorandreconnect_failed.asyncacks and per-socket timeout/retry overrides.connect_errorclient event and a per-socket connection timeout.auth.maxPayload.t=cache-buster is added to polling requests.SocketPacketErrorinstead of sending a different packet (2[]).Dateencodes liketoISOString().NaNandInfinityencode asnull.Upstream issues addressed
<,>,\and backtick inconnectParams. Keys and values are now encoded withencodeURIComponentsemantics.defaultSocketnow derives its namespace from the manager URL path..clientCertificate(URLCredential)for mutual TLS across polling, WebSocket and upgrades, scoped to the configured origin (IDNA/Punycode-aware host matching)..requestTimeout(Double)for polling handshake, GET and POST requests.Compatibility and documentation
This is a breaking change for 16.x users:
SocketIOVersion/.version(...).compress,.selfSigned(...),.enableSOCKSProxy(...)and.useCustomEngine(...)withCredentials(true).Sendable. Use them on theirhandleQueue..connectParamsreplaces it (as in JS).Migration and reference docs:
Documentation/JavaScriptParityContracts.json(source of truth). The CSV inventory is derived from the JSON and checked by a gate.Deliberately unsupported:
Dictionaryhas no insertion order), and slashes are escaped in JSON output.Before merging upstream, some fork-specific content needs changes:
Documentation/is written in German.I'm happy to adapt or drop these, or to split this PR into smaller pieces if the maintainers prefer that.
Validation
Everything runs on macOS CI (
xcode-27runner). The latest run on this exact head (2af783e) passed: https://github.com/kaeferfreund/socket.io-client-swift/actions/runs/35651443320The CI pipeline covers:
swift buildwithout third-party dependencies, plus an independent Swift Package consumer.Not covered: physical iOS/watchOS runtime checks (background/foreground, suspend, network changes). SDK compilation does not certify device behavior.
Preservation
PARITY.mdand the gated inventory. Evidence is kept underDocumentation/ReviewEvidence/.