zeromq.node icon indicating copy to clipboard operation
zeromq.node copied to clipboard

`.bind` (async) can throw even though it is asynchronous

Open AlmirKadric opened this issue 9 years ago • 5 comments

I think it is best practice to wrap these parts in a try catch and return the error via the callback

check: https://github.com/JustinTulloss/zeromq.node/blob/1a8977137a51aa2259fdc4b3c1441aca301ac0db/lib/index.js#L421

AlmirKadric avatar Apr 13 '16 06:04 AlmirKadric

These lines are outdated since you aim at the master branch. We can't really tell what you're pointing at (but tbh, I can guess)...

ronkorving avatar Apr 13 '16 08:04 ronkorving

I think it is best practice to wrap these parts in a try catch and return the error via the callback

is there a use case or reason for this proposal? Like.. is there a problem and does that solve it?

reqshark avatar Apr 13 '16 09:04 reqshark

oh now i see the subsequent issue, so is this basically answering #507?

reqshark avatar Apr 13 '16 09:04 reqshark

@reqshark: bindSync prevents sockets from being re-used on failed attempts to bind. This issues is more a problem that if i want to handle errors properly with the async version, i need to wrap the whole thing in a try catch, which kinda defeats the purpose of feeding error back via callback. I ultimately now have to handle errors in 2 places when it could just be the one (which is the asynchronous programming standard).

To help you visualise this for me to write a safe .bind function i have to do the following:

function listen(bindUrl, cb) {
    var socket = zmq.socket('router');
    try {
        socket.bind(bindUrl, cb);
    } catch (error) {
        return cb(error);
    }
}

And then this starts to get out of control if i start to put error handling into it.

AlmirKadric avatar Apr 13 '16 16:04 AlmirKadric

To update this issue, the relevant lines that need to wrapped in a try catch are: https://github.com/JustinTulloss/zeromq.node/blob/1a8977137a51aa2259fdc4b3c1441aca301ac0db/lib/index.js#L421

Which is now worse as the throw can happen asynchronously and cant be caught. Also prevents the bind operation from calling back.

AlmirKadric avatar Apr 18 '16 05:04 AlmirKadric