New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(http): correctly handle response body for 204 status code #12830
Conversation
367c741
to
b49603c
Compare
@@ -239,7 +239,7 @@ export declare class XHRConnection implements Connection { | |||
request: Request; | |||
response: Observable<Response>; | |||
constructor(req: Request, browserXHR: BrowserXhr, baseResponseOptions?: ResponseOptions); | |||
setDetectedContentType(req: any, _xhr: any): void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please refrain from changing the types - add a // TODO: ...
in the code (and open a meta issue "[3.0] Adding/fixing types" referencing the place where we should update). Thanks !
@@ -5,15 +5,13 @@ | |||
* Use of this source code is governed by an MIT-style license that can be | |||
* found in the LICENSE file at https://angular.io/license | |||
*/ | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you please leave a blank line here
b49603c
to
1b5c7bf
Compare
@@ -137,7 +148,7 @@ export class XHRConnection implements Connection { | |||
}); | |||
} | |||
|
|||
setDetectedContentType(req: any /** TODO #9100 */, _xhr: XMLHttpRequest) { | |||
setDetectedContentType(req: any /** TODO #9100 */, _xhr: any /** TODO #9100 */) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why ?
async.done(); | ||
}); | ||
|
||
existingXHRs[0].setStatusCode(statusCode); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would this have failed before ?
should you add an explicit body ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thx
// normalize IE9 bug (http://bugs.jquery.com/ticket/1450) | ||
let status: number = _xhr.status === 1223 ? 204 : _xhr.status; | ||
|
||
let body: any = null; | ||
|
||
// 204 is not supposed to have any response body |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: change to // HTTP 204 means no content
31a4c12
to
17d354a
Compare
17d354a
to
493c1b2
Compare
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Fixes #12393