Thrown error no longer propagates (caught in the try-catch inside invokeTask in patchEventTarget in events.ts)

[br-star]

I have an application that performs validation on mouse events. If validation fails, it throws an Error. We catch thrown errors in our application by doing:
window.addEventListener('error', (event) => { handleUncaughtError(event, logger); });

This worked fine: task.invoke() in the below code would call into our code, which would throw an error and would be caught by the above listener.

var invokeTask = function (task, target, event) {
        // for better performance, check isRemoved which is set
        // by removeEventListener
        if (task.isRemoved) {
            return;
        }
        var delegate = task.callback;
        if (typeof delegate === 'object' && delegate.handleEvent) {
            // create the bind version of handleEvent when invoke
            task.callback = function (event) { return delegate.handleEvent(event); };
            task.originalDelegate = delegate;
        }
        // invoke static task.invoke
        task.invoke(task, target, [event]);
        var options = task.options;
        if (options && typeof options === 'object' && options.once) {
            // if options.once is true, after invoke once remove listener here
            // only browser need to do this, nodejs eventEmitter will cal removeListener
            // inside EventEmitter.once
            var delegate_1 = task.originalDelegate ? task.originalDelegate : task.callback;
            target[REMOVE_EVENT_LISTENER].call(target, event.type, delegate_1, options);
        }
    };

However, now the callsite for task.invoke() looks like:

var invokeTask = function (task, target, event) {
        // for better performance, check isRemoved which is set
        // by removeEventListener
        if (task.isRemoved) {
            return;
        }
        var delegate = task.callback;
        if (typeof delegate === 'object' && delegate.handleEvent) {
            // create the bind version of handleEvent when invoke
            task.callback = function (event) { return delegate.handleEvent(event); };
            task.originalDelegate = delegate;
        }
        // invoke static task.invoke
        // need to try/catch error here, otherwise, the error in one event listener
        // will break the executions of the other event listeners. Also error will
        // not remove the event listener when `once` options is true.
        var error;
        try {
            task.invoke(task, target, [event]);
        }
        catch (err) {
            error = err;
        }
        var options = task.options;
        if (options && typeof options === 'object' && options.once) {
            // if options.once is true, after invoke once remove listener here
            // only browser need to do this, nodejs eventEmitter will cal removeListener
            // inside EventEmitter.once
            var delegate_1 = task.originalDelegate ? task.originalDelegate : task.callback;
            target[REMOVE_EVENT_LISTENER].call(target, event.type, delegate_1, options);
        }
        return error;
    };

and it seems like the error is ignored in globalCallback()

function globalCallback(context, event, isCapture) {
        // https://github.com/angular/zone.js/issues/911, in IE, sometimes
        // event will be undefined, so we need to use window.event
        event = event || _global.event;
        if (!event) {
            return;
        }
        // event.target is needed for Samsung TV and SourceBuffer
        // || global is needed https://github.com/angular/zone.js/issues/190
        var target = context || event.target || _global;
        var tasks = target[exports.zoneSymbolEventNames[event.type][isCapture ? utils_1.TRUE_STR : utils_1.FALSE_STR]];
        if (tasks) {
            // invoke all tasks which attached to current target with given event.type and capture = false
            // for performance concern, if task.length === 1, just invoke
            if (tasks.length === 1) {
                invokeTask(tasks[0], target, event);
            }
            else {
                // https://github.com/angular/zone.js/issues/836
                // copy the tasks array before invoke, to avoid
                // the callback will remove itself or other listener
                var copyTasks = tasks.slice();
                var errors = [];
                for (var i = 0; i < copyTasks.length; i++) {
                    if (event && event[IMMEDIATE_PROPAGATION_SYMBOL] === true) {
                        break;
                    }
                    var err = invokeTask(copyTasks[i], target, event);
                    err && errors.push(err);
                }
                var _loop_1 = function (i) {
                    var err = errors[i];
                    api.nativeScheduleMicroTask(function () {
                        throw err;
                    });
                };
                for (var i = 0; i < errors.length; i++) {
                    _loop_1(i);
                }
            }
        }
    }

IIUC, this is caused by the fix for this #41562 (comment)

This is a regression (I tested with a version from 3 weeks ago and it worked as expected).

Can we ensure that all caught errors are rethrown? Or whatever is the Angular way to undo this regression.

[JiaLiPassion]

@br-star , could you share are reproduce repo? The error should be rethrown by the following code.

api.nativeScheduleMicroTask(function () {
    throw err;
});

And you can catch it by using

window.addEventListener('unhandledrejection', function (event) {
}

[br-star]

In my callstack, it goes through

if (tasks.length === 1) {
    invokeTask(tasks[0], target, event);
}

[JiaLiPassion]

@br-star , in that case, the behavior doesn't change by this #41562 (comment) commit, the error will be re-thrown, and it will be caught by the global error handler of Angular, so you will not catch this error by window error event listener even before this commit.

[br-star]

I'm a little confused, where is it re-throwing? invokeTask catches the error, doesn't store it anywhere, returns it, and

if (tasks.length === 1) {
    invokeTask(tasks[0], target, event);
}

doesn't read the return value?

[JiaLiPassion]

On you are right, the code is not handled right here since the recent refactor, I will fix it. Thanks.

[br-star]

Thanks!

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}