Errors are not propagated to callback

[manuelsantillan]

Hi!

I've found that client.on("error", function(err){...}) only logs the error. Instead, it would nice to callback(err) to allow for proper error handling upwards, passing the error reference instead of an error flag.

cheers!

[albertosantini]

I agree.

If I am not wrong, rio manages the error (checking) bubbling in close event.

...
client.on("close", function (had_error) {
        log("Closed from Rserve");

        if (had_error) {
            callback(true);
        }
    });

    client.on("error", function (e) {
        log("Rserve exception - " + e);
    });
...

Indeed the close event will be called directly following the error event as stated in nodejs docs. http://nodejs.org/api/net.html#net_event_error

Am I missing anything?

Maybe you mean it would be nice passing to the callback the kind of the error (error reference).

For instance,

server.on("error", function (e) {
  callback(e.code);
  ...
});

Anyway, when there is an error, both, error and close, events are called. We need to call the callback only once. I don't remember if had_error is enough to catch the correct flow. This is the reason I did the tradeoff, calling the callback only in the close event and propagating the error flag and not the kind of the error.

Any thoughts? Or snippet to reproduce your use case?

[manuelsantillan]

I usually fix this double callback by using a closure (a flag variable yo avoid calling the callback twice) I'll try to post a snippet asap

El viernes, 25 de abril de 2014, albertosantini notifications@github.com escribió:

I agree.

If I am not wrong, rio manages the error (checking) bubbling in closeevent.

... client.on("close", function (had_error) { log("Closed from Rserve");

    if (had_error) {
        callback(true);
    }
});

client.on("error", function (e) {
    log("Rserve exception - " + e);
});

...

Indeed the close event will be called directly following the error event as stated in nodejs docs. http://nodejs.org/api/net.html#net_event_error

Am I missing anything?

Maybe you mean it would be nice passing to the callback the kind of the error (error reference).

For instance,

server.on("error", function (e) { callback(e.code); ... });

Anyway, when there is an error, both, error and close, events are called. We need to call the callback only once. I don't remember if had_error is enough to catch the correct flow. This is the reason I did the tradeoff, calling the callback only in the close event and propagating the error flag and not the kind of the error.

Any thoughts? Or snippet to reproduce your use case?

— Reply to this email directly or view it on GitHubhttps://github.com/albertosantini/node-rio/issues/13#issuecomment-41426635 .

Manuel Santillán Socio - Director de Operaciones www.solvver.com

[albertosantini]

Gracias Manuel.

Your contribution will be well accepted.

[albertosantini]

Thanks for the PR.

I would create a new release, 1.3.0, if you agree.

[manuelsantillan]

Glad to help! A new release is fine for me of course

Cheers!

El domingo, 27 de abril de 2014, albertosantini notifications@github.com escribió:

Thanks for the PR.

I would create a new release, 1.3.0, if you are agree.

— Reply to this email directly or view it on GitHubhttps://github.com/albertosantini/node-rio/issues/13#issuecomment-41497585 .

Manuel Santillán Socio - Director de Operaciones www.solvver.com

[albertosantini]

Published 1.3.0.