Surface Guzzle response-creation failures as UserException instead of an internal error

[matyas-jirat-keboola]

Behaviour impact: none — defensive-only

No change to the success path, the downloaded file, the output manifest, or any
configuration. The only difference is the shape of an already-failing run: one
specific exception that used to escape unhandled now fails with exit code 1 and a
readable message instead of exit code 2 and "Internal Server Error occurred."
No existing test was modified or deleted.

Root cause

A Datadog alert reported keboola.ex-http jobs ending with an internal error
(image v2.5.5):

GuzzleHttp\Exception\RequestException: An error was encountered while creating the response
errPrevious: RuntimeException
errFile: /code/src/HttpExtractor.php   (the $this->client->get(...) call)
errTrace: HttpExtractor::extract <- HttpExtractorComponent::run

Guzzle emits that exact message from CurlFactory::createRejection() when
EasyHandle::createResponse() throws — i.e. Guzzle could not build a PSR-7 response
out of the transfer. In Guzzle 7.3 the only source of a RuntimeException there is
HeaderProcessor::parseHeaders() — Expected a non-empty array of header data,
HTTP version missing from header data, or HTTP status code missing from header data.
In other words, the remote host answered with a malformed or non-HTTP status line.
(A bad header value would surface as InvalidArgumentException, and a sink that cannot
be opened is thrown outside createResponse(), so neither is this.)

When the header callback aborts, cURL reports CURLE_WRITE_ERROR (23). That errno is
not in this class's $userErrors list, so HttpExtractor::sendRequest() fell through
to throw $e, the raw RequestException reached src/run.php, and the entrypoint's
Throwable handler logged it as critical and exited 2. Both underlying causes are
deterministic — retrying is pointless — and actionable by whoever owns the
configuration or the remote host, but the user was shown nothing.

The fix

HttpExtractor::convertResponseCreationError() is consulted at the two points that
previously did a bare throw $e. It converts only when all of the following hold,
and otherwise returns $e completely untouched:

• the message is exactly Guzzle's response-creation error (===, not a substring, so
  another exception that merely echoes a response body cannot be caught here); and
• there is a previous exception carrying a non-empty message to show the user.

It is deliberately placed after the existing errno checks, so no message produced by
the current code can change.

Before:

Internal Server Error occurred.        (exit 2, pages the team)

After:

Error requesting "https://host/file.csv": the response could not be processed: HTTP version missing from header data   (exit 1)

Why not a retry

Neither cause is transient. A malformed status line and an unopenable destination both
reproduce on every attempt, so retrying would only add load and delay the same failure.

Tests

Two tests added to tests/phpunit/HttpExtractorTest.php, neither touching existing ones:

• testResponseCreationErrorIsThrownAsUserException — drives the newly handled branch,
  asserts the UserException message, and asserts the original RequestException
  survives as getPrevious() so the job log keeps the full context. Run against the
  unmodified master copy of src/HttpExtractor.php it reproduces the production
  symptom exactly:

  GuzzleHttp\Exception\RequestException: An error was encountered while creating the response
  Caused by RuntimeException: HTTP version missing from header data
  

• testUnrelatedRequestExceptionKeepsPropagating — the guard: a different
  RequestException must leave the extractor as the very same object
  (assertSame). Passes both with and without the change.

Exactly one response is queued in each MockHandler on purpose — cURL error 23 is not
a retryable code, so an unexpected retry fails loudly on an empty queue rather than
passing silently.

Results in the component's own php:7-cli image:

composer phplint	pass
composer phpcs	pass
composer phpstan (level max)	pass
composer tests-phpunit	exit 1 — 40 tests, 72 assertions, 1 failure (see below)
composer tests-datadir	pass (both functional tests)

CI is red, and it is red on master too

composer ci fails — and therefore so does the Run tests step — on a single
pre-existing, unrelated test: testThrowsUserExceptionForNonValidCert. It pins a
hard-coded third-party IP (142.251.36.68) via CURLOPT_RESOLVE and expects
cURL error 60. That IP no longer serves TLS on 443, so the test gets whatever the
network gives it instead:

• unmodified master in the component's own image: cURL error 7: Connection refused
• this branch, same image: cURL error 7: Connection refused (identical)
• this branch on a GitHub runner, two separate runs: cURL error 28: Connection timed out

Two different errors from two different networks — it is a stale fixture, not a
regression, and it is untouched by this PR. Both new tests pass in CI. Worth fixing
separately.

Follow-up (not in scope here)

With this merged, the underlying RuntimeException message lands in the job log, so a
future occurrence can be classified properly — which of the three parseHeaders()
messages it is says a lot about what the remote host is actually doing. Acting on that
would be a real behaviour change and does not belong in a defensive PR.

[keboola-pr-reviewer-bot]

Verdict: auto_approve (risk 2/5) · profile component-factory

Approve: defensive-only error-reclassification that turns one opaque Guzzle failure into a legible UserException without touching the success path.

Impact flags: high blast radius — see Check Run summary.

[Copilot]

Pull request overview

This PR makes HttpExtractor surface a specific Guzzle failure mode (“An error was encountered while creating the response”) as a UserException, so already-failing runs return exit code 1 with an actionable message instead of escaping as an internal error (exit code 2).

Changes:

• Add HttpExtractor::convertResponseCreationError() and consult it in the two RequestException rethrow paths to translate Guzzle response-creation failures into UserException.
• Add PHPUnit coverage for the newly handled branch and a guard test ensuring unrelated RequestExceptions still propagate unchanged.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/HttpExtractor.php	Converts Guzzle response-creation RequestExceptions into UserException with the underlying (previous) exception message.
tests/phpunit/HttpExtractorTest.php	Adds unit tests for the new conversion behavior and for preserving propagation of unrelated request exceptions.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[keboola-pr-reviewer-bot]

New commit on a977383 — dismissed 1 stale bot approval. Comment @keboola-pr-reviewer-bot review when you want a fresh review.

Dismissing prior approval — a new commit was pushed and this review was for an earlier SHA. Run @keboola-pr-reviewer-bot review to get a fresh verdict.

[keboola-pr-reviewer-bot]

Verdict: needs_human (risk 4/5) · profile component-factory

Head commit has failing checks (Build); no required-check rules discovered on the base branch, falling back to block-on-any-failure.

[matyas-jirat-keboola]

CI note for whoever picks this up: the red Build is not from this change.

composer ci fails on one pre-existing test, testThrowsUserExceptionForNonValidCert, which pins a hard-coded third-party IP (142.251.36.68) via CURLOPT_RESOLVE and expects cURL error 60. That IP no longer serves TLS on 443, so the test gets whatever the network returns instead — cURL error 7 locally, cURL error 28 on the runner. It fails identically on unmodified master in the component's own image.

Every CI run of this branch reports the same shape: 40 tests, 72 assertions, 1 failure, that test only. Both tests added here pass, and phplint / phpcs / phpstan --level=max / tests-datadir are clean. The stale fixture is worth fixing, but separately — folding it in here would mix an unrelated test change into a deliberately defensive-only diff.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/HttpExtractor.php:116

• The doc comment says the RequestException message "says nothing", but Guzzle’s message is still present (it’s just generic/non-actionable). This is slightly misleading when someone is debugging behavior based on the comment.

     * generic RequestException whose own message says nothing. cURL reports that abort as