Skip to content

Connection#close_write closes the connection on failure.#25

Merged
ioquatix merged 1 commit into
mainfrom
close_write-rescue-closed
May 28, 2026
Merged

Connection#close_write closes the connection on failure.#25
ioquatix merged 1 commit into
mainfrom
close_write-rescue-closed

Conversation

@ioquatix
Copy link
Copy Markdown
Member

@ioquatix ioquatix commented Apr 18, 2026

Possible fix for #23.

Types of Changes

  • Bug fix.
  • New feature.

Contribution

@ioquatix ioquatix force-pushed the close_write-rescue-closed branch from 264b36d to 03a98a6 Compare April 18, 2026 12:02
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes issue #23 where Connection#close_write would propagate exceptions (e.g., Protocol::HTTP2::ProtocolError: Cannot send data in state: closed) when the underlying transport was already closed. The fix rescues those errors and transitions the connection to :closed state, matching the resilient behavior of close!.

Changes:

  • close_write now rescues errors raised during send_close and marks the connection as closed.
  • Added tests covering close_write on a closed stream (with and without error), and that successful close_write leaves the connection still readable/not closed.
  • Added an Unreleased section to releases.md describing the fix.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
lib/protocol/websocket/connection.rb Rescue exceptions in close_write and mark connection as closed.
test/protocol/websocket/connection.rb New tests for close_write behavior on closed/open streams.
releases.md Document the close_write resilience fix under Unreleased.

@ioquatix ioquatix merged commit 146d493 into main May 28, 2026
37 of 43 checks passed
@ioquatix ioquatix deleted the close_write-rescue-closed branch May 28, 2026 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants