Skip to content

Commit

Permalink
fix(zone.js): should invoke xhr send task when no response error occu…
Browse files Browse the repository at this point in the history
…rs (#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
  • Loading branch information
JiaLiPassion authored and mhevery committed Sep 18, 2020
1 parent fb163df commit 33055da
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
6 changes: 5 additions & 1 deletion packages/zone.js/lib/browser/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 24 additions & 0 deletions packages/zone.js/test/browser/XMLHttpRequest.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 33055da

Please sign in to comment.