Skip to content
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

Closed
ybybzj opened this issue Jun 10, 2015 · 15 comments
Closed

Handle Error cases #35

ybybzj opened this issue Jun 10, 2015 · 15 comments

Comments

@ybybzj
Copy link

ybybzj commented Jun 10, 2015

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?

@ybybzj
Copy link
Author

ybybzj commented Jun 10, 2015

OK, i didn't see #20. but, still how about promise when it is rejected?

@paldepind
Copy link
Owner

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 null/undefined in certain cases and if you're using functions in a way so they might throw you use try/catch. Promises offers similar features for asynchronous code.

You know all of the above of course. But that Flyd doesn't do any n.catch(someErrHandler) might still seem like a problem to you. This is why I don't think it's a problem. First consider an example with synchronous error handling:

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 JSON.parse. If it doesn't throw the stream is set to it's return value and if it does some default data is used instead. The same thing can be achieved with promises today:

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 catch handlers to your promises before you return them to a stream.

The benefits of the above method is:

  • Flyd stays simple
  • One doesn't have to learn anything new about errors handling
  • All the powerful error handling than one normally uses works well with Flyd

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!

@roobie
Copy link

roobie commented Jun 10, 2015

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 p.catch(() => console.warn('flyd doesn\'t handle errors'))

@paldepind
Copy link
Owner

@roobie I think that is a very good idea!

@ybybzj
Copy link
Author

ybybzj commented Jun 10, 2015

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:

var api = require('./api');

var s1 = flyd.stream(api.getA());
var s2 = flyd.stream(api.getB());
var s3 = flyd.stream(api.getC());

var s = flyd.stream([s1, s2, s3], function(self){
    if(flyd.isErr(s1()) || flyd.isErr(s2()) || flyd.isErr(s3())){
         return {};
    }else{
        return {
             s1: s1(),
             s2: s2(),
             s3:s3()
        };
    }
});

@paldepind
Copy link
Owner

@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?

@ybybzj
Copy link
Author

ybybzj commented Jun 10, 2015

Yeah, something like this. Thanks for your reply!
And, users can easily forget to handle errors, maybe try-catch the synchronous code too?

@jasonkuhrt
Copy link

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:

My current opinion is that Flyd should not concern itself with error handling. I don't see any benefit in doing so.

Disagree:

The idea is that you should never send promises that might reject down a stream.

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 Either values and a library of combinators should be built around composition that transparently operates on only/only-not errors etc.

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.

@paldepind
Copy link
Owner

one of Flyd's goals is to be a full card-carrying FP citizen

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 catchAndWrapErr function you could do this:

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.

@jasonkuhrt
Copy link

What does full card-carrying mean?

It means being a full-fledged member of an organization or the like. flyd is trying to be a tool that is pure to the functional programming paradigm. I based this opinion on your Readme which states:

[...] aren't functional enough

: )

But yes!

Cool glad you see promise in this direction! Again I'll try to share more ideas later.

@ybybzj
Copy link
Author

ybybzj commented Jun 17, 2015

@jasonkuhrt , looking forward to your pseudo code example. 😊

@kwijibo
Copy link
Contributor

kwijibo commented Oct 29, 2015

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)

@StreetStrider
Copy link
Contributor

Either is a good idea. There're two considerations here:

  1. Should flyd implement its own Either or there's good stuff on npm?
  2. Should flyd's primitives (like map) be updated to work with Either (e.g map should unwrap Eithers data and just pass error through itself down to catcher), or they will be ok as now?

@paldepind
Copy link
Owner

Should flyd implement its own Either or there's good stuff on npm?

I don't think Flyd should provide its own Either. There are implementations already and they should work with Flyd just fine.

@squiddle
Copy link

squiddle commented Jul 19, 2016

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 Either.chain(func, either) (if the func takes the values in the stream) or flyd.map(Either.chain(func)) (if the func takes the streams).

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

No branches or pull requests

7 participants