Skip to content
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

[HttpClient] fix throwing HTTP exceptions when the 1st chunk is emitted #33643

Merged
merged 1 commit into from Sep 20, 2019

Conversation

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Sep 19, 2019

Q A
Branch? 4.3
Bug fix? yes
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR -

Spotted while discussing the client with @Seldaek
The current behavior is transient: depending on the speed of the network/server, the exception can be thrown, or not.

This forces one do deal with 3/4/5xx when the first chunk is yielded.

@nicolas-grekas nicolas-grekas added this to the 4.3 milestone Sep 19, 2019
nicolas-grekas added a commit that referenced this pull request Sep 20, 2019
…nk is emitted (nicolas-grekas)

This PR was merged into the 4.3 branch.

Discussion
----------

[HttpClient] fix throwing HTTP exceptions when the 1st chunk is emitted

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Spotted while discussing the client with @Seldaek
The current behavior is transient: depending on the speed of the network/server, the exception can be thrown, or not.

This forces one do deal with 3/4/5xx when the first chunk is yielded.

Commits
-------

3c93764 [HttpClient] fix throwing HTTP exceptions when the 1st chunk is emitted
@nicolas-grekas nicolas-grekas merged commit 3c93764 into symfony:4.3 Sep 20, 2019
1 of 3 checks passed
1 of 3 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
fabbot.io Your code looks good.
Details
@@ -152,6 +152,16 @@ public function testClientError()
$this->assertSame(404, $response->getStatusCode());
$this->assertSame(['application/json'], $response->getHeaders(false)['content-type']);
$this->assertNotEmpty($response->getContent(false));
$response = $client->request('GET', 'http://localhost:8057/404');

This comment has been minimized.

Copy link
@stof

stof Sep 20, 2019

Member

could we create separate tests rather than a single test testClientError actually performing multiple subsequent tests ? This would allow each test name to describe what it is testing.

@nicolas-grekas nicolas-grekas deleted the nicolas-grekas:hc-transient-fix branch Sep 20, 2019
@fabpot fabpot mentioned this pull request Oct 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.