ybogdanov / node-sync

Write simple and readable synchronous code in nodejs using fibers
MIT License
492 stars 70 forks source link

different types of error are propagated differently #48

Closed sa-0001 closed 9 years ago

sa-0001 commented 9 years ago

Here is a minimal example of the problem I am trying to solve:

sync = require('sync');

process.on('uncaughtException', function(err) {
    console.error('process.onUncaughtException');
    console.error(err.stack);
    process.exit(1);
});

try {
    sync(function() {
        try {
            sync(function() {
                try {
                    panic();
                } catch (err) {
                    console.log('Error1', err.message);
                    throw err;
                }
            }, function(err) {
                if (!err) return;
                console.log('Error2', err.message);
                throw err;
            });
        } catch (err) {
            console.log('Error3', err.message);
            throw err;
        }
    }, function(err) {
        if (!err) return;
        console.log('Error4', err.message);
        throw err;
    });
} catch (err) {
    console.log('Error5', err.message);
    throw err;
}

This script correctly outputs the following:

Error1 panic is not defined
Error2 panic is not defined
Error3 panic is not defined
Error4 panic is not defined
Error5 panic is not defined
process.onUncaughtException
ReferenceError: panic is not defined
    at /test/sync-error.js:14:8
    at /test/node_modules/sync/lib/sync.js:178:22

and it also works the same for plain errors (throw new Error(...)).


However, if I replace the call to panic() with any of the following:

then the output wil be as follows:

Error1 $relevantErrorMessage
Error2 $relevantErrorMessage
process.onUncaughtException
Error: $relevantErrorMessage

What's missing here is Error3,Error4,Error5 - so it seems that for some errors (those generated using Function.prototype.sync?) the error will not propagate upward through a chain of fibers. I discovered this when I removed 'domains' from an express service, and saw that some errors (plain, ReferenceError) were correctly handled, but others caused an uncaughtException.

Can anyone help me figure out why this happens, or point me in the right direction? Looking at the sync source has not yet helped me figure out this voodoo.

EDIT: I understand now why this is the case; for some reason I had assumed that a fiber started within a fiber would also run synchronously, but that probably doesn't make sense, and starting a fiber within a fiber is probably not considered an important use-case.

It is however actually possible to call sync() 'synchronously' when this is the desired behavior, so that can easily be done in a wrapper function. Full disclosure: the reason I start a fiber within a fiber, and occasionally want to do so 'synchronously', is to abuse the 'Fibers.current' property for passing around context data for profiling/debugging purposes.