Skip to content

fix(auth): keep the real upstream reason in lastError - #11376

Merged
diegosouzapw merged 2 commits into
cheaper-inference:release/v3.8.50from
ntdat812:fix/provider-error-detail-omniroute
Aug 24, 2026
Merged

diegosouzapw merged 2 commits into
cheaper-inference:release/v3.8.50from
ntdat812:fix/provider-error-detail-omniroute

Conversation

@ntdat812

Copy link
Copy Markdown
Contributor

⚠️ base-red inherited: #9985 — release/v3.8.50 is not green; the failures this PR
inherits are the ones tracked there, not introduced here.

Problem

markAccountUnavailable (src/sse/services/auth.ts) kept the upstream reason only
when it was already a string:

const errorMsg = typeof errorText === "string" ? errorText.slice(0, 100) : "Provider error";

Everything else collapsed to that literal, and it is what the operator reads —
lastError on the connection row, and the ❌ provider [status]: … console line.

The shape that matters most is not a string. A failed fetch arrives as
TypeError: fetch failed with the actionable part on error.cause.code, so a
wrong port, a firewall, a DNS failure and a blocked proxy were indistinguishable
from each other and from a provider that simply said no.

Change

describeUpstreamFailure(value, fallback?, maxLength?) in
src/shared/utils/upstreamError.ts, next to the code that already parses provider
error bodies. It reuses the module's extractErrorMessage (now exported rather
than duplicated) and adds what that helper does not cover:

  • an Error's message, with the transport code appended when the message does not
    already carry it — fetch failed (ECONNREFUSED);
  • a bare code when there is no message at all — Provider error (EAI_AGAIN);
  • whitespace collapsed, so the dashboard row stays one line.

The value is never serialized wholesale. Only message-shaped fields and codes
are read, so a request body or header attached to an error cannot reach the stored
reason. A test attaches an authorization header to the error and asserts it does
not appear in the output.

Tests

tests/unit/provider-error-detail-lastError.test.ts (new, 10 cases): the string
path unchanged including the 100-char clamp; fetch failed with ECONNREFUSED /
ENOTFOUND; no duplicated code when the message already names it; the provider JSON
shapes including errors[]; the code-only fallback and a custom fallback; {} /
null / undefined / a number; the no-wholesale-serialization guarantee; newline
collapsing; extractErrorMessage still behaving for its existing caller; and a
source assertion that auth.ts routes through the helper and that the string-only
collapse is gone.

Without the source change the file cannot even resolve describeUpstreamFailure,
so it fails at import (not ok 1 … # fail 1) rather than per-case — noted here so
the "fails on base" claim is not overstated.

npx tsc --noEmit clean; npx eslint --suppressions-location config/quality/eslint-suppressions.json
clean on the changed files.

Changelog fragment: changelog.d/fixes/lasterror-provider-error-detail.md.

@ntdat812
ntdat812 requested a review from diegosouzapw as a code owner August 24, 2026 10:36
`markAccountUnavailable` stored the reason only when it was already a
string, so every other shape collapsed to the literal "Provider error" —
the value the dashboard shows as `lastError` and the console line prints.

The shape that matters most is not a string. A failed `fetch` arrives as
`TypeError: fetch failed` with the actionable part on `error.cause.code`,
so a wrong port, a firewall, a DNS failure and a blocked proxy were
indistinguishable to the operator.

`describeUpstreamFailure` reuses the `extractErrorMessage` already in
`upstreamError.ts` (now exported) for provider JSON bodies, appends the
transport code when the message does not carry it, and falls back to the
code alone before giving up. It never serializes the error object
wholesale, so a request body or header attached to an error cannot leak
into the stored reason.
@ntdat812

Copy link
Copy Markdown
Contributor Author

CI note, so the red is not left ambiguous.

Unit Tests fast-path — openapi-coverage.test.ts — is base-side, not this PR.

AssertionError: OpenAPI operation coverage regressed: 34.5% < floor 34.6%

The same assertion fails identically on #11374, #11376, #11377 and #11380, whose
diffs are disjoint (bin/cli/*, src/sse/services/auth.ts, src/shared/utils/wsPath.ts,
src/lib/tokenHealthCheckKimi.ts). None of them adds or removes an API route, so
none can move that ratio. The floor sits 0.1 point above what release/v3.8.50
currently documents — routes landed without their openapi entries.

Build (advisory) and dast-smoke are the inherited base-red already tracked in #9985.

I have not touched the coverage floor: rebaselining a gate I did not break is not
mine to do, and it would hide the drift. Say the word if you would rather it be
bumped, or if you want the undocumented operations written up.

`Fast Quality Gates` refused the file-size ratchet:
`src/sse/services/auth.ts: 3346 > congelado 3344`.

The two lines, itemised in the baseline entry: the import of
`describeUpstreamFailure` (the logic itself lives in upstreamError.ts, so
nothing else moved into this file, and the call site is net zero), and
the repo's own lint-staged prettier pass splitting a pre-existing
two-statements-on-one-line at `getProviderCredentials` — which re-applies
on any commit touching this file, so it cannot be separated out.
@ntdat812

Copy link
Copy Markdown
Contributor Author

CI update — all three reds are base-side; none are reachable from this diff.

Unit Tests fast-path — same openapi-coverage.test.ts failure triaged above
(34.5% < floor 34.6%), just surfacing under the shard name now. That test reads exactly
two inputs, src/app/api/**/route.ts and docs/openapi.yaml. Both are byte-identical to
the base:

$ git rev-parse origin/release/v3.8.50:src/app/api  HEAD:src/app/api
969e4cfef5784db139716e16c68ba0458dd7ed72
969e4cfef5784db139716e16c68ba0458dd7ed72
$ git rev-parse origin/release/v3.8.50:docs/openapi.yaml  HEAD:docs/openapi.yaml
540f2abd52dba4f7a143613146eed70ca7f0c8e3
540f2abd52dba4f7a143613146eed70ca7f0c8e3

Identical inputs cannot produce a different coverage number, so this PR cannot have moved it.
Still not lowering a gate I did not break.

Build (advisory) and dast-smoke — both continue-on-error: true, and both died on
The operation was canceled with no compile error and no assertion, mid Creating an optimized production build / mid CLI-bundle build. Not diff-specific: #11378 (different author, disjoint
diff) is red on the same three checks right now, and #11381/#11382 are red on
Fast Production Build/dast-smoke. dast-smoke's job failed on 4 of the 5 other branches
sampled from the last 20 runs of that workflow.

Nothing to change here. Happy to rebase if you want a fresh run.

@diegosouzapw
diegosouzapw merged commit 019ad33 into cheaper-inference:release/v3.8.50 Aug 24, 2026
13 of 16 checks passed
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…nce#11376)

Merged via consolidated batch validation. markAccountUnavailable collapsed every non-string upstream error reason to a generic 'Provider error' literal, hiding the actual upstream detail operators need in lastError. Own test passes.
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.

2 participants