From aa77d53418985c5ac2ad351d498cd7b29c9244f0 Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Thu, 28 May 2026 13:54:07 -0400 Subject: [PATCH 1/3] Mirror index-error card status onto the HTTP response instead of always 500 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../realm-server/tests/card-endpoints-test.ts | 59 +++++++++++++++++++ packages/runtime-common/error.ts | 8 ++- packages/runtime-common/realm.ts | 18 ++++++ 3 files changed, 84 insertions(+), 1 deletion(-) diff --git a/packages/realm-server/tests/card-endpoints-test.ts b/packages/realm-server/tests/card-endpoints-test.ts index 777ed06abbf..2ea823a7ec3 100644 --- a/packages/realm-server/tests/card-endpoints-test.ts +++ b/packages/realm-server/tests/card-endpoints-test.ts @@ -1072,6 +1072,65 @@ module(basename(__filename), function () { 'broken friend relationship is preserved on the wire as a reference — the consumer renders the placeholder, the server does not error', ); }); + + test('GET on an existing-but-errored index entry mirrors the underlying error status onto the HTTP response, but never 404 (reserved for a missing row) and never a non-HTTP status', async function (assert) { + let cardURL = `${testRealmHref}person-1`; + let cases: { errorStatus: number; expectedHttp: number }[] = [ + // Real, card-level HTTP error statuses flow through unchanged. + { errorStatus: 401, expectedHttp: 401 }, + { errorStatus: 403, expectedHttp: 403 }, + { errorStatus: 422, expectedHttp: 422 }, + { errorStatus: 500, expectedHttp: 500 }, + // An existing-but-errored card is never "not found": a + // recorded 404 (e.g. the error's underlying cause was a + // missing linked instance) falls back to 500 so that a 404 + // on a card GET stays an unambiguous "card no longer exists" + // signal. + { errorStatus: 404, expectedHttp: 500 }, + // Non-HTTP / out-of-range statuses also fall back to 500: a + // fetch failure recorded as 0, and a non-error status that + // should never reach the error-row branch. + { errorStatus: 0, expectedHttp: 500 }, + { errorStatus: 200, expectedHttp: 500 }, + ]; + + for (let { errorStatus, expectedHttp } of cases) { + let errorDoc = { + message: 'boom', + status: errorStatus, + title: 'Some Error', + additionalErrors: null, + }; + for (let table of ['boxel_index', 'boxel_index_working']) { + await dbAdapter.execute( + `UPDATE ${table} + SET has_error = TRUE, error_doc = $1::jsonb + WHERE url = $2`, + { + bind: [JSON.stringify(errorDoc), cardURL], + }, + ); + } + + let response = await request + .get('/person-1') + .set('Accept', 'application/vnd.card+json'); + + assert.strictEqual( + response.status, + expectedHttp, + `errorDoc.status ${errorStatus} → HTTP ${expectedHttp}`, + ); + // The JSON:API body always carries the real underlying + // status regardless of the HTTP status chosen, so consumers + // can still see the precise cause. + assert.strictEqual( + response.body.errors?.[0]?.status, + errorStatus, + `JSON:API body preserves the underlying status (${errorStatus})`, + ); + } + }); }); module('permissioned realm', function (hooks) { diff --git a/packages/runtime-common/error.ts b/packages/runtime-common/error.ts index 53287eac4ca..1accb01764e 100644 --- a/packages/runtime-common/error.ts +++ b/packages/runtime-common/error.ts @@ -700,6 +700,7 @@ export function systemError({ body, id, lid, + status = 500, }: { requestContext: RequestContext; message: string; @@ -707,9 +708,14 @@ export function systemError({ body?: Record; id?: string; lid?: string; + // HTTP status for the response. Defaults to 500; callers serving an + // index error row pass the underlying error's status so a card whose + // recorded failure is, say, an auth or validation error returns that + // status rather than masquerading as a realm outage. + status?: number; }): Response { let err = new CardError(message, { - status: 500, + status, ...(id ? { id } : {}), ...(lid ? { lid } : {}), }); diff --git a/packages/runtime-common/realm.ts b/packages/runtime-common/realm.ts index b4a45764f36..8b38a6dcc60 100644 --- a/packages/runtime-common/realm.ts +++ b/packages/runtime-common/realm.ts @@ -4870,8 +4870,26 @@ export class Realm { } } if (maybeError.type === 'error') { + // The index has a row for this card, it just can't be served + // cleanly — so mirror the underlying error's HTTP status when it + // is a real HTTP error status (auth 401/403, validation 422, + // upstream 5xx, …) instead of flattening everything to 500. + // + // 404 is the one status we never mirror: an existing-but-errored + // card is not "not found". 404 is reserved for a missing index + // row (see `notFound` above) so that a 404 on a card GET is an + // unambiguous "this card no longer exists" signal. A recorded + // 404 (e.g. an error whose underlying cause was a missing linked + // instance) therefore falls back to 500, as do non-HTTP failures + // (fetch failures recorded as status 0) and any out-of-range + // value. + let errorStatus = maybeError.error.errorDetail.status; return systemError({ requestContext, + status: + errorStatus >= 400 && errorStatus <= 599 && errorStatus !== 404 + ? errorStatus + : 500, message: `cannot return card, ${request.url}, from index: ${maybeError.error.errorDetail.title} - ${maybeError.error.errorDetail.message}`, id: request.url, additionalError: CardError.fromSerializableError(maybeError.error), From ef99d325d13ceee78b9d705601a0eca2ec18a49d Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Thu, 28 May 2026 13:59:43 -0400 Subject: [PATCH 2/3] Give systemError a non-throwing status title so mirrored upstream codes 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) --- packages/realm-server/tests/card-endpoints-test.ts | 4 ++++ packages/runtime-common/error.ts | 11 +++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/realm-server/tests/card-endpoints-test.ts b/packages/realm-server/tests/card-endpoints-test.ts index 2ea823a7ec3..221f5d603e2 100644 --- a/packages/realm-server/tests/card-endpoints-test.ts +++ b/packages/realm-server/tests/card-endpoints-test.ts @@ -1081,6 +1081,10 @@ module(basename(__filename), function () { { errorStatus: 403, expectedHttp: 403 }, { errorStatus: 422, expectedHttp: 422 }, { errorStatus: 500, expectedHttp: 500 }, + // An unregistered-but-in-range upstream status (e.g. a proxied + // 520) is still mirrored and must not throw while building the + // error response. + { errorStatus: 520, expectedHttp: 520 }, // An existing-but-errored card is never "not found": a // recorded 404 (e.g. the error's underlying cause was a // missing linked instance) falls back to 500 so that a 404 diff --git a/packages/runtime-common/error.ts b/packages/runtime-common/error.ts index 1accb01764e..8de60f8d6fc 100644 --- a/packages/runtime-common/error.ts +++ b/packages/runtime-common/error.ts @@ -700,7 +700,7 @@ export function systemError({ body, id, lid, - status = 500, + status: httpStatus = 500, }: { requestContext: RequestContext; message: string; @@ -715,7 +715,14 @@ export function systemError({ status?: number; }): Response { let err = new CardError(message, { - status, + status: httpStatus, + // Supply a title explicitly so the CardError constructor never falls + // back to `getReasonPhrase`, which throws for unregistered codes — a + // mirrored upstream status (e.g. a proxied 520/499) would otherwise + // turn the error response itself into an unhandled failure. The + // `statuses` lookup returns undefined for unknown codes rather than + // throwing, and we backstop that with a generic phrase. + title: status.message[httpStatus] || `HTTP ${httpStatus}`, ...(id ? { id } : {}), ...(lid ? { lid } : {}), }); From c46fa6e904f9b8f72bdf9679b7a02451331d9ecd Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Thu, 28 May 2026 17:50:59 -0400 Subject: [PATCH 3/3] Fix card-error status test: target the index instance row by file_alias 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 = ` 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) --- packages/realm-server/tests/card-endpoints-test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/realm-server/tests/card-endpoints-test.ts b/packages/realm-server/tests/card-endpoints-test.ts index 221f5d603e2..23e66e30420 100644 --- a/packages/realm-server/tests/card-endpoints-test.ts +++ b/packages/realm-server/tests/card-endpoints-test.ts @@ -1105,11 +1105,14 @@ module(basename(__filename), function () { title: 'Some Error', additionalErrors: null, }; + // The instance row is keyed by `url` with the `.json` suffix; + // the bare card URL is the `file_alias`. Match either so the + // error flag lands on the row the GET read resolves. for (let table of ['boxel_index', 'boxel_index_working']) { await dbAdapter.execute( `UPDATE ${table} SET has_error = TRUE, error_doc = $1::jsonb - WHERE url = $2`, + WHERE (url = $2 OR file_alias = $2) AND type = 'instance'`, { bind: [JSON.stringify(errorDoc), cardURL], },