Inherit the index entry's error status on card responses instead of hardcoding 500#5026
Conversation
…ys 500 A GET on a card whose index entry is in an error state now returns the underlying error's HTTP status (auth 401/403, validation 422, upstream 5xx) rather than flattening everything to 500. `systemError` grows an optional `status` that defaults to 500. 404 is never mirrored: an existing-but-errored card is not "not found". 404 remains reserved for a missing index row (`notFound`) so a 404 on a card GET stays an unambiguous "this card no longer exists" signal — which is what the host store's reload path relies on to detect deletions. A recorded 404, a non-HTTP failure (status 0), or any out-of-range value falls back to 500. The JSON:API body still carries the real underlying status so consumers can see the precise cause. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa77d53418
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…es can't blow up the response CardError's constructor derives its title from `getReasonPhrase`, which throws for unregistered HTTP codes. Now that the error-row response mirrors the underlying status, a proxied upstream code (e.g. 520/499) could reach that fallback and turn the error response itself into an unhandled failure. Supply the title explicitly from the non-throwing `statuses` lookup, backstopped with a generic phrase. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates card GET error responses so existing index error rows return the underlying HTTP error status when safe, while preserving 404 exclusively for truly missing cards.
Changes:
- Derives card error-row HTTP status from the persisted index error status, with fallbacks to
500for404, non-error, and out-of-range statuses. - Extends
systemErrorto accept an optional response status and safely title unknown in-range status codes. - Adds coverage for mirrored statuses and fallback behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
packages/runtime-common/realm.ts |
Selects the HTTP status for card GET index-error responses from the stored error detail. |
packages/runtime-common/error.ts |
Adds optional status support to systemError and avoids throwing on unregistered status titles. |
packages/realm-server/tests/card-endpoints-test.ts |
Adds matrix-style assertions for HTTP status mirroring and JSON:API body preservation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…nceinstance-error-doc-propagation' into cs-11293-card-response-inherit-error-status
…nceinstance-error-doc-propagation' into cs-11293-card-response-inherit-error-status
Host Test Results 1 files ±0 1 suites ±0 1h 48m 9s ⏱️ +49s Results for commit 94279f8. ± Comparison against earlier commit 2fd1292. Realm Server Test Results 1 files ±0 1 suites ±0 11m 59s ⏱️ +37s Results for commit 94279f8. ± Comparison against earlier commit 2fd1292. |
…e-inherit-error-status
…nceinstance-error-doc-propagation' into cs-11293-card-response-inherit-error-status
Grafana previewPreview deployed for 1 dashboard in the staging Grafana.
Dashboards: Preview is torn down automatically when this PR is closed or merged. (Run: https://github.com/cardstack/boxel/actions/runs/26598855997) |
Observability diff (vs staging)Show diffdiff --git a/tmp/remote-canon.X8Sctn/dashboards/boxel-status/service-worker.json b/tmp/committed-canon.3tqdKP/dashboards/boxel-status/service-worker.json
index 14d31ea..cdcc357 100644
--- a/tmp/remote-canon.X8Sctn/dashboards/boxel-status/service-worker.json
+++ b/tmp/committed-canon.3tqdKP/dashboards/boxel-status/service-worker.json
@@ -1811,7 +1811,7 @@
{
"hide": 2,
"name": "env",
- "query": "staging",
+ "query": "__ENV__",
"skipUrlSync": true,
"type": "constant"
}
(Run: https://github.com/cardstack/boxel/actions/runs/26598855843) |
The injected error row never landed: instance rows are keyed by `url` with the `.json` suffix, while the bare card URL is the `file_alias`, so `WHERE url = <bare url>` matched nothing and the GET served the card normally. Match `file_alias` (and scope to the instance row) so the error flag lands on the row the read resolves. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nceinstance-error-doc-propagation' into cs-11293-card-response-inherit-error-status
…nceinstance-error-doc-propagation' into cs-11293-card-response-inherit-error-status
habdelra
changed the base branch from
cs-11212-indexer-terminate-instanceinstance-error-doc-propagation
to
main
Background and Goal
When the realm-server serves a card whose index entry is in an error state, the HTTP response status was always
500, even though the index entry already records the real cause (e.g. an auth403or a validation422). Cold-load consumers — browsers, AI tools, link checkers, HTTP caches — branch on status, so every errored card looked like a realm outage.This makes the card response inherit the index entry's status instead of hardcoding
500, while keeping404reserved for a genuinely missing card.Where to start
packages/runtime-common/realm.ts— the error-row branch of the card GET handler now derives the response status fromerrorDetail.status.packages/runtime-common/error.ts—systemErrorgrows an optionalstatus(defaults to500).packages/realm-server/tests/card-endpoints-test.ts— parametrized coverage over the status-selection rule.Key decisions
404.404stays reserved for a missing index row (notFound), so a404on a card GET is an unambiguous "this card no longer exists" signal — which the host store's reload-on-invalidation path relies on to detect deletions. A recorded404(e.g. an error whose underlying cause was a missing linked instance) therefore falls back to500. Because the realm upholds that invariant, the store needs no change.4xx/5xx(other than404) are mirrored; non-HTTP failures (a fetch failure recorded as status0) and out-of-range values fall back to500.Stacked on #5022.