don't put callback inside try block

[dominictarr]

this can be a weird bug: https://github.com/wankdanker/node-discover/blob/master/lib/network.js#L111-L116

if there is a runtime error when the callback in called (and that is in other code, so anything could happen) then the try will catch it, and then call the callback again. so the callback will be called twice, once with a success, then again with an error, except with an error that was actually IN the CALLBACK.

this will be very weird, so once you are bitten by that you never forget.

[wankdanker]

Good call. I don't completely remember what I was thinking when I wrote that, but I think it was trying to cover the JSON conversion and encrypting stuff with the try catch. I'd be better off doing that stuff within it's own try catch, then call the callback afterward.

Something like:

Network.prototype.encode = function (data, callback) {
    var self = this;
    var tmp;

    try {
        tmp = (self.key)
                ? encrypt(JSON.stringify(data),self.key)
                : JSON.stringify(data)
    }
    catch (e) {
        return callback(e, null);
    }

    return callback(null, tmp);
};

And the same should apply to the decode function. Do you agree?

I'll get a fix for that out real quick if this is what you are suggesting.

Thanks,

Dan

[dominictarr]

yup, exactly that.

[dominictarr]

oh, also this function is sync, so you don't really need a callback anyway.

[wankdanker]

@dominictarr, Thank you for reviewing this code; I really appreciate your time.

I may change the code to avoid callbacks altogether. If I recall correctly, I did it with callbacks in case anyone ever wanted to override the encode/decode functions in a way that might invoke other io. Although that is probably unlikely to happen.

[dominictarr]

sure, looks good!