Skip to content

Split shim and runner response errors into status and body - #4261

Merged
un-def merged 1 commit into
masterfrom
pr_runner_shim_response_errors
Sep 4, 2026
Merged

Split shim and runner response errors into status and body#4261
un-def merged 1 commit into
masterfrom
pr_runner_shim_response_errors

Conversation

@un-def

@un-def un-def commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Follow-up to #4251, which separated API errors from connection errors but left three loose ends.

ShimHTTPError/RunnerHTTPError were misnamed. We raise them only for status codes, while "HTTP error" suggests anything about the protocol, including transport. Rename them under a parent that says what the family means -- the peer answered, the answer is unusable, and repeating the request is not expected to help:

ShimError
|-- ShimAPIVersionError            # our bug, stays loud
`-- ShimResponseError
    |-- ShimResponseStatusError    # 4xx/5xx as API error codes
    `-- ShimResponseBodyError      # the body cannot be read

Call sites catch the *ResponseError parent: none of them cares which leaf it was.

Build the errors from the Response instead of wrapping the one from raise_for_status(). Its message carried a reason phrase that Go derives from the status code alone, a client/server split that 4xx vs 5xx already says, and a URL whose authority is always localhost or a percent-encoded socket path -- but not the body, which is where shim and runner put the actual message.

Before:

404 Client Error: Not Found for url:
http+unix://%2Ftmp%2F.../api/tasks/abc

After:

GET /api/tasks/abc: 404: Task not found

Parse response bodies with pydantic instead of Response.json(). Besides saving a decode and an intermediate dict, this fixes a misclassification: Response.json() raises requests.JSONDecodeError, which is a RequestException, so a peer sending garbage was reported as a transport failure and retried. Both malformed JSON and a schema mismatch now raise ValidationError, wrapped as *ResponseBodyError. Nothing inside a client method raises RequestException any more except genuine transport, which is what runner_ssh_tunnel already documents.

This also closes the pydantic.ValidationError leak noted in #4251: an unparsable response body no longer escapes the pipeline tasks.

Follow-up to #4251, which separated API errors from connection errors
but left three loose ends.

`ShimHTTPError`/`RunnerHTTPError` were misnamed. We raise them only for
status codes, while "HTTP error" suggests anything about the protocol,
including transport. Rename them under a parent that says what the
family means -- the peer answered, the answer is unusable, and repeating
the request is not expected to help:

    ShimError
    |-- ShimAPIVersionError            # our bug, stays loud
    `-- ShimResponseError
        |-- ShimResponseStatusError    # 4xx/5xx as API error codes
        `-- ShimResponseBodyError      # the body cannot be read

Call sites catch the `*ResponseError` parent: none of them cares which
leaf it was.

Build the errors from the `Response` instead of wrapping the one from
`raise_for_status()`. Its message carried a reason phrase that Go
derives from the status code alone, a client/server split that 4xx vs
5xx already says, and a URL whose authority is always localhost or a
percent-encoded socket path -- but not the body, which is where shim and
runner put the actual message.

Before:

    404 Client Error: Not Found for url:
    http+unix://%2Ftmp%2F.../api/tasks/abc

After:

    GET /api/tasks/abc: 404: Task not found

Parse response bodies with pydantic instead of `Response.json()`.
Besides saving a decode and an intermediate dict, this fixes a
misclassification: `Response.json()` raises `requests.JSONDecodeError`,
which is a `RequestException`, so a peer sending garbage was reported
as a transport failure and retried. Both malformed JSON and a schema
mismatch now raise `ValidationError`, wrapped as `*ResponseBodyError`.
Nothing inside a client method raises `RequestException` any more except
genuine transport, which is what `runner_ssh_tunnel` already documents.

This also closes the `pydantic.ValidationError` leak noted in #4251:
an unparsable response body no longer escapes the pipeline tasks.
@un-def
un-def merged commit 2d87397 into master Sep 4, 2026
26 checks passed
@un-def
un-def deleted the pr_runner_shim_response_errors branch September 4, 2026 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant