Search code examples
node.jserror-handlingpromisetry-catchbluebird

Why my try catch block didn't catch the fs.renameSync exception thrown by my Promise.promisifyAll object


The code is quite straightforward. ks3 is a library someone else developed. It has a start function to download files. It uses async.auto to do that. I wrap it with bluebird Promise.promisifyAll

let ks3p = Promise.promisifyAll(ks3) //ks3 has a start function
try {
    await ks3p.startAsync(param)
} catch (err) {
    //failed to catch fs.renameSync
}

But sometimes I get the error

fs.js:115
    throw err;
    ^
Error: ENOENT: no such file or directory, rename ... -> ...
    at Object.renameSync (fs.js:591:3)

So why the try catch block failed to catch that ?

I further checked the start() implementation. It uses async to download files, but nothing special.

async.auto({
    step_1 : ...
    step_2 : ...
    },
    function(err, results) {
    if (err) {
        if (cb) {
            cb(err, results)
        } else {
            fs.unlinkSync(configFile);
            throw err;
        }
    } else {
        fs.unlinkSync(configFile);
        fs.renameSync(downFileName, filePath);
        if (cb) {
            cb(err, {msg:'success',path:filePath}, null);
        }
    }
})

------ update -------

Part of reason I wrap with Promise.promisifyAll is I do not know how to catch that error. My original code was like this

ks3.download.start(param, (err, result) => {
    //But how do I catch the exception start throw asynchronously ?
    if (err) {
        // error handling 
    } else {
        log(`finished download ${file}`)
    }
    done()
})

------ update2 -------

After further digging the issue (the answer is helpful) I find either I modified ks3 codes or I will have to use domain to catch the exception. I know domain is deprecated. But for this specific issue I find it is appropriate b/c I know exactly what causes the problem and I also don't fix that problem at the moment(b/c it is a npm module I have no control).


Solution

  • See try/catch block not catching async/await error for some additional context, but to answer your question about how to catch the error in the un-promisified version, you have to understand the call stack when the error is thrown.

    You had

    ks3.download.start(param, (err, result) => {
        //But how do I catch the exception start throw asynchronously ?
        if (err) {
            // error handling 
        } else {
            log(`finished download ${file}`)
        }
        done()
    })
    

    which you should think of as:

    var cb = (err, result) => {
        //But how do I catch the exception start throw asynchronously ?
        if (err) {
            // error handling 
        } else {
            log(`finished download ${file}`)
        }
        done()
    });
    
    ks3.download.start(param, cb);
    

    and it's start itself that's throwing the exception (way before it gets around to calling or scheduling your callback), so you'd need to wrap that call in a try-catch:

    var cb = (err, result) => {
        //But how do I catch the exception start throw asynchronously ?
        if (err) {
            // error handling 
        } else {
            log(`finished download ${file}`)
        }
        done()
    });
    
    try {
        ks3.download.start(param, cb);
    } catch (ex) {
        // here.
    }
    

    I'm a little suspicious of async.auto's exception-handling, though, and i fear it might be running something throwing asynchronously without catching errors. In particular, it doesn't look like it expects the callback function to ever throw (see https://github.com/caolan/async/blob/master/lib/auto.js), but https://github.com/ks3sdk/ks3-nodejs-sdk/blob/master/lib/api/download.js does throw if the fs methods fail, like you're seeing.

    As such, there's nothing you can do other than a) fix ks3 or b) maybe find a way to monkey-patch the version of fs that ks3 sees to never throw. Of the two, (a) sounds a lot easier to me and i think it should look something like this:

    async.auto({
        // ...
    }, 
    function(err, results) {
        if (cb) {
            if (err) {
                cb(err);
                return;
            }
            try {
                fs.unlinkSync(configFile);
                fs.renameSync(downFileName, filePath);
            } catch (ex) {
                cb(ex);
                return;
            }
            cb(err, {msg:'success', path:filePath}, null);
        } else {
            // really, i wouldn't even bother fix this case and just make cb required.
        }
    })
    

    On top of that, i'm assuming it's a bug that the ks3 code doesn't delete the config file if there's an error and there's a callback.