From 2e72e9bfab07c541d403461e06a42af31219a7d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maty=C3=A1=C5=A1=20Jir=C3=A1t?= Date: Thu, 20 Aug 2026 06:01:56 +0200 Subject: [PATCH 1/3] Surface Guzzle response-creation failures as UserException Guzzle raises `RequestException: An error was encountered while creating the response` when `EasyHandle::createResponse()` throws - a malformed or missing HTTP status line, or a sink that cannot be opened. The wrapped cURL errno for that path (CURLE_WRITE_ERROR) is not in the recognized user-error list, so the exception escaped `HttpExtractor` unhandled and the job died with an opaque internal error (exit 2), hiding the real reason from the user and paging the team. Re-raise only that specific exception as a `UserException` carrying the previous exception's message, so the run still fails but with exit 1 and an actionable message. Every other `RequestException` is returned and rethrown untouched. Co-Authored-By: Claude Opus 5 (1M context) --- src/HttpExtractor.php | 35 ++++++++++++++++- tests/phpunit/HttpExtractorTest.php | 60 +++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 2 deletions(-) diff --git a/src/HttpExtractor.php b/src/HttpExtractor.php index 5a41390..7cf9b2d 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,29 @@ private function sendRequest(UriInterface $httpSource, array $options): void // will throw exception for HTTP errors, no need to signal back } + /** + * When Guzzle fails while building the PSR-7 response - the server sent a malformed status line, + * or the destination file could not be opened - it wraps the real cause into a generic + * RequestException whose own message says nothing. That exception carries no cURL error number we + * recognize, so it 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 + { + if (strpos($e->getMessage(), self::RESPONSE_CREATION_ERROR_MESSAGE) === false) { + return $e; + } + + $previous = $e->getPrevious(); + + return new UserException(sprintf( + 'Error requesting "%s": the response could not be processed: %s', + (string) $httpSource, + $previous !== null ? $previous->getMessage() : $e->getMessage() + ), 0, $e); + } + /** * @return mixed[] */ diff --git a/tests/phpunit/HttpExtractorTest.php b/tests/phpunit/HttpExtractorTest.php index 7071ca3..e60dffb 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,64 @@ 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). + */ + 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(); + + $this->expectException(UserException::class); + $this->expectExceptionMessage( + 'Error requesting "http://example.com/result.txt": ' . + 'the response could not be processed: HTTP version missing from header data' + ); + + $extractor->extract( + new Uri('http://example.com/result.txt'), + $temp->createTmpFile()->getPathname() + ); + } + + /** + * Guard for the above: any other RequestException the extractor does not recognize must keep + * propagating untouched, exactly as before. + */ + public function testUnrelatedRequestExceptionKeepsPropagating(): void + { + $guzzleException = new RequestException( + 'An error was encountered during the on_headers event', + new Request('GET', 'http://example.com/result.txt'), + null, + null, + ['errno' => CURLE_WRITE_ERROR, 'error' => 'Failed writing header'] + ); + + $client = $this->getMockedExtractorClient([$guzzleException]); + $extractor = new HttpExtractor($client, []); + $temp = new Temp(); + + $this->expectException(RequestException::class); + $this->expectExceptionMessage('An error was encountered during the on_headers event'); + + $extractor->extract( + new Uri('http://example.com/result.txt'), + $temp->createTmpFile()->getPathname() + ); + } + private function getTestLogger(): Logger { $this->testHandler = new TestHandler(); From a977383e0de3c11fba080312864e15ae599e4ea5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maty=C3=A1=C5=A1=20Jir=C3=A1t?= Date: Thu, 20 Aug 2026 06:18:13 +0200 Subject: [PATCH 2/3] Address review: tighten the match and strengthen the tests - Compare Guzzle's message with `===` instead of `strpos()`. Guzzle passes that literal verbatim, so an exact comparison is strictly narrower and cannot catch some other exception that merely echoes a response body. - Leave the exception untouched when there is no previous exception (or it carries no message) rather than building a tautological message. That case is unreachable with Guzzle 7.3 and now simply keeps today's behaviour. - Assert the original RequestException survives as `getPrevious()`, and that an unrecognized RequestException propagates as the very same object. - Use a message the component could actually see in the guard test, and document why exactly one response is queued in the mock handler. Co-Authored-By: Claude Opus 5 (1M context) --- src/HttpExtractor.php | 10 ++++-- tests/phpunit/HttpExtractorTest.php | 50 +++++++++++++++++------------ 2 files changed, 38 insertions(+), 22 deletions(-) diff --git a/src/HttpExtractor.php b/src/HttpExtractor.php index 7cf9b2d..d2bdba5 100644 --- a/src/HttpExtractor.php +++ b/src/HttpExtractor.php @@ -120,16 +120,22 @@ private function sendRequest(UriInterface $httpSource, array $options): void */ private function convertResponseCreationError(RequestException $e, UriInterface $httpSource): Throwable { - if (strpos($e->getMessage(), self::RESPONSE_CREATION_ERROR_MESSAGE) === false) { + // 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 !== null ? $previous->getMessage() : $e->getMessage() + $previous->getMessage() ), 0, $e); } diff --git a/tests/phpunit/HttpExtractorTest.php b/tests/phpunit/HttpExtractorTest.php index e60dffb..ae0ac1e 100644 --- a/tests/phpunit/HttpExtractorTest.php +++ b/tests/phpunit/HttpExtractorTest.php @@ -74,6 +74,9 @@ public function testTooManyRedirects(): void * 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 { @@ -89,43 +92,50 @@ public function testResponseCreationErrorIsThrownAsUserException(): void $extractor = new HttpExtractor($client, []); $temp = new Temp(); - $this->expectException(UserException::class); - $this->expectExceptionMessage( - 'Error requesting "http://example.com/result.txt": ' . - 'the response could not be processed: HTTP version missing from header data' - ); - - $extractor->extract( - new Uri('http://example.com/result.txt'), - $temp->createTmpFile()->getPathname() - ); + 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 untouched, exactly as before. + * propagating as the very same object, exactly as before. */ public function testUnrelatedRequestExceptionKeepsPropagating(): void { $guzzleException = new RequestException( - 'An error was encountered during the on_headers event', + 'cURL error 23: Failed writing body', new Request('GET', 'http://example.com/result.txt'), null, null, - ['errno' => CURLE_WRITE_ERROR, 'error' => 'Failed writing header'] + ['errno' => CURLE_WRITE_ERROR, 'error' => 'Failed writing body'] ); $client = $this->getMockedExtractorClient([$guzzleException]); $extractor = new HttpExtractor($client, []); $temp = new Temp(); - $this->expectException(RequestException::class); - $this->expectExceptionMessage('An error was encountered during the on_headers event'); - - $extractor->extract( - new Uri('http://example.com/result.txt'), - $temp->createTmpFile()->getPathname() - ); + 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 From fbd43c4023de30e91c52f4012d06f08b10851b4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maty=C3=A1=C5=A1=20Jir=C3=A1t?= Date: Thu, 20 Aug 2026 06:26:06 +0200 Subject: [PATCH 3/3] Correct the docblock: sink-open failures are thrown outside createResponse() Comment-only. In Guzzle 7.3 the RuntimeException wrapped by this specific RequestException can only come from HeaderProcessor::parseHeaders(); a sink that cannot be opened throws outside createResponse() and is unaffected. Co-Authored-By: Claude Opus 5 (1M context) --- src/HttpExtractor.php | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/HttpExtractor.php b/src/HttpExtractor.php index d2bdba5..1ae35b5 100644 --- a/src/HttpExtractor.php +++ b/src/HttpExtractor.php @@ -111,12 +111,13 @@ private function sendRequest(UriInterface $httpSource, array $options): void } /** - * When Guzzle fails while building the PSR-7 response - the server sent a malformed status line, - * or the destination file could not be opened - it wraps the real cause into a generic - * RequestException whose own message says nothing. That exception carries no cURL error number we - * recognize, so it 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. + * 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 {