From d92a0dd72f6adf3c8a145255bb1e08346780418e Mon Sep 17 00:00:00 2001 From: JiaLiPassion Date: Mon, 14 Sep 2020 15:54:12 +0900 Subject: [PATCH] fix(zone.js): should invoke xhr send task when no response error occurs (#38836) Close #38795 in the XMLHttpRequest patch, when get `readystatechange` event, zone.js try to invoke `load` event listener first, then call `invokeTask` to finish the `XMLHttpRequest::send` macroTask, but if the request failed because the server can not be reached, the `load` event listener will not be invoked, so the `invokeTask` of the `XMLHttpRequest::send` will not be triggered either, so we will have a non finished macroTask there which will make the Zone not stable, also memory leak. So in this PR, if the `XMLHttpRequest.status = 0` when we get the `readystatechange` event, that means something wents wrong before we reached the server, we need to invoke the task to finish the macroTask. PR Close #38836 --- packages/zone.js/lib/browser/browser.ts | 6 ++++- .../test/browser/XMLHttpRequest.spec.ts | 24 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/packages/zone.js/lib/browser/browser.ts b/packages/zone.js/lib/browser/browser.ts index 3023e1941d..bf6c419528 100644 --- a/packages/zone.js/lib/browser/browser.ts +++ b/packages/zone.js/lib/browser/browser.ts @@ -149,8 +149,12 @@ Zone.__load_patch('XHR', (global: any, Zone: ZoneType) => { // check whether the xhr has registered onload listener // if that is the case, the task should invoke after all // onload listeners finish. + // Also if the request failed without response (status = 0), the load event handler + // will not be triggered, in that case, we should also invoke the placeholder callback + // to close the XMLHttpRequest::send macroTask. + // https://github.com/angular/angular/issues/38795 const loadTasks = target[Zone.__symbol__('loadfalse')]; - if (loadTasks && loadTasks.length > 0) { + if (target.status !== 0 && loadTasks && loadTasks.length > 0) { const oriInvoke = task.invoke; task.invoke = function() { // need to load the tasks again, because in other diff --git a/packages/zone.js/test/browser/XMLHttpRequest.spec.ts b/packages/zone.js/test/browser/XMLHttpRequest.spec.ts index 3152c8dffe..da23b072ff 100644 --- a/packages/zone.js/test/browser/XMLHttpRequest.spec.ts +++ b/packages/zone.js/test/browser/XMLHttpRequest.spec.ts @@ -268,6 +268,30 @@ describe('XMLHttpRequest', function() { }); }); + it('should close xhr request if error happened when connect', function(done) { + const logs: boolean[] = []; + Zone.current + .fork({ + name: 'xhr', + onHasTask: + (delegate: ZoneDelegate, curr: Zone, target: Zone, taskState: HasTaskState) => { + if (taskState.change === 'macroTask') { + logs.push(taskState.macroTask); + } + return delegate.hasTask(target, taskState); + } + }) + .run(function() { + const req = new XMLHttpRequest(); + req.open('get', 'http://notexists.url', true); + req.send(); + req.addEventListener('error', () => { + expect(logs).toEqual([true, false]); + done(); + }); + }); + }); + it('should trigger readystatechange if xhr request trigger cors error', (done) => { const req = new XMLHttpRequest(); let err: any = null;