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
39 changes: 39 additions & 0 deletions .changeset/anchor-missing-relation-quoted-template.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
---
"@objectstack/rest": patch
---

fix(rest): anchor `looksLikeMissingRelation` on the driver's quoted template (#8264)

`mapDataError`'s Postgres limb read `relation` and `does not exist` anywhere in
the message, not necessarily the same sentence — so ordinary business prose
using both words (`This relation does not exist in the diagram`) matched.
`does not exist` is ordinary business English; #8132 already anchored the
shared `@objectstack/types` leak predicate on the driver's own quoted
template for exactly this reason, and pinned the identical string as a
negative case. This file's copy of the same question was not covered by that
change (different package, different call site) and kept the loose reading.

Anchored the same way here — a quoted identifier required between `relation`
and `does not exist` — as a locally-owned pattern rather than a call into the
shared leak predicate: that
predicate answers a different question ("may this be withheld from the
client"), and its other limbs (`sqlite_`, `unique constraint`, `foreign key`,
a bare SQL statement) have nothing to do with this file's question (is this
specifically an unknown-relation condition, for the 404-vs-500 split
`looksLikeMissingRelation` feeds). `relation-sub-object.ts` documents "two
widths, on purpose" for a neighbouring pair of consumers that ask genuinely
different questions; that does not extend to the two USES inside this file,
which both ask the same question and share one predicate correctly.

**Both of the predicate's two call sites are covered, not just the reported
one:** the `DATA_STORE_FAULT` (500) gate the issue named, and the
`looksLikeUnknownObject` (404) limb the issue's own text did not measure. A
business message no longer gets mislabelled a `DATABASE_ERROR`, and a
crafted unquoted-but-attributable message no longer gets silently answered
`OBJECT_NOT_FOUND` — both now fall through to the generic, still-sanitised
terminal fault, which is the direction the branch's own #5462 comment already
argues for ("the safe way to be wrong is loud").

No reachable production path producing the unanchored shape was found at this
call site — this is consistency/invariant restoration between two spellings
of one question, not a fix for a demonstrated live misclassification.
39 changes: 38 additions & 1 deletion packages/rest/src/rest-server.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -437,6 +437,17 @@ function declaredHttpStatus(error: any): number | undefined {
return declared;
}

/**
* [#8264] Postgres' missing-relation template, anchored on the QUOTED
* identifier the driver always emits — never on the bare "does not exist"
* tail, which is ordinary business English. Module-scoped (not re-compiled
* per {@link mapDataError} call) and named, not inlined, so both of its
* readers share the literal same pattern. See the long note above
* `looksLikeMissingRelation`'s definition, further down this file, for why
* this is one width, not "two widths, on purpose".
*/
const RELATION_DOES_NOT_EXIST = /\brelation\s+["'`][^"'`]+["'`]\s+does not exist/i;

function missingRelationIsObject(raw: string, object: string | undefined): boolean {
if (!object) return false;
const named =
Expand DownExpand Up@@ -1051,9 +1062,35 @@ export function mapDataError(error: any, object?: string): { status: number; bod
// `code: 'OBJECT_NOT_FOUND'` and is matched far above) is the primary
// producer of this 404 anyway; the driver-string limb has been a legacy
// safety net since.
//
// [#8264] The Postgres limb used to be a two-`includes()` conjunction —
// `relation` and `does not exist` anywhere in the message, not necessarily
// the same sentence. `does not exist` is ordinary business English ("This
// relation does not exist in the diagram" — the exact negative case
// `error-leak.test.ts` pins for #8132's shared leak predicate), so that
// reading could re-verdict a legitimate business message through EITHER
// consumer below: the 500 gate right here, or the `looksLikeUnknownObject`
// 404 limb two lines further down (both read this same const). Anchored on
// Postgres' own errmsg template — a QUOTED identifier — the same technique
// #8132 used for `looksLikeInternalErrorLeak` in `@objectstack/types`.
//
// Deliberately NOT a call into that shared predicate: it answers a
// different question ("may this message be withheld from the client at
// all?"), and its other limbs — `sqlite_`, `unique constraint`,
// `foreign key`, a bare SQL statement — have nothing to do with THIS
// question (is this specifically an unknown-relation condition, for the
// 404-vs-500 split below?). `relation-sub-object.ts` documents "two
// widths, on purpose" for a neighbouring pair of consumers for exactly
// this reason — different questions get different patterns even when they
// share a substring. That precedent does NOT extend to the two USES right
// here, though: both the 500 gate and the 404 limb are asking this file's
// one question, and `missingRelationIsObject` below already gates the 500
// path on attribution — so one width for both is correct, not "two
// widths, on purpose" a second time. See the reverse-verification note in
// `rest-unknown-object-heuristic.test.ts` for both paths measured.
const looksLikeMissingRelation =
lower.includes('no such table') ||
(lower.includes('relation') && lower.includes('does not exist')) ||
RELATION_DOES_NOT_EXIST.test(raw) ||
lower.includes('table not found');
if (looksLikeMissingRelation && !missingRelationIsObject(raw, object)) {
return DATA_STORE_FAULT();
Expand Down
81 changes: 81 additions & 0 deletions packages/rest/src/rest-unknown-object-heuristic.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -488,3 +488,84 @@ describe('[#5462] the declared-status band is untouched', () => {
expect(r.body.field).toBe('label');
});
});

// ---------------------------------------------------------------------------
// 5. [#8264] The Postgres limb is anchored on the quoted template — both
// decision paths this one const feeds
// ---------------------------------------------------------------------------
//
// `looksLikeMissingRelation` used to read `relation` and `does not exist`
// anywhere in the message, not necessarily the same sentence, so ordinary
// business prose using both words matched — `error-leak.test.ts` pins the
// identical negative case (`'This relation does not exist in the diagram'`)
// for the shared #8132 leak predicate this heuristic was never wired to.
// This section pins the same anchor here, for BOTH of this file's readers of
// the const: the 500 gate right where it is defined, and the
// `looksLikeUnknownObject` 404 limb a few lines below it.
//
// ---------------------------------------------------------------------------
// Reverse verification, direction predicted BEFORE running
// ---------------------------------------------------------------------------
// Restoring the old `(lower.includes('relation') && lower.includes('does not
// exist'))` conjunction:
//
// §5a (decision 1, the 500 gate) RED — the business message reverts to
// `DATABASE_ERROR`/500 instead of the
// generic terminal `INTERNAL_ERROR`
// §5b (decision 2, the 404 limb) RED — the crafted unattributed-but-
// word-matching message reverts to a
// SILENT 404 `OBJECT_NOT_FOUND`
// §5c/§5d (real driver phrasings) GREEN — untouched; every case here is
// already quoted, matching both the
// old and the new predicate
//
// Both are the ordinary RED direction, not one of the inverted families.
// Measured after predicting it; the run is quoted in the PR.

describe('[#8264] anchored on the driver quoted template, not a bare conjunction', () => {
it('§5a decision 1 (the 500 gate): business prose using both words is no longer DATA_STORE_FAULT', () => {
// The card's own counter-example. No `object`, so it could never be
// attributed either way — the fixed predicate simply stops calling it
// a missing-relation condition at all, and it falls through to the
// generic terminal fault instead of the DATABASE-flavoured one.
const r = mapDataError(driverError('This relation does not exist in the diagram'));
expect(r.status).toBe(500);
expect(r.body.code).not.toBe('DATABASE_ERROR');
expect(r.body.code).toBe('INTERNAL_ERROR');
});

it('§5a holds with an object present too, and does not spill into the 404 limb either', () => {
const r = mapDataError(driverError('This relation does not exist in the diagram'), 'diagram');
expect(r.body.code).not.toBe('DATABASE_ERROR');
expect(r.body.code).not.toBe('OBJECT_NOT_FOUND');
});

it('§5b decision 2 (the 404 limb): an unquoted "relation <name> does not exist" no longer silently 404s', () => {
// Crafted to isolate decision 2, which the card's own example cannot
// reach: `missingRelationIsObject`'s Postgres branch tolerates
// UNQUOTED names (it answers a different, narrower question — which
// relation, once one is already suspected), so it still extracts
// `acct` and matches it to the object. Under the OLD predicate this
// fell through to the `looksLikeMissingRelation` OR-limb of
// `looksLikeUnknownObject` and silently answered 404 — the exact
// second consequence the card's own text never measured.
const r = mapDataError(driverError('Sorry, relation acct does not exist in our records'), 'acct');
expect(r.status).not.toBe(404);
expect(r.body.code).not.toBe('OBJECT_NOT_FOUND');
});

it('§5c quoted forms — every quote style Postgres could use — still trip the 500 gate', () => {
for (const quote of ['"', "'", '`']) {
const msg = `relation ${quote}sys_metadata${quote} does not exist`;
const r = mapDataError(driverError(msg));
expect(r.status, msg).toBe(500);
expect(r.body.code, msg).toBe('DATABASE_ERROR');
}
});

it('§5d the quoted form still attributes to 404 when the relation IS the object — decision 2, real case, unchanged', () => {
const r = mapDataError(driverError('relation "ghost" does not exist'), 'ghost');
expect(r.status).toBe(404);
expect(r.body.code).toBe('OBJECT_NOT_FOUND');
});
});
Loading