Exceptions create dead streams

[c-dante]

It is possible to create dead streams if you have code that runs in async frames such as setTimeout or in event listeners.

This gist has the example, you can run it in RequireBin and see int the console.

I'm using a setTimeout to push to a stream where a child stream throws an uncaught exception. The only place to catch the exception is inside the stream. The result is that the parent stream is now broken / unusable. You can update values to any stream, but no updates occur and no new streams can be attached.

I don't THINK that this warrants a change to the base level combine API, but perhaps a safer api that wraps stream updates in error handling can be useful. If you run into this case, you end up with a seemingly unresponsive app with no other errors than the one that breaks the stream.

There also seems to be no way to recover or investigate this state once you're in it.

[nordfjord]

you could always do something like

const attempt = fn => (...args)=> {
  try {
    return fn(...args);
  } catch(e) {
    console.error(e);
  }
}
const safe_combine = (fn, streams)=> flyd.combine(attempt(fn), streams);

To solve this on your side.

The reason errors break applications is because flyd has a global inStream variable that is populated with the current stream being updated. When an error happens inStream is never reset to undefined and flyd assumes everything is happening in a stream body and pushes results and updates to the toUpdate array.

[c-dante]

Yes, wrapping functions in an attempt also works. I can 100% solve this on my end, and have -- but I am throwing into question a way to either inspect when a stream is in this "dead" state, if it makes sense for a stream to include an error stream as well as an end stream, or any other thoughts around this issue.

The problem stems from using flyd streams from other sources in your own or other codebases -- there would be no way to recover from an error in an existing code base without some kind of monkey patching, since this kind of error cannot be caught, investigated, or recovered from in the current API

[StreetStrider]

@c-dante, you engage important question, but I think the situation is way bigger. I think this issue will be naturally resolved (as a consequence) when there will be any consensus on error handling in general. #35

flyd does not handle errors and haven't got any mechanisms to work with it (analogous to Promise.reject and .catch). So, like many other things, error handling footballed to user's end. I'm not judging it as good or bad, it's just a fact.

[c-dante]

Sure, I'll reference that issue and close this one out -- that or we can revive discussion on it somewhere else.

Specific to this issue is the property that throwing an exception in a child stream can kill the parent streams.

Duplication of #35. #20, #69.
Related to #104 .

[nordfjord]

I'm sorry if you construed my comment as saying "handle this yourself", I think this is an important discussion to have and merely wanted to point out a way for you to have safe streams while the discussion is being hashed out in the flyd community.

Regards,
Einar

[c-dante]

Nah, not offended / misconstrued -- more realizing we have like, 5 other issues around this topic.

I'm interested in an answer to this thread of questions, as well as #106

[c-dante]

I'm going to re-open this because I have some thoughts on some not-terrible functionality, inspired by this issue and the recent #170 issue.

My thoughts:

• Within one dependency graph, error handling should not really be flyd's concern, and should be handled by the user.
• Errors in one graph should not break another graph.
• An uncaught exception in a stream graph should end that stream immediately with that error.

I'm not terribly terribly offended by answers of, "Wrap your functions in try...catch", but I feel this is loose since flyd offers no control over how its dependency graph runs or handles errors.

My goal is to prevent completely unrelated flyd usage/exceptions from breaking other code. For instance, UI consuming flyd results.

Edit:

Some other thoughts -- it might make sense for flyd to re-throw exceptions after ending the stream and finishing the update flush. This way, the surrounding app can either handle or be aware of the exception, instead of it just living inside the end value of the stream.

Edit 2:

A not-well-thought-out-but-just-might-work idea on a fork: https://github.com/c-dante/flyd/commit/57267f550134c1e6daa7e4a38e0c0a1deea2054a

Thoughts?