[HttpClient] Fix exception triggered with AsyncResponse #38378

jderusse · Oct 2, 2020

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 in 73ca97c

    
           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 ''; 
        
               } 
        
               throw new TransportException('Cannot get the content of the response twice: buffering is disabled.');

Because CommonResponseTrait::$content is null and self::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 in 73ca97c

    
           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 = new ErrorChunk($r->offset, new TransportException(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

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

nicolas-grekas · Oct 2, 2020

Thank you @jderusse.

carsonbot added Status: Needs Review Bug labels Oct 2, 2020

jderusse force-pushed the fix-async branch from b88b7cf to d635d3b Compare October 2, 2020 09:18

nicolas-grekas added this to the next milestone Oct 2, 2020

nicolas-grekas reviewed Oct 2, 2020

View reviewed changes

src/Symfony/Component/HttpClient/Response/CommonResponseTrait.php Outdated Show resolved Hide resolved

nicolas-grekas changed the title ~~Fix exception triggered with AsyncResponse~~ [HttpClient] Fix exception triggered with AsyncResponse Oct 2, 2020

[HttpClient] Fix exception triggered with AsyncResponse

3aedb51

nicolas-grekas force-pushed the fix-async branch from d635d3b to 3aedb51 Compare October 2, 2020 13:57

nicolas-grekas merged commit 1f7b8fd into symfony:master Oct 2, 2020

nicolas-grekas modified the milestones: next, 5.2 Oct 5, 2020

jderusse deleted the fix-async branch October 15, 2020 10:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[HttpClient] Fix exception triggered with AsyncResponse #38378

[HttpClient] Fix exception triggered with AsyncResponse #38378

Uh oh!

jderusse commented Oct 2, 2020 •

edited

Loading

Uh oh!

Uh oh!

nicolas-grekas commented Oct 2, 2020

Uh oh!

Uh oh!

	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 '';
	}

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

	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 = new ErrorChunk($r->offset, new TransportException(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();
	}

Search code, repositories, users, issues, pull requests...

Uh oh!

[HttpClient] Fix exception triggered with AsyncResponse #38378

[HttpClient] Fix exception triggered with AsyncResponse #38378

Uh oh!

Conversation

jderusse commented Oct 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

nicolas-grekas commented Oct 2, 2020

Uh oh!

Uh oh!

jderusse commented Oct 2, 2020 •

edited

Loading