Error-surface follow-ups from #103/#107: four defects, four different remedies

[tonychang04]

Four follow-ups fall out of #103 and #107. They share one shape — the error surface tells the reader something other than what happened — but they have different remedies, and conflating them ships internal text to users. Filing rather than leaving in Slack, for the reason @infra-bot gave on #108: "worth a line in the doc" is how an item goes missing for a month.

None is blocking. All are after the cutover window.

---

1. The server_error card promises details it never renders

humanFormOf's server_error branch says "If it keeps happening, the details below are what support will ask for." Rendered, the card is <h1> + <p> and ends there.

<h1>Something went wrong on our side</h1>
<p>…the details below are what support will ask for.</p>     <- card ends
action: undefined | 'server_error' on the card: false | description: false

Do not fix this by rendering the description. @blog-bot established the provenance: across the four emitting sites one description is a caught error.message and one is "Set INSFORGE_CLIENT_ID and INSFORGE_CLIENT_SECRET" — an operator instruction shown to a stranger. And the error code is a constant on that branch (all emissions are the literal server_error), so rendering it tells support nothing.

What is actually missing is a correlation id, which the page has no concept of. Wording is parked in the cutover checklist under "the OAuth error card promises details it never renders", including an interim string that stops promising, plus the implementation note that the id must render beneath the message so "below" becomes literally true.

2. PlatformTokens models a field the platform never sends

Measured on POST /api/oauth/v1/token, unauthenticated:

keys: ["details", "error", "message"]
has_error_description: false      has_message: true

// insforge-api.ts
export interface PlatformTokens {
  error?: string;
  error_description?: string;   // NEVER set by this platform
  // no `message`, no `details`
}

So tokens.error_description || tokens.error (server.ts) is not a fallback — the first operand is undefined by construction, every time, and the user is shown the error code while the platform's human-readable message is discarded. It reads like defensive coding and is a straight-line discard.

Unlike item 1, this description is worth rendering: it is an upstream diagnostic rather than one of our own artefacts. details is unexamined — do not render it until someone has looked at what it contains.

3. A caller-supplied bad X-Base-URL returns 500 "unreachable" about a host that answered

src/shared/tools/index.ts:131  throw new Error(`Health check failed with status ${status}`)
                               ^ a PLAIN Error — the status is in the sentence,
                                 not the value, so nothing downstream can map it
src/shared/tools/index.ts:178  throw new Error(`backend at ${url} is unreachable. …`)
                               ^ wraps EVERY failure as unreachable, including 404

Two visible defects — wrong word, wrong status class — both caused by a third: the status never survives as data. error-status.ts already has statusForHttpError/isPlatformUnavailable, and #80 (fix/upstream-http-status-mapping) is the same fix one file over.

This matters more than it looks because it is the error on the header path we would point CI/VM users at — and the same code ships in the stdio bundle, which is the product npm users actually run.

4. The runbook's discriminator is unasserted

The cutover uses grant_types_supported containing refresh_token to tell which container form it is talking to (/health reports 1.2.12 either side of #107, so it cannot). Nothing tests it: grep grant_types_supported\|grantTypes across every *.test.ts returns nothing.

Design corrected by @qa-bot before it was written. The obvious test — every advertised grant is one /oauth/token accepts — would have been green across a three-commit window where the instrument was already lying:

commit     advertises refresh_token   callback 503 branch
1cbf12e    YES                        no      <- signal appears
3cef02d    YES                        no
d8a3c2e    YES                        YES     <- certified behaviour appears

The runbook reads the metadata to predict the callback's behaviour, so the test must couple the metadata to the callback's 503 — not to what the token endpoint accepts. That assertion already exists in credential-guards.test.ts; this only has to state that the two travel together.

General form worth keeping: an instrument must name which behaviour it certifies. refresh_token honestly certifies the refresh grant and is being read as certifying the callback; they coincide today only because these commits merge as one unit.

---

Related: #108 (publish gap, and the publish-time gate asserting the bin serves rather than starts — that one belongs with the release work rather than here).

[tonychang04]

Correction to item 2 — the remedy is reversed

@manager verified the platform at source and item 2 as filed is wrong in the direction that matters. I wrote that the platform's description "is worth rendering: it is an upstream diagnostic rather than one of our own artefacts." It is not.

insforge-cloud-backend/src/controllers/oauth.controller.ts:447-452:

} catch (error: any) {
  res.status(400).json({
    error: error.message || OAuthErrorCode.INVALID_GRANT,   // internal exception text
    message: 'Failed to exchange authorization code',       // a per-site literal
    statusCode: 400,
  });
}

So on the token-exchange path the two fields are the opposite of what I assumed:

• message is a fixed string per call site. Known, and useless as a diagnostic — it says nothing the status and code do not.
• error carries whatever threw. The deliberate throws inside exchangeCodeForTokens are all new Error(OAuthErrorCode.*), so it usually reads as a clean code — which is why it measures as invalid_grant and looks safe. But any unintended exception in that function — a driver error, a TypeError — puts its message straight into the field.

@blog-bot's framing, which is sharper than the split I filed: message is known and useless, error is unknown and unsafe. Neither is showable. That is a stronger argument than "we have not measured it yet", because the unsafe field cannot be measured at all — only bounded.

What this changes

Items 1 and 2 collapse into one remedy: a correlation id. Nothing the platform hands us on this path is fit to render.

And a labelled trap for whoever implements this. Our tokens.error_description || tokens.error (server.ts) surfaces exactly that error field. It is not rendered today — the token-exchange branch passes a static human override — but the obvious-looking fix for item 1 is "render the description we already have", and doing that would publish the platform's internal exception text into a stranger's browser. That is the failure this whole issue exists to stop, arriving through the fix for it.

Item 2 therefore stays open but with an inverted goal: stop error from ever reaching a rendered field, rather than start rendering it. Adding message/details to PlatformTokens is still correct as type hygiene — the interface currently models error_description, which this platform never sends — but no new field becomes user-visible.