Movatterモバイル変換

Skip to content

symfony/symfonyPublic

NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[HttpClient] Fix exception triggered with AsyncResponse#38378

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

nicolas-grekas merged 1 commit intosymfony:masterfromjderusse:fix-async

Oct 2, 2020

Merged

[HttpClient] Fix exception triggered with AsyncResponse#38378

nicolas-grekas merged 1 commit intosymfony:masterfromjderusse:fix-async

Oct 2, 2020

Conversation

@jderusse

Copy link

Member

jderusse commentedOct 2, 2020•
edited
Loading

Q	A
Branch?	master
Bug fix?	yes
New feature?	no
Deprecations?	no
Tickets	#38289 (comment)
License	MIT
Doc PR	/

When a request is replayed with the AsyncResponse and the 2nd request failed, users get an exception even when they pass$throw=false.

In fact, there are 2 exceptions:

Cannot get the content of the response twice: buffering is disabled.

symfony/src/Symfony/Component/HttpClient/Response/CommonResponseTrait.php

Lines 51 to 68 in73ca97c

	if (null ===$this->content) {
	$content =null;

	foreach (self::stream([$this])as$chunk) {
	if (!$chunk->isLast()) {
	$content .=$chunk->getContent();
	}
	}

	if (null !==$content) {
	return$content;
	}

	if ('HEAD' ===$this->getInfo('http_method') \|\|\in_array($this->getInfo('http_code'), [204,304],true)) {
	return'';
	}

	thrownewTransportException('Cannot get the content of the response twice: buffering is disabled.');

Because CommonResponseTrait::$content is null andself::stream yield only the LastChunk. The content is stays null

HTTP/2 429 returned for "https://httpbin.org/status/429"

symfony/src/Symfony/Component/HttpClient/Response/AsyncResponse.php

Lines 209 to 225 in73ca97c

	if (!$r->passthru) {
	if (null !==$chunk->getError() \|\|$chunk->isLast()) {
	unset($asyncMap[$response]);
	}elseif (null !==$r->content &&'' !== ($content =$chunk->getContent()) &&\strlen($content) !==fwrite($r->content,$content)) {
	$chunk =newErrorChunk($r->offset,newTransportException(sprintf('Failed writing %d bytes to the response buffer.',\strlen($content))));
	$r->info['error'] =$chunk->getError();
	$r->response->cancel();
	}

	yield$r =>$chunk;
	continue;
	}

	if (null ===$chunk->getError() &&$chunk->isFirst()) {
	// Ensure no exception is thrown on destruct for the wrapped response
	$r->response->getStatusCode();
	}

Because on the second request passthru is null, it didn't disable the the exception thrown on destruct for the wrapped response.
This

@carsonbot

carsonbot added Status: Needs Review Bug labels

@jderusse

jderusse force-pushed thefix-async branch fromb88b7cf tod635d3bCompare

October 2, 2020 09:18

@nicolas-grekas

nicolas-grekas added this to thenext milestone

nicolas-grekas

nicolas-grekas reviewed

View reviewed changes

src/Symfony/Component/HttpClient/Response/CommonResponseTrait.php OutdatedShow resolvedHide resolved

@nicolas-grekas

nicolas-grekas changed the title~~Fix exception triggered with AsyncResponse~~[HttpClient] Fix exception triggered with AsyncResponse

@jderusse

@nicolas-grekas

[HttpClient] Fix exception triggered with AsyncResponse

3aedb51

@nicolas-grekas

nicolas-grekas force-pushed thefix-async branch fromd635d3b to3aedb51Compare

October 2, 2020 13:57

@nicolas-grekas

Copy link

Member

nicolas-grekas commentedOct 2, 2020

Thank you@jderusse.

@nicolas-grekas

nicolas-grekas merged commit1f7b8fd intosymfony:master

@nicolas-grekas

nicolas-grekas modified the milestones:next,5.2

@jderusse

jderusse deleted the fix-async branch

October 15, 2020 10:01

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

Labels

Bug Status: Needs Review

3 participants

@jderusse

@nicolas-grekas

@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp