douglascrockford / parseq

Better living thru immediacy!
214 stars 28 forks source link

Are callbacks supposed to throw exceptions? #13

Closed bunglegrind closed 1 year ago

bunglegrind commented 1 year ago

I'm not sure if it's really a bug or I have misunderstood some parseq basic concepts. It is stated at page 20.9 that requestors cannot throw exceptions but they're supposed to pass their failures as a second parameter (first is undefined) when invoking the callback, but I'm not sure if the same applies to callbacks. Let me show you some examples:

Example 1

parseq.sequence([
    function requestor(callback) {
        callback(1);
    }
])(function (value, reason) {
    console.count("pass");
    if (value === undefined) {
        console.log("There is a failure");
        return console.log(reason);
    }
    throw new Error("Booom!");
});

Output:

pass: 1
pass: 2
There is a failure
Error: Booom!

Are callbacks supposed to be called more than wunce?

Example 2

//taken from page 20.5
function requestorize(unary) {
    return function requestor(callback, value) {
    try {
        return callback(unary(value));
    } catch (exception) {
        return callback(undefined, exception);
    }
}

parseq.sequence([
    requestorize(function () {
        return 1;
    })
])(function (value, reason) {
    console.count("pass");
    if (value === undefined) {
        console.log("There is a failure");
        return console.log(reason);
    }
    throw new Error("Booom!");
});

Output:

pass: 1

In this case, the program silently fails. Which is not good. But I can replicate the example 1 behavior with the following requestorize factory:

function requestorize(unary) {
    return function requestor(callback, value) {
        let return_value;
        try {
            return_value = unary(value);
        } catch (exception) {
            return callback(undefined, exception);
        }
        callback(return_value);
    };
}

...but I think the same applies (in more complex contexts) to the try catch block within the start_requestor function on line 141 in parseq.js. EDITED: Like in the following example:

Example 3

parseq.sequence([
    parseq.sequence([
        function requestor(callback) {
            callback(1);
        }
    ])
])(function (value, reason) {
    console.count("pass");
    if (value === undefined) {
        console.log("There is a failure");
        return console.log(reason);
    }
    throw new Error("Booom!");
});

Output:

pass: 1

And of course, if I execute the requestors without any parseq factory, I have another different behavior. But probably requestors shouldn't be executed "stand-alone".

In conclusion, I think there is ambiguity in terms of how the callbacks should manage failures. What is your opinion, mr. Crockford? I'm a bit puzzled...

douglascrockford commented 1 year ago

Computation happens over many turns. An exception can not communicate with another turn because time travel is not allowed. Parseq provides a way to communicate a failure to a turn in the future.

bunglegrind commented 1 year ago

Ok, I know that. But what happens if the final callback throws an error? There's no future to communicate with at the end of the line...I'm expecting that in this particular case, the program would explode, but, what I can see is that, it's not always happening. Sometimes they program crashes, sometimes the final callback is called twice, sometimes nothing happens at all...

Anyway, thanks for your reply

douglascrockford commented 1 year ago

It is your responsibility to call each callback once, or to back it up with a timeout.

bunglegrind commented 1 year ago

well, I can think to add some more constraint on the requestors;

  1. except for some data preparation, where requestors may return the cancel function, they must execute their work in a different turn; (that means that the requestorize factory example in the book is too simple)
  2. requestors must not catch callback errors. Otherwise we'll have callbacks possibly called two times, or silent failures.

I'm not sure if it's enough, because the callback start_requestor inside the run function can actually invoke the callback too times (first one on line 161, second one on line 186). But I cannot think of insert setTimeout in the final callback, it's too intricate.

Anyway, thanks for your patience

jamesdiacono commented 1 year ago

If, on the same turn, a requestor is called, its callback throws, and the requestor does not handle it, the action function (and hence the callback) gets called twice.

We could say that such a requestor is malformed, because requestors should not throw. But ultimately, a requestor is beholden to its callback:

function responsible_requestor(callback) {
    try {
        // do stuff
    } catch (exception) {
        callback(undefined, exception);     // <-- this can throw
    }
}

This seems to imply a new requestor pattern rule: callbacks should not throw.

douglascrockford commented 1 year ago

A general principle of exception handling is that nothing should be thrown that isn't caught. Conventional exceptions fail in inter-turn computation, and so should only be used locally.