handle errors better - Githubissues

dominictarr commented 7 years ago

sink and duplex should throw if there is no callback provided, but transform should pass the error on to the sink (after destroying the node stream)

dominictarr commented 7 years ago

kumavis commented 5 years ago

I encountered unhandled errors: calling something like below without a callback and expected the error to propagate

pull(
  sourceThatWillError,
  toPull.duplex(through2(...)),
  sinkThatWantsThatError
)

also did not propagate the error with toPull.transform(...)

kumavis commented 4 years ago

I had made a repo demonstrating this error propagating issue, forgot to post it https://github.com/kumavis/stream-to-pull-stream-bug/blob/80afddd25d47df7aab7b73b89152fa7abe3ec531/index.js

dominictarr commented 4 years ago

hmm, yeah it should probably do that. I wrote this module when I was starting with pull-streams, but then I wrote more and more pull-streams and I didn't really ever use stream-to-pull-stream very often. When ipfs.js started using pull-streams they sent a bunch of PRs to improve this module, so I think they are the main user of it.

transform is really simple: https://github.com/pull-stream/stream-to-pull-stream/blob/master/index.js#L208-L213 sink actually takes a callback.

exports.transform = function (stream) {
  return function (read) {
    var _source = source(stream)
    sink(stream, function (err) {
     if(err) stream.emit('error', err)
   })(read); return _source
  }
}

That might work, but also emitting an error on another stream is a bit naughty. Hmm, better would be if source sets an onError function on it's returned read function, and when that's called it does the same thing as if the stream emitted an error.

pull-stream / stream-to-pull-stream

handle errors better #11