Conversation
- Rejoin channel immediately after join fails - Throw error when pushing unjoined channel - Leave channel on join timeout
There was a problem hiding this comment.
Pull request overview
This PR tightens Phoenix socket/channel lifecycle behavior around connect/disconnect/reconnect edge-cases, and improves protocol correctness during join/rejoin flows.
Changes:
- Add stricter channel behavior (throw when sending/requesting before
join(); sendphx_leaveafter join timeout; ignore stale close/error messages while rejoining). - Improve socket resilience (reconnect when the incoming message stream finishes; handle
connect()during client-driven closing; avoid stale close affecting new connections; fix reconnect backoff indexing). - Update WebSocket creation to forward
MakeWebSocketparameters and bumpwebsocket-appleto 4.1.1.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/PhoenixTests/PhoenixSocketTests.swift | Adds coverage for connect-while-closing and reconnect when the message stream ends unexpectedly. |
| Tests/PhoenixTests/PhoenixChannelTests.swift | Adds/adjusts join/rejoin tests, validates pre-join send/request throws, and verifies leave-before-rejoin on join timeout + stale-close handling. |
| Tests/PhoenixTests/MessageTests.swift | Adds regression test for decoding malformed/short messages. |
| Sources/Phoenix/Socket.swift | Updates system socket factory to forward options/callbacks and propagate WebSocket id. |
| Sources/Phoenix/PhoenixSocket.swift | Refines connection state transitions, push preparation, stream-finish handling, and reconnect delay calculation. |
| Sources/Phoenix/PhoenixError.swift | Introduces channelNotJoined error case. |
| Sources/Phoenix/PhoenixChannel.swift | Enforces join-before-send/request, tracks join refs for stale message filtering, and sends leave after join timeout. |
| Sources/Phoenix/Message.swift | Adds array-length validation during decode to avoid out-of-bounds access. |
| Package.swift | Bumps websocket-apple dependency constraint to 4.1.1. |
| Package.resolved | Updates resolved websocket-apple version/revision accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let push = Push( | ||
| topic: topic, | ||
| event: .leave, | ||
| timeout: Date(timeIntervalSinceNow: timeout) | ||
| ) | ||
|
|
||
| push.prepareToSend(ref: await socket.makeRef(), joinRef: joinRef) | ||
| try? await socket.send(push) |
There was a problem hiding this comment.
sendLeaveAfterJoinTimeout awaits socket.send(push), which waits until the leave push is actually sent (and can itself take up to the socket/join timeout). This can extend the overall join() call beyond the intended timeout in cases where sending is blocked. Consider firing the leave push in a detached task (best-effort) or adding a non-blocking send path so the original join timeout returns promptly while still attempting to notify the server.
| let push = Push( | |
| topic: topic, | |
| event: .leave, | |
| timeout: Date(timeIntervalSinceNow: timeout) | |
| ) | |
| push.prepareToSend(ref: await socket.makeRef(), joinRef: joinRef) | |
| try? await socket.send(push) | |
| Task.detached { [socket, topic] in | |
| let push = Push( | |
| topic: topic, | |
| event: .leave, | |
| timeout: Date(timeIntervalSinceNow: timeout) | |
| ) | |
| push.prepareToSend(ref: await socket.makeRef(), joinRef: joinRef) | |
| try? await socket.send(push) | |
| } |
|
|
||
| private struct NotReadyToJoinError: Error {} | ||
|
|
||
| private struct JoinTimeOutError: Error { |
There was a problem hiding this comment.
JoinTimeOutError uses inconsistent capitalization compared to TimeoutError ("TimeOut" vs "Timeout"). Renaming to JoinTimeoutError would better match Swift naming conventions and the existing type name.
| private struct JoinTimeOutError: Error { | |
| private struct JoinTimeoutError: Error { |
MakeWebSocketto createdWebSocket