Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions packages/realm-server/tests/card-endpoints-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1072,6 +1072,72 @@ 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 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
// 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,
};
// 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 OR file_alias = $2) AND type = 'instance'`,
{
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) {
Expand Down
15 changes: 14 additions & 1 deletion packages/runtime-common/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -700,16 +700,29 @@ export function systemError({
body,
id,
lid,
status: httpStatus = 500,
}: {
requestContext: RequestContext;
message: string;
additionalError?: CardError | Error;
body?: Record<string, any>;
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: 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 } : {}),
});
Expand Down
18 changes: 18 additions & 0 deletions packages/runtime-common/realm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4798,8 +4798,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
Comment thread
habdelra marked this conversation as resolved.
? 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),
Expand Down
Loading