Skip to content

Commit 201f189

Browse files
robwormaldrobwormald
committed
fix(http): error on non-200 status codes
BREAKING CHANGE: previously http would only error on network errors to match the fetch specification. Now status codes less than 200 and greater than 299 will cause Http's Observable to error. Closes #5130.
1 parent a35a93d commit 201f189

3 files changed

Lines changed: 67 additions & 13 deletions

File tree

‎modules/angular2/src/http/backends/xhr_backend.ts‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {Injectable} from 'angular2/angular2';
77
import {BrowserXhr} from './browser_xhr';
88
import {isPresent} from 'angular2/src/facade/lang';
99
import {Observable} from 'angular2/angular2';
10+
import {isSuccess} from '../http_utils';
1011
/**
1112
* Creates connections using `XMLHttpRequest`. Given a fully-qualified
1213
* request, an `XHRConnection` will immediately create an `XMLHttpRequest` object and send the
@@ -33,24 +34,30 @@ export class XHRConnection implements Connection {
3334
// responseText is the old-school way of retrieving response (supported by IE8 & 9)
3435
// response/responseType properties were introduced in XHR Level2 spec (supported by
3536
// IE10)
36-
let response = isPresent(_xhr.response) ? _xhr.response : _xhr.responseText;
37+
let xhrResponse = isPresent(_xhr.response) ? _xhr.response : _xhr.responseText;
38+
3739

3840
// normalize IE9 bug (http://bugs.jquery.com/ticket/1450)
39-
let status = _xhr.status === 1223 ? 204 : _xhr.status;
41+
let status: number = _xhr.status === 1223 ? 204 : _xhr.status;
4042

4143
// fix status code when it is 0 (0 status is undocumented).
4244
// Occurs when accessing file resources or on Android 4.1 stock browser
4345
// while retrieving files from application cache.
4446
if (status === 0) {
45-
status = response ? 200 : 0;
47+
status = xhrResponse ? 200 : 0;
4648
}
47-
var responseOptions = new ResponseOptions({body: response, status: status});
49+
var responseOptions = new ResponseOptions({body: xhrResponse, status: status});
4850
if (isPresent(baseResponseOptions)) {
4951
responseOptions = baseResponseOptions.merge(responseOptions);
5052
}
51-
responseObserver.next(new Response(responseOptions));
52-
// TODO(gdi2290): defer complete if array buffer until done
53-
responseObserver.complete();
53+
let response = new Response(responseOptions);
54+
if (isSuccess(status)) {
55+
responseObserver.next(response);
56+
// TODO(gdi2290): defer complete if array buffer until done
57+
responseObserver.complete();
58+
return;
59+
}
60+
responseObserver.error(response);
5461
};
5562
// error event handler
5663
let onError = (err) => {

‎modules/angular2/src/http/http_utils.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {isString} from 'angular2/src/facade/lang';
22
import {RequestMethods} from './enums';
33
import {makeTypeError} from 'angular2/src/facade/exceptions';
4+
import {Response} from './static_response';
45

56
export function normalizeMethodName(method): RequestMethods {
67
if (isString(method)) {
@@ -14,4 +15,6 @@ export function normalizeMethodName(method): RequestMethods {
1415
return method;
1516
}
1617

18+
export const isSuccess = (status: number): boolean => (status >= 200 && status < 300);
19+
1720
export {isJsObject} from 'angular2/src/facade/lang';

‎modules/angular2/test/http/backends/xhr_backend_spec.ts‎

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ export function main() {
9999
expect(res.type).toBe(ResponseTypes.Error);
100100
async.done();
101101
});
102+
existingXHRs[0].setStatusCode(200);
102103
existingXHRs[0].dispatchEvent('load');
103104
}));
104105

@@ -107,7 +108,7 @@ export function main() {
107108
new ResponseOptions({type: ResponseTypes.Error}));
108109
connection.response.subscribe(res => { expect(res.type).toBe(ResponseTypes.Error); },
109110
null, () => { async.done(); });
110-
111+
existingXHRs[0].setStatusCode(200);
111112
existingXHRs[0].dispatchEvent('load');
112113
}));
113114

@@ -164,15 +165,57 @@ export function main() {
164165
var connection = new XHRConnection(sampleRequest, new MockBrowserXHR(),
165166
new ResponseOptions({status: statusCode}));
166167

167-
connection.response.subscribe(res => {
168-
expect(res.status).toBe(statusCode);
169-
async.done();
170-
});
168+
connection.response.subscribe(
169+
res => {
170+
171+
},
172+
errRes => {
173+
expect(errRes.status).toBe(statusCode);
174+
async.done();
175+
});
176+
177+
existingXHRs[0].setStatusCode(statusCode);
178+
existingXHRs[0].dispatchEvent('load');
179+
}));
180+
181+
it('should call next and complete on 200 codes', inject([AsyncTestCompleter], async => {
182+
var nextCalled = false;
183+
var errorCalled = false;
184+
var statusCode = 200;
185+
var connection = new XHRConnection(sampleRequest, new MockBrowserXHR(),
186+
new ResponseOptions({status: statusCode}));
187+
188+
connection.response.subscribe(
189+
res => {
190+
nextCalled = true;
191+
expect(res.status).toBe(statusCode);
192+
},
193+
errRes => { errorCalled = true; }, () => {
194+
expect(nextCalled).toBe(true);
195+
expect(errorCalled).toBe(false);
196+
async.done();
197+
});
171198

172199
existingXHRs[0].setStatusCode(statusCode);
173200
existingXHRs[0].dispatchEvent('load');
174201
}));
175202

203+
it('should call error and not complete on 300+ codes', inject([AsyncTestCompleter], async => {
204+
var nextCalled = false;
205+
var errorCalled = false;
206+
var statusCode = 301;
207+
var connection = new XHRConnection(sampleRequest, new MockBrowserXHR(),
208+
new ResponseOptions({status: statusCode}));
209+
210+
connection.response.subscribe(res => { nextCalled = true; }, errRes => {
211+
expect(errRes.status).toBe(statusCode);
212+
expect(nextCalled).toBe(false);
213+
async.done();
214+
}, () => { throw 'should not be called'; });
215+
216+
existingXHRs[0].setStatusCode(statusCode);
217+
existingXHRs[0].dispatchEvent('load');
218+
}));
176219
it('should normalize IE\'s 1223 status code into 204', inject([AsyncTestCompleter], async => {
177220
var statusCode = 1223;
178221
var normalizedCode = 204;
@@ -204,10 +247,11 @@ export function main() {
204247
expect(ress.text()).toBe(responseBody);
205248
async.done();
206249
});
250+
existingXHRs[1].setStatusCode(200);
207251
existingXHRs[1].setResponse(responseBody);
208252
existingXHRs[1].dispatchEvent('load');
209253
});
210-
254+
existingXHRs[0].setStatusCode(200);
211255
existingXHRs[0].setResponseText(responseBody);
212256
existingXHRs[0].dispatchEvent('load');
213257
}));

0 commit comments

Comments
 (0)