Skip to content

stream: improving error handling#18438

Closed
mafintosh wants to merge 15 commits intonodejs:masterfrom
mafintosh:streams-error-handling-fix
Closed

stream: improving error handling#18438
mafintosh wants to merge 15 commits intonodejs:masterfrom
mafintosh:streams-error-handling-fix

Conversation

@mafintosh
Copy link
Copy Markdown
Member

@mafintosh mafintosh commented Jan 29, 2018

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

stream

This PR improves error handling for streams in a few ways.

  1. It ensures that no user defined methods (_read, _write, ...) are run after .destroy has been called.
  2. It introduces an explicit error to tell the user if they are write to write, etc to the stream after it has been destroyed.
  3. It makes streams always emit close as the last thing after they have been destroyed
  4. Changes the default _destroy to not gracefully end streams.

Especially 4. makes it easier for userland modules to handle stream errors as they don't appear as graceful closes anymore. Seen issues being with end-of-stream/pump because of this.

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fs Issues and PRs related to the fs subsystem / file system. http2 Issues or PRs related to the http2 subsystem. semver-major PRs that contain breaking changes and should be released in the next major version. stream Issues and PRs related to the stream subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.