From c3614662cb0ae5f93eb4d77f97f68fbba3cc360e Mon Sep 17 00:00:00 2001 From: JiaLiPassion Date: Sun, 11 Apr 2021 14:58:00 +0800 Subject: [PATCH] test(zone.js): should invoke XHR task even onload handler throw error. (#41562) Close #41520. This case related to the issue #41522. ``` Zone.root .fork({ name: 'xhr', onHasTask(delegate, currentZone, zone, taskState) { console.log('hasMacrotask', taskState.macroTask); return delegate.hasTask(zone, taskState); }, }) .run(() => { const xhr = new XMLHttpRequest(); xhr.open('GET', 'https://cdnjs.cloudflare.com/ajax/libs/zone.js/0.11.4/zone.min.js'); xhr.addEventListener('load', () => { throw new Error(); }); xhr.send(); }); ``` zone.js invoke all `onload` event handlers before change the XHR task's state from `scheduled` to `notscheduled`, so if any `onload` listener throw error, the XHR task wlll be hang to `scheduled`, and leave the macroTask status in the zone wrongly. This has been fixed in the previous commit, this commit add test to verify the case. PR Close #41562 --- .../test/browser/XMLHttpRequest.spec.ts | 78 +++++++++ packages/zone.js/test/browser/browser.spec.ts | 161 ++++++++++-------- 2 files changed, 167 insertions(+), 72 deletions(-) diff --git a/packages/zone.js/test/browser/XMLHttpRequest.spec.ts b/packages/zone.js/test/browser/XMLHttpRequest.spec.ts index da23b072ff..ee370c31dc 100644 --- a/packages/zone.js/test/browser/XMLHttpRequest.spec.ts +++ b/packages/zone.js/test/browser/XMLHttpRequest.spec.ts @@ -103,6 +103,84 @@ describe('XMLHttpRequest', function() { req!.send(); }); + it('should run onload listeners before internal readystatechange', function(done) { + const logs: string[] = []; + const xhrZone = Zone.current.fork({ + name: 'xhr', + onInvokeTask: (delegate, curr, target, task, applyThis, applyArgs) => { + logs.push('invokeTask ' + task.source); + return delegate.invokeTask(target, task, applyThis, applyArgs); + } + }); + + xhrZone.run(function() { + const req = new XMLHttpRequest(); + req.onload = function() { + logs.push('onload'); + (window as any)[Zone.__symbol__('setTimeout')](() => { + expect(logs).toEqual([ + 'invokeTask XMLHttpRequest.addEventListener:load', 'onload', + 'invokeTask XMLHttpRequest.send' + ]) + done(); + }); + }; + req.open('get', '/', true); + req.send(); + }); + }); + + it('should invoke xhr task even onload listener throw error', function(done) { + const oriWindowError = window.onerror; + window.onerror = function() {}; + try { + const logs: string[] = []; + const xhrZone = Zone.current.fork({ + name: 'xhr', + onInvokeTask: (delegate, curr, target, task, applyThis, applyArgs) => { + logs.push('invokeTask ' + task.source); + return delegate.invokeTask(target, task, applyThis, applyArgs); + }, + onHasTask: (delegate, curr, target, hasTaskState) => { + if (hasTaskState.change === 'macroTask') { + logs.push('hasTask ' + hasTaskState.macroTask); + } + return delegate.hasTask(target, hasTaskState); + } + }); + + xhrZone.run(function() { + const req = new XMLHttpRequest(); + req.onload = function() { + logs.push('onload'); + throw new Error('test'); + }; + const unhandledRejection = (e: PromiseRejectionEvent) => { + logs.push(e.reason.message); + }; + window.addEventListener('unhandledrejection', unhandledRejection); + req.addEventListener('load', () => { + logs.push('onload1'); + (window as any)[Zone.__symbol__('setTimeout')](() => { + expect(logs).toEqual([ + 'hasTask true', 'invokeTask XMLHttpRequest.addEventListener:load', 'onload', + 'invokeTask XMLHttpRequest.addEventListener:load', 'onload1', + 'invokeTask XMLHttpRequest.send', 'hasTask false', + 'invokeTask Window.addEventListener:unhandledrejection', 'test' + ]); + window.removeEventListener('unhandledrejection', unhandledRejection); + window.onerror = oriWindowError; + done(); + }); + }); + req.open('get', '/', true); + req.send(); + }); + } catch (e: any) { + window.onerror = oriWindowError; + } + }); + it('should return null when access ontimeout first time without error', function() { let req: XMLHttpRequest = new XMLHttpRequest(); expect(req.ontimeout).toBe(null); diff --git a/packages/zone.js/test/browser/browser.spec.ts b/packages/zone.js/test/browser/browser.spec.ts index 7eea07815f..3e7c6f9c24 100644 --- a/packages/zone.js/test/browser/browser.spec.ts +++ b/packages/zone.js/test/browser/browser.spec.ts @@ -2493,92 +2493,109 @@ describe('Zone', function() { it('should be able to continue to invoke remaining listeners even some listener throw error', function(done: DoneFn) { - let logs: string[] = []; - const listener1 = function() { - logs.push('listener1'); - }; - const listener2 = function() { - throw new Error('test1'); - }; - const listener3 = function() { - throw new Error('test2'); - }; - const listener4 = { - handleEvent: function() { - logs.push('listener2'); - } - }; + // override global.onerror to prevent jasmine report error + let oriWindowOnError = window.onerror; + window.onerror = function() {}; + try { + let logs: string[] = []; + const listener1 = function() { + logs.push('listener1'); + }; + const listener2 = function() { + throw new Error('test1'); + }; + const listener3 = function() { + throw new Error('test2'); + }; + const listener4 = { + handleEvent: function() { + logs.push('listener2'); + } + }; - button.addEventListener('click', listener1); - button.addEventListener('click', listener2); - button.addEventListener('click', listener3); - button.addEventListener('click', listener4); + button.addEventListener('click', listener1); + button.addEventListener('click', listener2); + button.addEventListener('click', listener3); + button.addEventListener('click', listener4); - const mouseEvent = document.createEvent('MouseEvent'); - mouseEvent.initEvent('click', true, true); + const mouseEvent = document.createEvent('MouseEvent'); + mouseEvent.initEvent('click', true, true); - const unhandledRejection = (e: PromiseRejectionEvent) => { - logs.push(e.reason.message); - }; - window.addEventListener('unhandledrejection', unhandledRejection); + const unhandledRejection = (e: PromiseRejectionEvent) => { + logs.push(e.reason.message); + }; + window.addEventListener('unhandledrejection', unhandledRejection); - button.dispatchEvent(mouseEvent); - expect(logs).toEqual(['listener1', 'listener2']); + button.dispatchEvent(mouseEvent); + expect(logs).toEqual(['listener1', 'listener2']); - setTimeout(() => { - expect(logs).toEqual(['listener1', 'listener2', 'test1', 'test2']); - window.removeEventListener('unhandledrejection', unhandledRejection); - done() - }); + setTimeout(() => { + expect(logs).toEqual(['listener1', 'listener2', 'test1', 'test2']); + window.removeEventListener('unhandledrejection', unhandledRejection); + window.onerror = oriWindowOnError; + done() + }); + } catch (e: any) { + window.onerror = oriWindowOnError; + } }); it('should be able to continue to invoke remaining listeners even some listener throw error in the different zones', function(done: DoneFn) { - let logs: string[] = []; - const zone1 = Zone.current.fork({ - name: 'zone1', - onHandleError: (delegate, curr, target, error) => { - logs.push(error.message); - return false; - } - }); - const listener1 = function() { - logs.push('listener1'); - }; - const listener2 = function() { - throw new Error('test1'); - }; - const listener3 = function() { - throw new Error('test2'); - }; - const listener4 = { - handleEvent: function() { - logs.push('listener2'); - } - }; + // override global.onerror to prevent jasmine report error + let oriWindowOnError = window.onerror; + window.onerror = function() {}; + try { + let logs: string[] = []; + const zone1 = Zone.current.fork({ + name: 'zone1', + onHandleError: (delegate, curr, target, error) => { + logs.push(error.message); + return false; + } + }); + const listener1 = function() { + logs.push('listener1'); + }; + const listener2 = function() { + throw new Error('test1'); + }; + const listener3 = function() { + throw new Error('test2'); + }; + const listener4 = { + handleEvent: function() { + logs.push('listener2'); + } + }; - button.addEventListener('click', listener1); - zone1.run(() => { - button.addEventListener('click', listener2); - }); - button.addEventListener('click', listener3); - button.addEventListener('click', listener4); + button.addEventListener('click', listener1); + zone1.run(() => { + button.addEventListener('click', listener2); + }); + button.addEventListener('click', listener3); + button.addEventListener('click', listener4); - const mouseEvent = document.createEvent('MouseEvent'); - mouseEvent.initEvent('click', true, true); + const mouseEvent = document.createEvent('MouseEvent'); + mouseEvent.initEvent('click', true, true); - const unhandledRejection = (e: PromiseRejectionEvent) => { - logs.push(e.reason.message); - }; - window.addEventListener('unhandledrejection', unhandledRejection); + const unhandledRejection = (e: PromiseRejectionEvent) => { + logs.push(e.reason.message); + }; + window.addEventListener('unhandledrejection', unhandledRejection); - button.dispatchEvent(mouseEvent); - expect(logs).toEqual(['listener1', 'test1', 'listener2']); + button.dispatchEvent(mouseEvent); + expect(logs).toEqual(['listener1', 'test1', 'listener2']); - setTimeout(() => { - expect(logs).toEqual(['listener1', 'test1', 'listener2', 'test2']); - done() - }); + setTimeout(() => { + expect(logs).toEqual(['listener1', 'test1', 'listener2', 'test2']); + window.removeEventListener('unhandledrejection', unhandledRejection); + window.onerror = oriWindowOnError; + done() + }); + } catch (e: any) { + window.onerror = oriWindowOnError; + } }); });