Skip to content
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

Inaccurate end event semantics #59

Closed
clue opened this issue Nov 1, 2016 · 0 comments · Fixed by #70
Closed

Inaccurate end event semantics #59

clue opened this issue Nov 1, 2016 · 0 comments · Fixed by #70

Comments

@clue
Copy link
Member

clue commented Nov 1, 2016

The documentation currently suggests the following behavior:

  • An error event will be emitted when the stream/connection encounters an error (and will then continue to close the connection)
  • A close event will be emitted when a stream/connection closes (regardless of whether an error occurred or the stream simply ended)
  • An end event will be emitted when the stream/connection reached EOF (end of file or other side closes the connection)

However, the actual stream implementations do not accurately follow this behavior:

  • Stream::close() should only emit a close event, no end event.
  • Stream::end() is not to be confused with the end event. It should simply call close() once the outgoing buffer has been flushed
  • When an error occurs, the Stream should only emit an error and then a close event.
  • The Stream should only emit an end and then a close event if the sending side closes (i.e. an empty read event occurs)
  • WritableStream does not implement ReadableStreamInterface, so it can never emit a data event and hence no end event
  • Possibly many others throughout React's ecosystem

I think we should emphasize these event semantics. This is actually documented already, however we should probably be a bit more verbose and explicit about this.

I'm currently looking into actually fixing all of the above occurrences. This will result in a BC break for anybody who relies on the current buggy behavior - this will not be a BC break for anybody following the documentation.

This means we should probably target a v0.5.0 release here and provide some upgrade instructions. Many library will simply need to upgrade their version requirements, however some may also have to check their event semantics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant