Skip to content

Conversation

@nicolas-grekas
Copy link
Member

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
$this->assertSame(['application/json'], $response->getHeaders(false)['content-type']);
$this->assertNotEmpty($response->getContent(false));

$response = $client->request('GET', 'http://localhost:8057/404');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 hc-transient-fix branch September 20, 2019 10:16
@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

Development

Successfully merging this pull request may close these issues.

3 participants