Skip to content

Commit e7bccaa

Browse files
os-zhuangclaude
andauthored
fix(rest): anchor looksLikeMissingRelation on the driver's quoted template (#8264) (#8644)
`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 matched. Anchored on the quoted identifier Postgres always emits, mirroring #8132's anchor for the shared `looksLikeInternalErrorLeak` predicate in @objectstack/types (deliberately NOT reused here — it answers a different question and its other limbs are unrelated to this file's 404-vs-500 attribution). Both call sites of the predicate are covered: the DATA_STORE_FAULT (500) gate the issue named, and the looksLikeUnknownObject (404) limb the issue's own text did not measure. Tests pin both decision paths. Claude-Session: https://claude.ai/code/session_01P7vaLs7bhBPi9m3JyzkhDj Co-authored-by: Claude <noreply@anthropic.com>
1 parent 56724c5 commit e7bccaa

3 files changed

Lines changed: 158 additions & 1 deletion

File tree

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
---
2+
"@objectstack/rest": patch
3+
---
4+
5+
fix(rest): anchor `looksLikeMissingRelation` on the driver's quoted template (#8264)
6+
7+
`mapDataError`'s Postgres limb read `relation` and `does not exist` anywhere in
8+
the message, not necessarily the same sentence — so ordinary business prose
9+
using both words (`This relation does not exist in the diagram`) matched.
10+
`does not exist` is ordinary business English; #8132 already anchored the
11+
shared `@objectstack/types` leak predicate on the driver's own quoted
12+
template for exactly this reason, and pinned the identical string as a
13+
negative case. This file's copy of the same question was not covered by that
14+
change (different package, different call site) and kept the loose reading.
15+
16+
Anchored the same way here — a quoted identifier required between `relation`
17+
and `does not exist` — as a locally-owned pattern rather than a call into the
18+
shared leak predicate: that
19+
predicate answers a different question ("may this be withheld from the
20+
client"), and its other limbs (`sqlite_`, `unique constraint`, `foreign key`,
21+
a bare SQL statement) have nothing to do with this file's question (is this
22+
specifically an unknown-relation condition, for the 404-vs-500 split
23+
`looksLikeMissingRelation` feeds). `relation-sub-object.ts` documents "two
24+
widths, on purpose" for a neighbouring pair of consumers that ask genuinely
25+
different questions; that does not extend to the two USES inside this file,
26+
which both ask the same question and share one predicate correctly.
27+
28+
**Both of the predicate's two call sites are covered, not just the reported
29+
one:** the `DATA_STORE_FAULT` (500) gate the issue named, and the
30+
`looksLikeUnknownObject` (404) limb the issue's own text did not measure. A
31+
business message no longer gets mislabelled a `DATABASE_ERROR`, and a
32+
crafted unquoted-but-attributable message no longer gets silently answered
33+
`OBJECT_NOT_FOUND` — both now fall through to the generic, still-sanitised
34+
terminal fault, which is the direction the branch's own #5462 comment already
35+
argues for ("the safe way to be wrong is loud").
36+
37+
No reachable production path producing the unanchored shape was found at this
38+
call site — this is consistency/invariant restoration between two spellings
39+
of one question, not a fix for a demonstrated live misclassification.

packages/rest/src/rest-server.ts

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,17 @@ function declaredHttpStatus(error: any): number | undefined {
437437
return declared;
438438
}
439439

440+
/**
441+
* [#8264] Postgres' missing-relation template, anchored on the QUOTED
442+
* identifier the driver always emits — never on the bare "does not exist"
443+
* tail, which is ordinary business English. Module-scoped (not re-compiled
444+
* per {@link mapDataError} call) and named, not inlined, so both of its
445+
* readers share the literal same pattern. See the long note above
446+
* `looksLikeMissingRelation`'s definition, further down this file, for why
447+
* this is one width, not "two widths, on purpose".
448+
*/
449+
const RELATION_DOES_NOT_EXIST = /\brelation\s+["'`][^"'`]+["'`]\s+does not exist/i;
450+
440451
function missingRelationIsObject(raw: string, object: string | undefined): boolean {
441452
if (!object) return false;
442453
const named =
@@ -1051,9 +1062,35 @@ export function mapDataError(error: any, object?: string): { status: number; bod
10511062
// `code: 'OBJECT_NOT_FOUND'` and is matched far above) is the primary
10521063
// producer of this 404 anyway; the driver-string limb has been a legacy
10531064
// safety net since.
1065+
//
1066+
// [#8264] The Postgres limb used to be a two-`includes()` conjunction —
1067+
// `relation` and `does not exist` anywhere in the message, not necessarily
1068+
// the same sentence. `does not exist` is ordinary business English ("This
1069+
// relation does not exist in the diagram" — the exact negative case
1070+
// `error-leak.test.ts` pins for #8132's shared leak predicate), so that
1071+
// reading could re-verdict a legitimate business message through EITHER
1072+
// consumer below: the 500 gate right here, or the `looksLikeUnknownObject`
1073+
// 404 limb two lines further down (both read this same const). Anchored on
1074+
// Postgres' own errmsg template — a QUOTED identifier — the same technique
1075+
// #8132 used for `looksLikeInternalErrorLeak` in `@objectstack/types`.
1076+
//
1077+
// Deliberately NOT a call into that shared predicate: it answers a
1078+
// different question ("may this message be withheld from the client at
1079+
// all?"), and its other limbs — `sqlite_`, `unique constraint`,
1080+
// `foreign key`, a bare SQL statement — have nothing to do with THIS
1081+
// question (is this specifically an unknown-relation condition, for the
1082+
// 404-vs-500 split below?). `relation-sub-object.ts` documents "two
1083+
// widths, on purpose" for a neighbouring pair of consumers for exactly
1084+
// this reason — different questions get different patterns even when they
1085+
// share a substring. That precedent does NOT extend to the two USES right
1086+
// here, though: both the 500 gate and the 404 limb are asking this file's
1087+
// one question, and `missingRelationIsObject` below already gates the 500
1088+
// path on attribution — so one width for both is correct, not "two
1089+
// widths, on purpose" a second time. See the reverse-verification note in
1090+
// `rest-unknown-object-heuristic.test.ts` for both paths measured.
10541091
const looksLikeMissingRelation =
10551092
lower.includes('no such table') ||
1056-
(lower.includes('relation') && lower.includes('does not exist')) ||
1093+
RELATION_DOES_NOT_EXIST.test(raw) ||
10571094
lower.includes('table not found');
10581095
if (looksLikeMissingRelation && !missingRelationIsObject(raw, object)) {
10591096
return DATA_STORE_FAULT();

packages/rest/src/rest-unknown-object-heuristic.test.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,3 +492,84 @@ describe('[#5462] the declared-status band is untouched', () => {
492492
expect(r.body.field).toBe('label');
493493
});
494494
});
495+
496+
// ---------------------------------------------------------------------------
497+
// 5. [#8264] The Postgres limb is anchored on the quoted template — both
498+
// decision paths this one const feeds
499+
// ---------------------------------------------------------------------------
500+
//
501+
// `looksLikeMissingRelation` used to read `relation` and `does not exist`
502+
// anywhere in the message, not necessarily the same sentence, so ordinary
503+
// business prose using both words matched — `error-leak.test.ts` pins the
504+
// identical negative case (`'This relation does not exist in the diagram'`)
505+
// for the shared #8132 leak predicate this heuristic was never wired to.
506+
// This section pins the same anchor here, for BOTH of this file's readers of
507+
// the const: the 500 gate right where it is defined, and the
508+
// `looksLikeUnknownObject` 404 limb a few lines below it.
509+
//
510+
// ---------------------------------------------------------------------------
511+
// Reverse verification, direction predicted BEFORE running
512+
// ---------------------------------------------------------------------------
513+
// Restoring the old `(lower.includes('relation') && lower.includes('does not
514+
// exist'))` conjunction:
515+
//
516+
// §5a (decision 1, the 500 gate) RED — the business message reverts to
517+
// `DATABASE_ERROR`/500 instead of the
518+
// generic terminal `INTERNAL_ERROR`
519+
// §5b (decision 2, the 404 limb) RED — the crafted unattributed-but-
520+
// word-matching message reverts to a
521+
// SILENT 404 `OBJECT_NOT_FOUND`
522+
// §5c/§5d (real driver phrasings) GREEN — untouched; every case here is
523+
// already quoted, matching both the
524+
// old and the new predicate
525+
//
526+
// Both are the ordinary RED direction, not one of the inverted families.
527+
// Measured after predicting it; the run is quoted in the PR.
528+
529+
describe('[#8264] anchored on the driver quoted template, not a bare conjunction', () => {
530+
it('§5a decision 1 (the 500 gate): business prose using both words is no longer DATA_STORE_FAULT', () => {
531+
// The card's own counter-example. No `object`, so it could never be
532+
// attributed either way — the fixed predicate simply stops calling it
533+
// a missing-relation condition at all, and it falls through to the
534+
// generic terminal fault instead of the DATABASE-flavoured one.
535+
const r = mapDataError(driverError('This relation does not exist in the diagram'));
536+
expect(r.status).toBe(500);
537+
expect(r.body.code).not.toBe('DATABASE_ERROR');
538+
expect(r.body.code).toBe('INTERNAL_ERROR');
539+
});
540+
541+
it('§5a holds with an object present too, and does not spill into the 404 limb either', () => {
542+
const r = mapDataError(driverError('This relation does not exist in the diagram'), 'diagram');
543+
expect(r.body.code).not.toBe('DATABASE_ERROR');
544+
expect(r.body.code).not.toBe('OBJECT_NOT_FOUND');
545+
});
546+
547+
it('§5b decision 2 (the 404 limb): an unquoted "relation <name> does not exist" no longer silently 404s', () => {
548+
// Crafted to isolate decision 2, which the card's own example cannot
549+
// reach: `missingRelationIsObject`'s Postgres branch tolerates
550+
// UNQUOTED names (it answers a different, narrower question — which
551+
// relation, once one is already suspected), so it still extracts
552+
// `acct` and matches it to the object. Under the OLD predicate this
553+
// fell through to the `looksLikeMissingRelation` OR-limb of
554+
// `looksLikeUnknownObject` and silently answered 404 — the exact
555+
// second consequence the card's own text never measured.
556+
const r = mapDataError(driverError('Sorry, relation acct does not exist in our records'), 'acct');
557+
expect(r.status).not.toBe(404);
558+
expect(r.body.code).not.toBe('OBJECT_NOT_FOUND');
559+
});
560+
561+
it('§5c quoted forms — every quote style Postgres could use — still trip the 500 gate', () => {
562+
for (const quote of ['"', "'", '`']) {
563+
const msg = `relation ${quote}sys_metadata${quote} does not exist`;
564+
const r = mapDataError(driverError(msg));
565+
expect(r.status, msg).toBe(500);
566+
expect(r.body.code, msg).toBe('DATABASE_ERROR');
567+
}
568+
});
569+
570+
it('§5d the quoted form still attributes to 404 when the relation IS the object — decision 2, real case, unchanged', () => {
571+
const r = mapDataError(driverError('relation "ghost" does not exist'), 'ghost');
572+
expect(r.status).toBe(404);
573+
expect(r.body.code).toBe('OBJECT_NOT_FOUND');
574+
});
575+
});

0 commit comments

Comments
 (0)