Skip to content

Throw an appropriate error from the writer when the channel closed #2744

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

Merged
merged 2 commits into from
Jun 24, 2024

Conversation

FranzBusch
Copy link
Member

Motivation

Currently, when the channel closes and a user tries to write something the writer throws an AlreadyFinished error. This error can also be thrown when calling finish on the writer and then trying to call write again. This makes it hard to distinguish if the thrown error was due to the channel being closed or due to a business logic error in handling the writer.

Modification

This PR finishes the writer with a ChannelError.ioOnClosedChannel if the writer gets finished to due a channel inactive or handler removed.

Result

Users can now distinguish if they did something wrong with the writer or if the channel closed.

# Motivation
Currently, when the channel closes and a user tries to write something the writer throws an `AlreadyFinished` error. This error can also be thrown when calling `finish` on the writer and then trying to call `write` again. This makes it hard to distinguish if the thrown error was due to the channel being closed or due to a business logic error in handling the writer.

# Modification
This PR finishes the writer with a `ChannelError.ioOnClosedChannel` if the writer gets finished to due a channel inactive or handler removed.

# Result
Users can now distinguish if they did something wrong with the writer or if the channel closed.
@FranzBusch FranzBusch requested a review from Lukasa June 17, 2024 10:11
@FranzBusch FranzBusch added the 🔨 semver/patch No public API change. label Jun 17, 2024
Copy link
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

Nice, this looks like a good improvement.

@FranzBusch
Copy link
Member Author

@swift-server-bot test this please

@FranzBusch FranzBusch enabled auto-merge (squash) June 18, 2024 12:15
@FranzBusch
Copy link
Member Author

@swift-server-bot test this please

@FranzBusch
Copy link
Member Author

@swift-server-bot test this please

@FranzBusch FranzBusch merged commit 5d7a999 into apple:main Jun 24, 2024
8 of 9 checks passed
@FranzBusch FranzBusch deleted the fb-async-channel-error branch June 24, 2024 14:11
chkp-aviads added a commit to chkp-aviads/swift-nio that referenced this pull request Jul 21, 2024
* commit 'fc79798d5a150d61361a27ce0c51169b889e23de':
  NIOSendableBox: allow off-loop initialisation iff Value is Sendable (apple#2753)
  Throw an appropriate error from the writer when the channel closed (apple#2744)
  put snippet code inside @available function (apple#2750)
  fix link to NIOFileSystem from NIO index page (apple#2747)
  convert the NIOFileSystem example code to a Snippet (apple#2746)
  Silence warning about missing include in macOS builds (apple#2741)
  Correctly mark 304 as not having a response body (apple#2737)
  Update availability guard (apple#2739)
  Add API for setting last accessed and last modified file times (apple#2735)
  Add a fallback path if renameat2 fails (apple#2733)
  Release file handles back to caller on failure to take ownership (apple#2715)
  Add a version of 'write' for 'ByteBuffer' (apple#2730)
  Imrprove rename error (apple#2731)
  Remove storage indirection for FileSystemError (apple#2726)
  testSimpleMPTCP should not fail for ENOPROTOOPT (apple#2725)
  Fix race in TCPThroughputBenchmark (apple#2724)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants