8000 fix(ajax): Handle timeouts as errors (#3653) · ReactiveX/rxjs@e4128ea · GitHub
[go: up one dir, main page]
Skip to content

Commit e4128ea

Browse files
bbonnetbenlesh
bbonnet
authored and
benlesh
committed
fix(ajax): Handle timeouts as errors (#3653)
* fix(ajax): Handle timeouts as errors * fix(ajax): Use TestScheduler in timeout test
1 parent c9acc61 commit e4128ea

File tree

2 files changed

+62
-31
lines changed

2 files changed

+62
-31
lines changed

‎spec/observables/dom/ajax-spec.ts

Lines changed: 49 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ import { expect } from 'chai';
22
import * as sinon from 'sinon';
33
import * as Rx from 'rxjs/Rx';
44
import { root } from 'rxjs/util/root';
5+
import { TestScheduler } from 'rxjs/testing';
56

67
declare const global: any;
8+
declare const rxTestScheduler: TestScheduler;
79

810
/** @test {ajax} */
911
describe('Observable.ajax', () => {
@@ -381,6 +383,39 @@ describe('Observable.ajax', () => {
381383
});
382384
});
383385

386+
it('should error on timeout of asynchronous request', () => {
387+
const obj: Rx.AjaxRequest = {
388+
url: '/flibbertyJibbet',
389+
responseType: 'text',
390+
timeout: 10
391+
};
392+
393+
Rx.Observable.ajax(obj)
394+
.subscribe((x: any) => {
395+
throw 'should not have been called';
396+
}, (e) => {
397+
expect(e.status).to.equal(0);
398+
expect(e.xhr.method).to.equal('GET');
399+
expect(e.xhr.async).to.equal(true);
400+
expect(e.xhr.timeout).to.equal(10);
401+
expect(e.xhr.responseType).to.equal('text');
402+
});
403+
404+
const request = MockXMLHttpRequest.mostRecent;
405+
406+
expect(request.url).to.equal('/flibbertyJibbet');
407+
408+
rxTestScheduler.schedule(() => {
409+
request.respondWith({
410+
'status': 200,
411+
'contentType': 'text/plain',
412+
'responseText': 'Wee! I am text!'
413+
});
414+
}, 1000);
415+
416+
rxTestScheduler.flush();
417+
});
418+
384419
it('should create a synchronous request', () => {
385420
const obj: Rx.AjaxRequest = {
386421
url: '/flibbertyJibbet',
@@ -1030,8 +1065,20 @@ class MockXMLHttpRequest {
10301065
MockXMLHttpRequest.requests.push(this);
10311066
}
10321067

1068+
timeout: number;
1069+
10331070
send(data: any): void {
10341071
this.data = data;
1072+
if (this.timeout && this.timeout > 0) {
1073+
setTimeout(() => {
1074+
if (this.readyState != 4) {
1075+
this.readyState = 4;
1076+
this.status = 0;
1077+
this.triggerEvent('readystatechange');
1078+
this.triggerEvent('timeout');
1079+
}
1080+
}, this.timeout);
1081+
}
10351082
}
10361083

10371084
open(method: any, url: any, async: any, user: any, password: any): void {
@@ -1041,7 +1088,7 @@ class MockXMLHttpRequest {
10411088
this.user = user;
10421089
this.password = password;
10431090
this.readyState = 1;
1044-
this.triggerEvent('readyStateChange');
1091+
this.triggerEvent('readystatechange');
10451092
const originalProgressHandler = this.upload.onprogress;
10461093
Object.defineProperty(this.upload, 'progress', {
10471094
get() {
@@ -1054,24 +1101,6 @@ class MockXMLHttpRequest {
10541101
this.requestHeaders[key] = value;
10551102
}
10561103

1057-
addEventListener(name: string, handler: any): void {
1058-
this.eventHandlers.push({ name: name, handler: handler });
1059-
}
1060-
1061-
removeEventListener(name: string, handler: any): void {
1062-
for (let i = this.eventHandlers.length - 1; i--; ) {
1063-
let eh = this.eventHandlers[i];
1064-
if (eh.name === name && eh.handler === handler) {
1065-
this.eventHandlers.splice(i, 1);
1066-
}
1067-
}
1068-
}
1069-
1070-
throwError(err: any): void {
1071-
// TODO: something better with errors
1072-
this.triggerEvent('error');
1073-
}
1074-
10751104
protected jsonResponseValue(response: any) {
10761105
try {
10771106
this.response = JSON.parse(response.responseText);
@@ -1119,17 +1148,11 @@ class MockXMLHttpRequest {
11191148

11201149
triggerEvent(name: any, eventObj?: any): void {
11211150
// TODO: create a better default event
1122-
const e: any = eventObj || {};
1151+
const e: any = eventObj || { type: name };
11231152

11241153
if (this['on' + name]) {
11251154
this['on' + name](e);
11261155
}
1127-
1128-
this.eventHandlers.forEach(function (eh) {
1129-
if (eh.name === name) {
1130-
eh.handler.call(this, e);
1131-
}
1132-
});
11331156
}
11341157

11351158
triggerUploadEvent(name: any, eventObj?: any): void {

‎src/internal/observable/dom/AjaxObservable.ts

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,15 @@ export class AjaxSubscriber<T> extends Subscriber<Event> {
353353
}
354354

355355
function xhrReadyStateChange(this: XMLHttpRequest, e: Event) {
356-
const { subscriber, progressSubscriber, request } = (<any>xhrReadyStateChange);
356+
return;
357+
}
358+
xhr.onreadystatechange = xhrReadyStateChange;
359+
(<any>xhrReadyStateChange).subscriber = this;
360+
(<any>xhrReadyStateChange).progressSubscriber = progressSubscriber;
361+
(<any>xhrReadyStateChange).request = request;
362+
363+
function xhrLoad(this: XMLHttpRequest, e: Event) {
364+
const { subscriber, progressSubscriber, request } = (<any>xhrLoad);
357365
if (this.readyState === 4) {
358366
// normalize IE9 bug (http://bugs.jquery.com/ticket/1450)
359367
let status: number = this.status === 1223 ? 204 : this.status;
@@ -382,10 +390,10 @@ export class AjaxSubscriber<T> extends Subscriber<Event> {
382390
}
383391
}
384392
}
385-
xhr.onreadystatechange = xhrReadyStateChange;
386-
(<any>xhrReadyStateChange).subscriber = this;
387-
(<any>xhrReadyStateChange).progressSubscriber = progressSubscriber;
388-
(<any>xhrReadyStateChange).request = request;
393+
xhr.onload = xhrLoad;
394+
(<any>xhrLoad).subscriber = this;
395+
(<any>xhrLoad).progressSubscriber = progressSubscriber;
396+
(<any>xhrLoad).request = request;
389397
}
390398

391399
unsubscribe() {

0 commit comments

Comments
 (0)
0