Skip to content

Commit cb57cdb

Browse files
arturovtAndrewKushnir
authored andcommitted
fix(zone.js): do not silence uncaught promise rejections when the finally is called (#45449)
The native promise implementation logs unhandled promise rejections after `onFinally` has been called. We may rely on the `scheduledByFinally` argument, which is `false` by default, to avoid creating a breaking change; this will also allow us to reduce the number of changes. The `finally` function should not silence unhandled promise rejections, because they're not silenced in any implementation. If the `scheduleResolveOrReject` is called within the `finally` we won't clear unhandled rejections. We'll keep the same behaviour if the `scheduleResolveOrReject` is called within the `then`. PR Close #43206 PR Close #45449
1 parent fac6944 commit cb57cdb

2 files changed

Lines changed: 35 additions & 3 deletions

File tree

‎packages/zone.js/lib/common/promise.ts‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -249,8 +249,17 @@ Zone.__load_patch('ZoneAwarePromise', (global: any, Zone: ZoneType, api: _ZonePr
249249
function scheduleResolveOrReject<R, U1, U2>(
250250
promise: ZoneAwarePromise<any>, zone: Zone, chainPromise: ZoneAwarePromise<any>,
251251
onFulfilled?: ((value: R) => U1)|null|undefined,
252-
onRejected?: ((error: any) => U2)|null|undefined): void {
253-
clearRejectedNoCatch(promise);
252+
onRejected?: ((error: any) => U2)|null|undefined, scheduledByFinally = false): void {
253+
// Note: the native promise implementation logs unhandled promise rejections after `onFinally`
254+
// has been called. We may rely on the `scheduledByFinally` argument, which is `false` by
255+
// default, to avoid creating a breaking change; this will also allow us to reduce the number of
256+
// changes. The `finally` function should not silence unhandled promise rejections, because
257+
// they're not silenced in any implementation. If the `scheduleResolveOrReject` is called within
258+
// the `finally` we won't clear unhandled rejections. We'll keep the same behaviour if the
259+
// `scheduleResolveOrReject` is called within the `then`.
260+
if (!scheduledByFinally) {
261+
clearRejectedNoCatch(promise);
262+
}
254263
const promiseState = (promise as any)[symbolState];
255264
const delegate = promiseState ?
256265
(typeof onFulfilled === 'function') ? onFulfilled : forwardResolution :
@@ -506,7 +515,8 @@ Zone.__load_patch('ZoneAwarePromise', (global: any, Zone: ZoneType, api: _ZonePr
506515
if ((this as any)[symbolState] == UNRESOLVED) {
507516
(<any[]>(this as any)[symbolValue]).push(zone, chainPromise, onFinally, onFinally);
508517
} else {
509-
scheduleResolveOrReject(this, zone, chainPromise as any, onFinally, onFinally);
518+
scheduleResolveOrReject(
519+
this, zone, chainPromise as any, onFinally, onFinally, /* scheduledByFinally */ true);
510520
}
511521
return chainPromise;
512522
}

‎packages/zone.js/test/common/Promise.spec.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,28 @@ describe(
376376
});
377377
});
378378

379+
it('should not silence rejected promise errors after .finally has been called', done => {
380+
Zone.current.fork({name: 'promise-error'}).run(() => {
381+
(Zone as any)[Zone.__symbol__('ignoreConsoleErrorUncaughtError')] = false;
382+
const originalConsoleError = console.error;
383+
const spy = console.error = jasmine.createSpy('console.error');
384+
new Promise((resolve, reject) => {
385+
throw new Error('promise error');
386+
}).finally(() => {
387+
// Ensure that uncaught promise `console.error` is called after the `finally` is
388+
// called.
389+
expect(spy).not.toHaveBeenCalled();
390+
setTimeout(() => {
391+
const {args} = spy.calls.mostRecent();
392+
expect(args).toContain('Unhandled Promise rejection:');
393+
expect(args).toContain('promise error');
394+
console.error = originalConsoleError;
395+
done();
396+
}, 10);
397+
});
398+
});
399+
});
400+
379401
it('should not output error to console if ignoreConsoleErrorUncaughtError is true',
380402
(done) => {
381403
Zone.current.fork({name: 'promise-error'}).run(() => {

0 commit comments

Comments
 (0)