jdonaldson / promhx

A promise and functional reactive programming library for Haxe
MIT License
146 stars 24 forks source link

Strange error handling semantic. #12

Closed sledorze closed 10 years ago

sledorze commented 10 years ago

a Promise should not behave differently when composed or not. for instance; it should not throw an exception when rejected.

    var ooo = new Promise();
    ooo.reject(5);      // thorws!!!

(Remember; it's a monad; the error is a value).

sledorze commented 10 years ago

Also a Promise is a value; binding before or after resolution should not change the semantic; otherwise the API is sensible to Async computation order; which it tries to abstract to ease reasonning about.

    var ooo = new Promise();
    ooo.reject(5);
    ooo.error(function (x) Node.console.info('should print $x')); // SHOULD PRINT :)
jdonaldson commented 10 years ago

The short answer is that errors are not part of the semantics for handling values. I think this actually ends up being a feature, but I'm interested in hearing more about problems.

The error function is really there as a safety net for asynchronous operations. In all of your examples, it's trivial to handle those situations synchronously without using the error function or any other asynchronous code.

If you really must have an error function defined when you create the promise, you can do something like:

var ooo = new Promise(function(error) trace(error));
ooo.reject(5);
sledorze commented 10 years ago

Doing that, you cannot delegate the error management to the lib user. This is a show stopper.

On Wed, Dec 4, 2013 at 5:20 PM, Justin Donaldson notifications@github.comwrote:

The short answer is that errors are not part of the semantics for handling values. I think this actually ends up being a feature, but I'm interested in hearing more about problems.

The error function is really there as a safety net for asynchronous operations. In all of your examples, it's trivial to handle those situations synchronously without using the error function or any other asynchronous code.

If you really must have an error function defined when you create the promise, you can do something like:

var ooo = new Promise(function(error) trace(error));ooo.reject(5);

— Reply to this email directly or view it on GitHubhttps://github.com/jdonaldson/promhx/issues/12#issuecomment-29819478 .

Stéphane Le Dorze

http://lambdabrella.blogspot.com/ http://www.linkedin.com/in/stephaneledorze http://twitter.com/stephaneledorzehttps://twitter.com/#!/stephaneledorze/status/74020060125077504

Tel: +33 (0) 6 08 76 70 15

jdonaldson commented 10 years ago

Please explain why this is so.

sledorze commented 10 years ago

if I consume a service that expose Promises; and that there's an error; I want to be able to deal with it as in case of underlying error; I can't substitue it an alternative value. Just think about how you would implement a timeout with the current API..

On a side note: I've worked a lot with Futures / Promises / Async in general and the current behavior/semantic is not workable with.

You may check scala Futures, even if I don't always buy the dsl naming, the semantic is maintained. http://docs.scala-lang.org/overviews/core/futures.html

Best :)

jdonaldson commented 10 years ago

I'll look into it. Setting a timeout is one way, but it seems like a hack. The other way I could see working is by using a "start()" method that will kick off the promise evaluations, and make sure that error handlers are set. That adds more overhead though, so I'm more inclined to try a timeout approach first.

I'm still not seeing any of this as being a big limitation of promhx. The library is fully capable of managing errors, you just need to understand the mechanism.

sledorze commented 10 years ago

fair enough

labe-me commented 10 years ago

Just to tell that I felt the pain too :)

The main interest of futures/promises is to removes the burden of passing callbacks to method calls.

In the current implementation, to make things safe you have to create APIs like this:

class MyApi {
  public function doSomething(errorCb){
    var p = new promhx.Promise(errorCb);
    // ...
    return p;
  }
}

It forces the Api user to create an error handler before calling the API and preferably to pass the error handler through the entire Api calls.

Moreover I am not sure how well it integrates with the dO notation

// define the error callback before hand to be safe
function onError(err){
  // do something
}

promhx.mdo.PromiseM.dO({
    value <= doSomething(onError);
    other <= doOtherThing(onError);
})
.error(onError); // mmmm not sure it is a good idea

Regards Laurent

jdonaldson commented 10 years ago

Yeah, that's a pain. I should be able to do something to improve this. The setTimeout option is probably the best bet now. I'll have to see about making sure that the errors are all handled in the same order. Also, I have doubts about that working with cpp/java/etc... those seem to require a separate thread.

FWIW I've seen all the other libraries struggle with this.

Best, -Justin

On Thu, Dec 5, 2013 at 1:25 AM, Laurent Bédubourg notifications@github.comwrote:

Just to tell that I felt the pain too :)

The main interest of futures/promises is to removes the burden of passing callbacks to method calls.

In the current implementation, to make things safe you have to create APIs like this:

class MyApi { public function doSomething(errorCb){ var p = new promhx.Promise(errorCb); // ... return p; } }

It forces the Api user to create an error handler before calling the API and preferably to pass the error handler through the entire Api calls.

Moreover I am not sure how well it integrates with the dO notation

// define the error callback before hand to be safe function onError(err){ // do something } promhx.mdo.PromiseM.dO({ value <= doSomething(onError); other <= doOtherThing(onError); }) .error(onError); // mmmm not sure it is a good idea

I too very much prefer to see the promise as a value containing a result or an error :)

— Reply to this email directly or view it on GitHubhttps://github.com/jdonaldson/promhx/issues/12#issuecomment-29882971 .

sledorze commented 10 years ago

I m not sure you re on the right path at all.. The problem is that error should be treated like values and not escape this monad. Also no order should be maintained. This is about asynchronicity if you requier order, it implies you re doing it wrong. What exactly are all other libraries struggling with?

Le 5 déc. 2013 19:11, "Justin Donaldson" notifications@github.com a écrit :

Yeah, that's a pain. I should be able to do something to improve this. The setTimeout option is probably the best bet now. I'll have to see about making sure that the errors are all handled in the same order. Also, I have doubts about that working with cpp/java/etc... those seem to require a separate thread.

FWIW I've seen all the other libraries struggle with this.

Best, -Justin

On Thu, Dec 5, 2013 at 1:25 AM, Laurent Bédubourg notifications@github.comwrote:

Just to tell that I felt the pain too :)

The main interest of futures/promises is to removes the burden of passing callbacks to method calls.

In the current implementation, to make things safe you have to create APIs like this:

class MyApi { public function doSomething(errorCb){ var p = new promhx.Promise(errorCb); // ... return p; } }

It forces the Api user to create an error handler before calling the API and preferably to pass the error handler through the entire Api calls.

Moreover I am not sure how well it integrates with the dO notation

// define the error callback before hand to be safe function onError(err){ // do something } promhx.mdo.PromiseM.dO({ value <= doSomething(onError); other <= doOtherThing(onError); }) .error(onError); // mmmm not sure it is a good idea

I too very much prefer to see the promise as a value containing a result or an error :)

— Reply to this email directly or view it on GitHub< https://github.com/jdonaldson/promhx/issues/12#issuecomment-29882971> .

— Reply to this email directly or view it on GitHub.

jdonaldson commented 10 years ago

There's a discussion on error handling buried in here: https://github.com/promises-aplus/promises-spec/issues/94

The main issue is that if you want synchronous error handling on the function inside of "then", you have to define the error handler along with the function argument:

p.then(handleValue, handleError);

This brings up a number of issues and questions. Can "then" work as a "map" function and enable do-notation? How does chaining proceed in the case of an error? E.g., if an error is handled with handleError, does the promise value resolve with its return value? Does it halt the chain? Must all "then" methods include an error handler? I'd like to think that there is a "right path", but you're going to find a lot of variation in implementation. Javascript alone has dozens of such libraries and specs right now.

Keep in mind that you can still manage errors with values in promhx. You can pass haxe.ds.Option types as a promise value, and keep your error values there. The "error" method is meant as a safety net for errors that come from stateful operations, and misc. other situations that aren't handled by a well specified error value. Since Haxe allows for OO and FP style, I think it's a good idea to include such a function.

wighawag commented 10 years ago

what about another option: save the error until the "error" funciton is called ? (same as it does for value and the "then" function)

jdonaldson commented 10 years ago

I've got that working already for JavaScript.... I'm working on the other targets that don't have event lops. On Dec 8, 2013 2:42 AM, "wighawag" notifications@github.com wrote:

what about another option: save the error until the "error" funciton is called ? (same as it does for value and the "then" function)

— Reply to this email directly or view it on GitHubhttps://github.com/jdonaldson/promhx/issues/12#issuecomment-30078979 .

jdonaldson commented 10 years ago

I've just pushed a rather large update that tries to address this error handling problem. It'll let you set error handlers after you've already set a then function, rejected a promise, etc. You can check out the test for it here.

It works pretty well for js and should be good for other runtimes with native event loops. But, it caused a lot of problems on other targets. I've rewritten test handlers for targets without event loops, and patched utest to make this sort of testing possible. I also had to disable php testing for the time being, it was giving me a mysterious runtime error.

labe-me commented 10 years ago

It seems to work pretty well (except for a little thing but I will open a new issue).

jdonaldson commented 10 years ago

sticking to this new error handling mechanism. Let me know if there's any new issues.

brendanjerwin commented 10 years ago

@jdonaldson I'm having some issues with the error behavior. Is this a reasonable place to discuss, or should I open a new issue?

jdonaldson commented 10 years ago

Could you start a new thread? I did make changes to the error handling behavior, and I'd like the new thread to reflect the current state.

brendanjerwin commented 10 years ago

Done. See #37