Some fixes for common bugs in HttpClient by alxhub · Pull Request #19958 · angular/angular

alxhub · 2017-10-26T18:14:16Z

No description provided.

mary-poppins · 2017-10-26T18:55:23Z

You can preview 62c7b98 at https://pr19958-62c7b98.ngbuilds.io/.

gkalpak

The 4th commit also closes PR #19917 btw.

gkalpak · 2017-10-26T19:05:47Z

packages/common/http/test/response_spec.ts

gkalpak · 2017-10-26T19:15:04Z

packages/common/http/src/xhr.ts

I wonder if it is worth preserving the XSSI_PREFIX in this case 🤔

Yes, done in a new commit.

mary-poppins · 2017-10-26T22:33:05Z

You can preview 45006a5 at https://pr19958-45006a5.ngbuilds.io/.

gkalpak

The 4th commit has incorrect scope (service-worker instead of common).

mary-poppins · 2017-11-02T20:28:08Z

You can preview 4fa1792 at https://pr19958-4fa1792.ngbuilds.io/.

quanterion · 2017-11-08T15:55:52Z

When is it going to be merged? Without those fixes it's impossible to use most HTTP DELETE requests that returns empty body

Maximaximum · 2017-11-15T10:55:34Z

Hopefully this PR will get released as soon as possible, because the empty body parse issue is a real show stopper.

dinerotah · 2017-11-16T15:42:30Z

When this fix will be released ???
I spent too much time to migrate to the new HttpClient, it will be a frustration to downgrade.................

valkoun · 2017-11-16T19:07:04Z

FYI, in the meantime you can choose one of 3 options to workaround this:

Make sure responses always return a response body with 200 responses, or
Make each response with no response body return a 204 No Content code, or
Set responseType: 'text' for each http method call:

return this.http.post('api/groups', group, {
      responseType: 'text',
});

Maximaximum · 2017-11-16T19:16:22Z

@valkoun yeah, thanks, but it's a whole lot of stupid error-prone work, so I hope this fix will get into release ASAP

dinerotah · 2017-11-20T09:56:20Z

Meanwhile I fixed the problem on the server side returning http status 204 for all void service method and so no more parsing error.

realshadow · 2017-11-20T14:10:24Z

Will this also get merged back to 4.4.6 or 7?

sougiovn · 2017-11-21T16:53:17Z

When will it be released?

vinayakpatil · 2017-11-25T06:54:57Z

@alxhub Will this be merged anytime soon?

alxhub · 2017-11-25T14:20:17Z

Yes, hopefully next week.

…

On Sat, Nov 25, 2017, 7:55 AM Vinayak Patil ***@***.***> wrote: @alxhub <https://github.com/alxhub> Will this be merged anytime soon? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#19958 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AD-1JYQglTdz-O3hJtlLgkfl3mY9hx6Uks5s57nzgaJpZM4QH_of> .

Previously, HttpClient used the overly clever test "body || null" to determine when a body parameter was provided. This breaks when the valid bodies '0' or 'false' are provided. This change tests directly against 'undefined' to detect the presence of the body parameter, and thus correctly allows falsy values through. Fixes angular#19825. Fixes angular#19195.

An invalid "if" condition is always true, and is thus useless. This change removes it. No behavior changes. Fixes angular#19223.

Previously, XhrBackend would call JSON.parse('') if the response body was empty (a 200 status code with content-length 0). This changes the XhrBackend to attempt the JSON parse only if the response body is non-empty. Otherwise, the body is left as null. Fixes angular#18680. Fixes angular#19413. Fixes angular#19502. Fixes angular#19555.

This changes XhrBackend to not strip the XSSI prefix from error text if such a prefix is present but the remaining body does not parse as JSON.

mary-poppins · 2017-11-28T16:26:28Z

You can preview 4a4cb9f at https://pr19958-4a4cb9f.ngbuilds.io/.

An invalid "if" condition is always true, and is thus useless. This change removes it. No behavior changes. Fixes #19223. PR Close #19958

…ent (#19958) Previously, XhrBackend would call JSON.parse('') if the response body was empty (a 200 status code with content-length 0). This changes the XhrBackend to attempt the JSON parse only if the response body is non-empty. Otherwise, the body is left as null. Fixes #18680. Fixes #19413. Fixes #19502. Fixes #19555. PR Close #19958

This changes XhrBackend to not strip the XSSI prefix from error text if such a prefix is present but the remaining body does not parse as JSON. PR Close #19958

hspier · 2017-11-29T18:21:38Z

Any chance we can get a 4.4.7 release with this fix soon?

Previously, HttpClient used the overly clever test "body || null" to determine when a body parameter was provided. This breaks when the valid bodies '0' or 'false' are provided. This change tests directly against 'undefined' to detect the presence of the body parameter, and thus correctly allows falsy values through. Fixes #19825. Fixes #19195. PR Close #19958

An invalid "if" condition is always true, and is thus useless. This change removes it. No behavior changes. Fixes #19223. PR Close #19958

…ent (#19958) Previously, XhrBackend would call JSON.parse('') if the response body was empty (a 200 status code with content-length 0). This changes the XhrBackend to attempt the JSON parse only if the response body is non-empty. Otherwise, the body is left as null. Fixes #18680. Fixes #19413. Fixes #19502. Fixes #19555. PR Close #19958

This changes XhrBackend to not strip the XSSI prefix from error text if such a prefix is present but the remaining body does not parse as JSON. PR Close #19958

Previously, HttpClient used the overly clever test "body || null" to determine when a body parameter was provided. This breaks when the valid bodies '0' or 'false' are provided. This change tests directly against 'undefined' to detect the presence of the body parameter, and thus correctly allows falsy values through. Fixes angular#19825. Fixes angular#19195. PR Close angular#19958

An invalid "if" condition is always true, and is thus useless. This change removes it. No behavior changes. Fixes angular#19223. PR Close angular#19958

…ent (angular#19958) Previously, XhrBackend would call JSON.parse('') if the response body was empty (a 200 status code with content-length 0). This changes the XhrBackend to attempt the JSON parse only if the response body is non-empty. Otherwise, the body is left as null. Fixes angular#18680. Fixes angular#19413. Fixes angular#19502. Fixes angular#19555. PR Close angular#19958

…#19958) This changes XhrBackend to not strip the XSSI prefix from error text if such a prefix is present but the remaining body does not parse as JSON. PR Close angular#19958

angular-automatic-lock-bot · 2019-09-12T22:51:33Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

gkalpak reviewed Oct 26, 2017

View reviewed changes

alxhub force-pushed the http-fixes branch from 62c7b98 to f657486 Compare October 26, 2017 22:20

googlebot added the cla: yes label Oct 26, 2017

alxhub force-pushed the http-fixes branch from f657486 to 45006a5 Compare October 26, 2017 22:22

gkalpak approved these changes Oct 27, 2017

View reviewed changes

gkalpak added area: common/http Issues related to HTTP and HTTP Client action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews PR target: TBD labels Oct 31, 2017

gkalpak reviewed Oct 31, 2017

View reviewed changes

gkalpak mentioned this pull request Oct 31, 2017

fix(common): strip XSSI prefix for all JSON responses #19917

Closed

3 tasks

alxhub force-pushed the http-fixes branch 2 times, most recently from 9b5c135 to 4fa1792 Compare November 2, 2017 20:08

trotyl mentioned this pull request Nov 7, 2017

fix(common): do not replace zero/empty string with null request body … #19303

Closed

3 tasks

kevinkjt2000 mentioned this pull request Nov 14, 2017

HttpClient fails to parse an empty 200 response in IE11 #18680

Closed

8 tasks

Maximaximum mentioned this pull request Nov 15, 2017

Angular 4 or 5? souldreamer/node-angular-http-client#2

Open

karptonite mentioned this pull request Nov 21, 2017

HttpClient tries and fails to parse the non-existent body of a HEAD request as JSON #19090

Closed

Toxicable mentioned this pull request Nov 23, 2017

fix(core/http): allow falsy primitive values in HttpResponse #20597

Closed

alxhub added 4 commits November 28, 2017 08:12

fix(common): remove useless guard in HttpClient

70c0764

An invalid "if" condition is always true, and is thus useless. This change removes it. No behavior changes. Fixes angular#19223.

fix(common): don't strip XSSI prefix for if error isn't JSON

4a4cb9f

This changes XhrBackend to not strip the XSSI prefix from error text if such a prefix is present but the remaining body does not parse as JSON.

alxhub force-pushed the http-fixes branch from 4fa1792 to 4a4cb9f Compare November 28, 2017 16:15

vikerman approved these changes Nov 28, 2017

View reviewed changes

alxhub added target: patch This PR is targeted for the next patch release action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews PR target: TBD labels Nov 28, 2017

mhevery closed this in 15a54df Nov 29, 2017

mhevery pushed a commit that referenced this pull request Nov 29, 2017

fix(common): remove useless guard in HttpClient (#19958)

eb01ad5

An invalid "if" condition is always true, and is thus useless. This change removes it. No behavior changes. Fixes #19223. PR Close #19958

mhevery pushed a commit that referenced this pull request Nov 29, 2017

fix(common): remove useless guard in HttpClient (#19958)

e099911

An invalid "if" condition is always true, and is thus useless. This change removes it. No behavior changes. Fixes #19223. PR Close #19958

trotyl mentioned this pull request Feb 19, 2018

Http clinet response with empty body #20879

Closed

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 12, 2019

Conversation

alxhub commented Oct 26, 2017

Uh oh!

mary-poppins commented Oct 26, 2017

Uh oh!

gkalpak left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gkalpak Oct 26, 2017

Choose a reason for hiding this comment

Uh oh!

alxhub Oct 26, 2017

Choose a reason for hiding this comment

Uh oh!

gkalpak Oct 26, 2017

Choose a reason for hiding this comment

Uh oh!

alxhub Oct 26, 2017

Choose a reason for hiding this comment

Uh oh!

mary-poppins commented Oct 26, 2017

Uh oh!

gkalpak left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mary-poppins commented Nov 2, 2017

Uh oh!

quanterion commented Nov 8, 2017

Uh oh!

Maximaximum commented Nov 15, 2017

Uh oh!

dinerotah commented Nov 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

valkoun commented Nov 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Maximaximum commented Nov 16, 2017

Uh oh!

dinerotah commented Nov 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

realshadow commented Nov 20, 2017

Uh oh!

sougiovn commented Nov 21, 2017

Uh oh!

vinayakpatil commented Nov 25, 2017

Uh oh!

alxhub commented Nov 25, 2017 via email

Uh oh!

mary-poppins commented Nov 28, 2017

Uh oh!

hspier commented Nov 29, 2017

Uh oh!

angular-automatic-lock-bot bot commented Sep 12, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

gkalpak left a comment •

edited

Loading

gkalpak left a comment •

edited

Loading

dinerotah commented Nov 16, 2017 •

edited

Loading

valkoun commented Nov 16, 2017 •

edited

Loading

dinerotah commented Nov 20, 2017 •

edited

Loading