-
Notifications
You must be signed in to change notification settings - Fork 84
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
Handle Error cases #35
Comments
OK, i didn't see #20. but, still how about promise when it is rejected? |
This is a great question and I'm aware that the documentation doesn't describe errors at all :( My current opinion is that Flyd should not concern itself with error handling. I don't see any benefit in doing so. For normal synchronous code developers know how to handle errors. You need to handle that some functions might return You know all of the above of course. But that Flyd doesn't do any var userStringStream = flyd.stream(); // user information that should be formatted as JSON
var userData = flyd.map(function(userString) {
try {
return JSON.parse(userString);
} catch (err) {
return emptyUserData;
}
}, userStringStream); The above is pretty straight forward. We do an operation that might throw var userDataUrl = flyd.stream(); // and URL from which user information can be loaded
var userData = flyd.map(function(url) {
return fetch(url).catch(function(err) {
return emptyUserData;
});
}); Is you've properly noticed and mentioned Flyd doesn't handle rejected promises. The idea is that you should never send promises that might reject down a stream. This is achieved by adding The benefits of the above method is:
This is my current thinking and this is what I've done until know. But this is not set in stone. If you see any problems with the above approach by all means express your concerns! |
Would be nice, though, if flyd could tell if user code forgot to make sure that a promise never gets rejected down a stream. I'm thinking |
@roobie I think that is a very good idea! |
Just an opinion though, the "catch" method of Promise provides a way to handle all the up stream errors in one place, so that you don't need to write catch code to each one of them. If just let stream pass on its error to its dependencies then i can handle all errors in one place, and reduce some code. For instance, when you compose a complex stream with merge or map or some other module methods, then if i can only handle errors in this final stream that would be nice:). Sometimes, you may consume promises from outside that don't catch errors, and you have to wrap a function to each of them to catch errors. For example, I hope i can do this:
|
@ybybzj I see what you mean. In some cases the current approach might increase the amount of error handling boiler plate since it's harder to handle errors in a central place. If I understand you correctly what you're suggesting is that Flyd automatically does something like this: var api = require('./api');
function catchAndWrapErr(promise) {
return promise.catch(function(err) {
return {isErr: true, err: err};
});
}
var s1 = flyd.stream(catchAndWrapErr(api.getA()));
var s2 = flyd.stream(catchAndWrapErr(api.getB()));
var s3 = flyd.stream(catchAndWrapErr(api.getC()));
var s = flyd.stream([s1, s2, s3], function(self){
if (s1().isErr || s2().isErr) || s3().isErr) {
return {};
} else {
return {
s1: s1(),
s2: s2(),
s3: s3()
};
}
}); Is that correct? |
Yeah, something like this. Thanks for your reply! |
When Kefir was developing this feature I suggested that users should deal with Errors as first-class values via an abstraction such as Either. This direction did not get any traction in the discussion, but since one of Flyd's goals is to be a full card-carrying FP citizen, it seems that maybe it will here? ––Edit Agree:
Disagree:
This is very hostile toward generic programming which will not know enough about errors to handle them at all. You are basically forbidding the ability to build I/O into libraries with this line of thinking. It seems @ybybzj is intuitively thinking along the same lines. Allow errors to flow through the stream. I am of the opinion that I/O values should be represented as I will try to find time to write some pseudo code example of what this could look like at the library-layer and at the application-layer. |
What does full card-carrying mean? But yes! I had not though of that but using an either type would be a splendid idea. FRP is based on functional programming and an either type is how one typically handles errors with functional programming. Instead of the previous function wrapEither(promise) {
return promise.then(function(result) {
return Either.Right(result);
}).catch(function(err) {
return Either.Left(err);
});
} And then wrap promises with that function before you pass them to Flyd. |
It means
: )
Cool glad you see promise in this direction! Again I'll try to share more ideas later. |
@jasonkuhrt , looking forward to your pseudo code example. 😊 |
It would be great if the docs could mention error handling, and at least one of the examples include error handling from various sources (promises, node-backs, etc) |
|
I don't think Flyd should provide its own |
could it be useful to provide an error stream like the end stream? So for proper error handling the suggested solution should be using either. The error stream would allow to have error handling for errors in flyd or its modules (which should not handle Either so they must handle all errors). i like the decision to move the usage of either out of the scope of flyd. Even though it probably means every own function (eg. a map fn) which just needs to pass errors along, would need to be wrapped with a curried fn of |
Any thoughts on how to handle errors in a stream? For example, if a promise is rejected, from what i saw in code the stream will never be triggered, since the promise only invoke like this
n.then(s) //line 134
.I try to improve this, but have not come up a rational solution. Should stream just end itself when it encounters an error, or should it provide a way to be listened to its errors?
The text was updated successfully, but these errors were encountered: