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(common): parse error response body for responseType "json" #19477

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
5 participants
@dherges
Contributor

dherges commented Sep 29, 2017

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

For req.responseType = "json", a non-2xx response body is always "text"

Issue Number: #18682

What is the new behavior?

For req.responseType = "json", response body is JSON.parse()'d

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@googlebot googlebot added the cla: yes label Sep 29, 2017

@dherges dherges changed the title from fix(common/http): parse error response body for responseType "json" to fix(common): parse error response body for responseType "json" Sep 29, 2017

@dherges

This comment has been minimized.

Show comment
Hide comment
@dherges

dherges Sep 29, 2017

Contributor

I accidentally duplicated #19378

Related issues are #19377 and #19103

Related code change introducing the issues seems to be 452a7ae (PR #18466)

Contributor

dherges commented Sep 29, 2017

I accidentally duplicated #19378

Related issues are #19377 and #19103

Related code change introducing the issues seems to be 452a7ae (PR #18466)

@ashgibson

This comment has been minimized.

Show comment
Hide comment
@ashgibson

ashgibson Oct 11, 2017

Is this going to be merged or the issue that it fixes updated soon? My app is full of forms which have server side validation that returns a 422 with json errors object. I then use this object to display field errors and this is now broken. I can fix by adding some logic to my interceptor to manually parse the json but not going to bother if the breaking change that was brought in 4.4.4 will be reversed in 4.4.5.

ashgibson commented Oct 11, 2017

Is this going to be merged or the issue that it fixes updated soon? My app is full of forms which have server side validation that returns a 422 with json errors object. I then use this object to display field errors and this is now broken. I can fix by adding some logic to my interceptor to manually parse the json but not going to bother if the breaking change that was brought in 4.4.4 will be reversed in 4.4.5.

@Linskeyd

This comment has been minimized.

Show comment
Hide comment
@Linskeyd

Linskeyd Oct 15, 2017

Contributor

@dherges you may want to update this comment as it seems to still assume that only 'ok' results are being processed: https://github.com/angular/angular/pull/19477/files#diff-26683cfb34fa97883213282055aef1e7R189

Additionally, you may want to add a test which explicitly checks for XSSI prefix support in the error response: https://gist.github.com/Linskeyd/df32564d539862350e927721ffad86df

Contributor

Linskeyd commented Oct 15, 2017

@dherges you may want to update this comment as it seems to still assume that only 'ok' results are being processed: https://github.com/angular/angular/pull/19477/files#diff-26683cfb34fa97883213282055aef1e7R189

Additionally, you may want to add a test which explicitly checks for XSSI prefix support in the error response: https://gist.github.com/Linskeyd/df32564d539862350e927721ffad86df

@dherges

This comment has been minimized.

Show comment
Hide comment
@dherges

dherges Oct 19, 2017

Contributor

Is it fixed by #19773 ?

Contributor

dherges commented Oct 19, 2017

Is it fixed by #19773 ?

@dherges dherges closed this Oct 19, 2017

@dherges dherges reopened this Oct 20, 2017

fix(common): parse error response body for "json" responseType
Since XSSI-prefixes, `xhr.responseType` is set to "text", thus skipping browser's built-in JSON parsing. `HttpXhrBackend` now tries to parse a JSON error response regardless of the HTTP status code.

Resolves #18682

@dherges dherges closed this Nov 20, 2017

@dherges dherges deleted the dherges:patch-3 branch Nov 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment