Skip to content

Surface the cause chain on alphaXiv transport errors - #403

Open
Skyline-9 wants to merge 1 commit into
alphaXiv:mainfrom
Skyline-9:error-cause-chain
Open

Skyline-9 wants to merge 1 commit into
alphaXiv:mainfrom
Skyline-9:error-cause-chain

Conversation

@Skyline-9

Copy link
Copy Markdown
Contributor

Related to #402.

The five alphaXiv request sites in src/client.rs report only reqwest's own
wrapper, so a network failure arrives with no usable detail:

Could not reach alphaXiv at https://api.alphaxiv.org: error sending request for url (…)

The actionable part is always one or two levels down source() — a refused
connection, a DNS failure, or invalid peer certificate: UnknownIssuer behind
a TLS-inspecting proxy. None of it reaches the user today.

This is not a theoretical gap. Diagnosing a TLS trust failure against this
client currently requires attaching a debugger or writing a separate probe
binary, because the one string the CLI prints is identical for every possible
transport failure.

Change

Adds transport_error, which walks the source() chain and appends each
distinct cause to the message, and routes the five map_err sites through it.

A plain .context(…) layer does not solve this: anyhow only renders context
chains under {:#}, and main prints {} (src/main.rs:912, documented at
the top of src/error.rs). Flattening the chain into the message keeps that
entry-point contract untouched.

Before:

Could not reach alphaXiv at https://api.alphaxiv.org: error sending request for url (…)

After:

Could not reach alphaXiv at https://api.alphaxiv.org: error sending request for url (…): client error (Connect): invalid peer certificate: UnknownIssuer

Message prefixes are unchanged, so existing output is a strict subset of the
new output and nothing that matches on the old text breaks.

Notes

  • Duplicate cause strings are skipped, so nested errors that restate their
    parent do not produce repeated text.
  • One regression test added (transport_error_includes_the_source_chain),
    using a connection to reserved port 1 to produce a real nested error rather
    than a synthetic one.
  • No API or behavior change beyond the error text.

Verification

  • cargo fmt --check — clean
  • cargo clippy --all-targets -- -D warnings — clean
  • cargo test --bin orx — 890 passed, 0 failed, 2 ignored

A few tests in local::native_store::opencode_database and jobs::localbox
fail intermittently under full-suite concurrency (roughly one run in two, and
not always the same test). They do the same on an unmodified ecc531e
checkout, so they are pre-existing and unrelated to these changes — noted here
only so a red run is not mistaken for a regression.

(The crate is binary-only, so cargo test --lib reports no library targets;
--bin orx is the working invocation.)

`main` prints an error's `Display`, so the five alphaXiv request sites
reported only reqwest's own wrapper:

    Could not reach alphaXiv at https://api.alphaxiv.org: error sending
    request for url (...)

The actionable detail always lives in `source()` — a refused connection,
a DNS failure, or `invalid peer certificate: UnknownIssuer` behind a
TLS-inspecting proxy. None of it reached the user, so a network failure
could not be diagnosed without attaching a debugger or writing a probe
binary against the same client.

Add `transport_error`, which walks `source()` and appends each distinct
cause to the message. An `anyhow` context layer alone would not work
here: that detail is only rendered by `{:#}`, and the entry point uses
`{}`.

Message prefixes are unchanged, so existing output is a strict subset of
the new output.

This branch has not been deployed

No deployments
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