diff --git a/src/HttpExtractor.php b/src/HttpExtractor.php index 5a41390..1ae35b5 100644 --- a/src/HttpExtractor.php +++ b/src/HttpExtractor.php @@ -13,10 +13,18 @@ use Keboola\Component\UserException; use Keboola\HttpExtractor\Exception\EncodingException; use Psr\Http\Message\UriInterface; +use Throwable; use function in_array; class HttpExtractor { + /** + * Message Guzzle uses when it cannot build a PSR-7 response out of the transfer. + * + * @see \GuzzleHttp\Handler\CurlFactory::createRejection() + */ + private const RESPONSE_CREATION_ERROR_MESSAGE = 'An error was encountered while creating the response'; + /** @var Client */ private $client; @@ -78,12 +86,12 @@ private function sendRequest(UriInterface $httpSource, array $options): void ]; $context = $e->getHandlerContext(); if (!isset($context['errno'])) { - throw $e; + throw $this->convertResponseCreationError($e, $httpSource); } $curlErrorNumber = $context['errno']; if (!in_array($curlErrorNumber, $userErrors)) { - throw $e; + throw $this->convertResponseCreationError($e, $httpSource); } $curlErrorMessage = $context['error']; throw new UserException(sprintf( @@ -102,6 +110,36 @@ private function sendRequest(UriInterface $httpSource, array $options): void // will throw exception for HTTP errors, no need to signal back } + /** + * When Guzzle cannot build the PSR-7 response out of a transfer - typically because the remote + * host answered with a malformed or missing HTTP status line - it wraps the real cause into a + * generic RequestException whose own message says nothing. cURL reports that abort as + * CURLE_WRITE_ERROR, which is not one of the error numbers recognized above, so the exception used + * to leave this class unhandled and the job died with an opaque internal error. Re-raise it as a + * UserException instead, so the run still fails but the underlying reason is visible. Anything + * else is returned untouched and keeps propagating exactly as before. + */ + private function convertResponseCreationError(RequestException $e, UriInterface $httpSource): Throwable + { + // Guzzle passes this message verbatim, so match it exactly rather than as a substring - + // a response body echoed into some other exception message must not be caught here. + if ($e->getMessage() !== self::RESPONSE_CREATION_ERROR_MESSAGE) { + return $e; + } + + $previous = $e->getPrevious(); + if ($previous === null || $previous->getMessage() === '') { + // Nothing to tell the user beyond the generic message, so leave it exactly as it was. + return $e; + } + + return new UserException(sprintf( + 'Error requesting "%s": the response could not be processed: %s', + (string) $httpSource, + $previous->getMessage() + ), 0, $e); + } + /** * @return mixed[] */ diff --git a/tests/phpunit/HttpExtractorTest.php b/tests/phpunit/HttpExtractorTest.php index 7071ca3..ae0ac1e 100644 --- a/tests/phpunit/HttpExtractorTest.php +++ b/tests/phpunit/HttpExtractorTest.php @@ -4,6 +4,7 @@ namespace Keboola\HttpExtractor\Tests; +use GuzzleHttp\Exception\RequestException; use GuzzleHttp\Handler\MockHandler; use GuzzleHttp\HandlerStack; use GuzzleHttp\Middleware; @@ -17,6 +18,7 @@ use Keboola\Temp\Temp; use Monolog\Handler\TestHandler; use PHPUnit\Framework\TestCase; +use RuntimeException; use function file_get_contents; use function sys_get_temp_dir; @@ -68,6 +70,74 @@ public function testTooManyRedirects(): void $extractor->extract($resource, $destination); } + /** + * Guzzle throws this when it cannot build a PSR-7 response out of the transfer, e.g. the server + * sent a malformed HTTP status line. The real cause only lives in the previous exception, and the + * generic RequestException used to escape the extractor as an opaque internal error (exit code 2). + * + * Exactly one response is queued on purpose: cURL error 23 is not a retryable code, so an + * unexpected retry must fail loudly on an empty mock queue instead of passing silently. + */ + public function testResponseCreationErrorIsThrownAsUserException(): void + { + $guzzleException = new RequestException( + 'An error was encountered while creating the response', + new Request('GET', 'http://example.com/result.txt'), + null, + new RuntimeException('HTTP version missing from header data'), + ['errno' => CURLE_WRITE_ERROR, 'error' => 'Failed writing header'] + ); + + $client = $this->getMockedExtractorClient([$guzzleException]); + $extractor = new HttpExtractor($client, []); + $temp = new Temp(); + + try { + $extractor->extract( + new Uri('http://example.com/result.txt'), + $temp->createTmpFile()->getPathname() + ); + $this->fail('Expected a UserException to be thrown'); + } catch (UserException $e) { + $this->assertSame( + 'Error requesting "http://example.com/result.txt": ' . + 'the response could not be processed: HTTP version missing from header data', + $e->getMessage() + ); + // the original exception must survive so the job log keeps the full context + $this->assertSame($guzzleException, $e->getPrevious()); + } + } + + /** + * Guard for the above: any other RequestException the extractor does not recognize must keep + * propagating as the very same object, exactly as before. + */ + public function testUnrelatedRequestExceptionKeepsPropagating(): void + { + $guzzleException = new RequestException( + 'cURL error 23: Failed writing body', + new Request('GET', 'http://example.com/result.txt'), + null, + null, + ['errno' => CURLE_WRITE_ERROR, 'error' => 'Failed writing body'] + ); + + $client = $this->getMockedExtractorClient([$guzzleException]); + $extractor = new HttpExtractor($client, []); + $temp = new Temp(); + + try { + $extractor->extract( + new Uri('http://example.com/result.txt'), + $temp->createTmpFile()->getPathname() + ); + $this->fail('Expected the original RequestException to be thrown'); + } catch (RequestException $e) { + $this->assertSame($guzzleException, $e); + } + } + private function getTestLogger(): Logger { $this->testHandler = new TestHandler();