Repository navigation
Add close_output for half-close WebSocket support - #145
Merged
Merged
Conversation
- Implement close_output/2 to send close frame while continuing to receive - Add output_closed_loop to process incoming frames after output is closed - Handle callback replies gracefully when output is closed (ignore sends) - Add error messages for closing and output_closed connection states Implements #140
Add catch-all {:error, reason} clause to handle errors like
FrameEncodeError when an invalid close code is provided.
This prevents process crashes and returns the error to the caller,
matching the pattern used in sync_send/5.
Reduce code duplication by using a single unified loop with an output_closed_reason field in state to track the half-close state. This removes ~160 lines of duplicated code including: - output_closed_loop (duplicate of websocket_loop) - handle_frame_output_closed (duplicate of handle_frame) - handle_fragment_output_closed (duplicate of handle_fragment) - common_handle_output_closed (duplicate of common_handle) The unified websocket_loop now handles both normal and output_closed states by checking the output_closed_reason field.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 9 potential issues.
Bugbot Autofix prepared fixes for all 9 issues found in the latest run.
- ✅ Fixed: Reconnect keeps half-close flag
- reconnect/3 now clears output_closed_reason along with the buffer and fragment, so a new socket is not treated as already half-closed.
- ✅ Fixed: Proxy TLS omits target hostname
- The CONNECT tunnel upgrade now sets server_name_indication to the target host unless the caller already supplied it, so verify_peer checks that hostname.
- ✅ Fixed: close_output return mismatches documented API
- close_output replies :ok from both the success and already-closed paths, matching the spec and send_frame.
- ✅ Fixed: Connecting state ignores close_output
- open_loop now answers $websockex_close_output immediately with NotConnectedError connection_state :opening, so the call does not time out and half-close the later socket.
- ✅ Fixed: Half-close reports remote disconnect
- A close frame received after close_output completes the handshake with the stored local reason instead of handle_remote_close.
- ✅ Fixed: Half-close drops required pong replies
- After a local close, data-frame replies are still dropped but pong replies are sent until a close is received.
- ✅ Fixed: Proxy credentials are never sent
- CONNECT now sends Proxy-Authorization when username/password are set, and proxy URLs keep decoded userinfo.
- ✅ Fixed: Non-TLS connections ignore proxy
- ws:// connections with :proxy use the same HTTP CONNECT tunnel and return the tunneled TCP socket without a TLS upgrade.
- ✅ Fixed: Proxy path drops socket options
- The proxy TCP connect uses socket_connection_options/1, so caller socket_options such as a bind address are applied.
You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit efc4d44. Configure here.
The rebased half-close code failed format --check-formatted on an extra indent in system_continue/3 and a long NotConnectedError reply. Co-authored-by: Dominic Letz <dominicletz@users.noreply.github.com>
Clear the half-close flag on reconnect, return :ok from close_output, reject close_output while opening, and report the local close reason when the peer completes a half-close. Still send required pongs after a local close.
A ping made the test server reply with a pong that TestClient does not handle. Send a binary frame and wait for the server to echo it instead. Co-authored-by: Dominic Letz <dominicletz@users.noreply.github.com>
cursor
Bot
force-pushed
the
cursor/rebase-close-output-141-b57e
branch
from
October 29, 2026 21:56
9aea3df to
8dfc482
Compare
Cowboy can finish the close handshake before send_frame, so the client is already :closing. Stall the test server first so the send is rejected with :output_closed. Co-authored-by: Dominic Letz <dominicletz@users.noreply.github.com>
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
Adds
WebSockex.close_output/1,2,3so a client can send a close frame and keep reading until the peer finishes the handshake. This is the half-close pattern used by .NETClientWebSocket.CloseOutputAsyncand by Azure Relay Hybrid Connections when a relayed TCP connection shuts down its write side. Rebased onto currentmasterfrom #141. The HTTP CONNECT proxy work that had landed on this branch is now #148 and is not in this diff.After
close_output:send_framereturns%WebSockex.NotConnectedError{connection_state: :output_closed}handle_disconnectreceives the local close reasonclose_outputwhile the socket is still opening returnsconnection_state: :openingTests
Covers send rejection, a custom close code, handshake completion, the local close reason, sending again after reconnect, rejection while opening, and a pong still going out after
close_output.On Elixir 1.18.4 / OTP 27,
mix format --check-formattedandmix compile --warnings-as-errorspass.mix testhas the same pre-existing failure asmaster:open_socket rejects a hostname mismatch with supplied cacertsexpects{:tls_alert, {:bad_certificate, _}}, and OTP 27 reports{:tls_alert, {:handshake_failure, _}}. Theclose_outputtests pass.Note
Medium Risk
Half-close changes core connection lifecycle and send paths; HTTP CONNECT proxy adds a new TLS setup path for wss without dedicated tests in this PR.
Reviewed by Cursor Bugbot for commit efc4d44. Configure here.