Skip to content

Throw the first seen stream error in writeToStream#33

Merged
reconbot merged 8 commits intobustle:masterfrom
dotboris:write-to-stream-errors
Mar 31, 2019
Merged

Throw the first seen stream error in writeToStream#33
reconbot merged 8 commits intobustle:masterfrom
dotboris:write-to-stream-errors

Conversation

@dotboris
Copy link
Copy Markdown
Contributor

Fixes #30

This makes it so that if the stream we're writing to emit's an error, we interrupt reading the iterator and throw an error.

This makes a somewhat reasonable assumption about stream. It assumes that we're pretty much done with the stream once it emits it's first error.

Technically speaking this is not 100% true when it comes to node since a writable stream can still work and process data even if it has already emitted an error. Also, node streams can emit as many errors as they want.

I think that this is still a reasonable approach. Fail-fast is generally speaking a good idea. On top of what node streams will crash the process on their first unhandled error which is similar behaviour.

@dotboris
Copy link
Copy Markdown
Contributor Author

I'm currently testing this with the example I posted in #30 and it doesn't really work as it should.

@reconbot
Copy link
Copy Markdown
Contributor

ha! beat me to it! What do you think of our two approaches? #34

@dotboris
Copy link
Copy Markdown
Contributor Author

@reconbot I like your approach better. I solves some of the problems that I've been hitting when testing with the example script.

@reconbot
Copy link
Copy Markdown
Contributor

I like your tests better than mine, but I like my approach better because we'll never wait for a drain event after an error has occurred. If you want to interoperate my fix and docs with your tests I'd happily merge.

@dotboris
Copy link
Copy Markdown
Contributor Author

@reconbot I'll do exactly that

@dotboris
Copy link
Copy Markdown
Contributor Author

@reconbot here you go

  • I took your documentation and Promise.race approach
  • I kept my tests
  • I removed the listener before doing the last error check
  • I added an error check at every instance of the loop so that we stop reading the iterator as soon as possible

Copy link
Copy Markdown
Contributor

@reconbot reconbot left a comment

Choose a reason for hiding this comment

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

👏

@reconbot reconbot merged commit 7b91eb4 into bustle:master Mar 31, 2019
@dotboris dotboris deleted the write-to-stream-errors branch March 31, 2019 12:56
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