Skip to content

Commit cb1e01e

Browse files
committed
wip(drivers): give the uncompilable-filter refusal an ADR-0112 code and drop the driver prefix (#4436)
IN PROGRESS — code change complete, regression test not yet written and the real-boot curl repro not yet run. A filter carrying an operator the driver cannot compile is already REFUSED rather than silently matched (#4209/#4029), but the refusal had no wire identity: the thrown `Error` carried no `code`, so `mapDataError`'s default branch served `{"error": "[sql-driver] Unsupported filter operator …"}` — no `error.code` at all, breaking the ADR-0112 contract every sibling rejection on the same route already honours (`INVALID_FIELD`, `INVALID_FILTER`, `RECORD_NOT_FOUND`), and leaking the `[sql-driver]` internal prefix that the #3867 sanitiser exists to keep off the wire. Both drivers now throw through a local `unsupportedFilterError` that stamps `code = StandardErrorCode.enum.INVALID_FILTER` (the same catalogued code `metadata-protocol` emits when a filter fails to parse upstream — one condition, one wire code however the caller reached it) and `status = 400`, which also puts the rejection on `isExpectedQueryRejection` so a client mistake stops being logged as an unhandled server error. The internal prefix is gone from the message; the actionable operator/field/vocabulary detail stays. Applied to every filter-COMPILATION refusal in both backends, not just the one branch the issue names — they are the same envelope defect on adjacent lines, and #3948 made the two drivers agree that an uncompilable filter is a refusal, so their refusal envelopes have to agree too. TODO: regression tests (driver-sql, driver-memory, REST envelope) + boot repro. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD
1 parent f566a1d commit cb1e01e

2 files changed

Lines changed: 67 additions & 13 deletions

File tree

packages/plugins/driver-memory/src/memory-driver.ts

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import type { QueryAST, QueryInput, DriverOptions } from '@objectstack/spec/data';
44
import { canonicalAstOperator } from '@objectstack/spec/data';
55
import type { IDataDriver } from '@objectstack/spec/contracts';
6+
import { StandardErrorCode } from '@objectstack/spec/api';
67
import { Logger, createLogger, nextUtcCalendarDay } from '@objectstack/core';
78
import { Query, Aggregator } from 'mingo';
89
import { getValueByPath } from './memory-matcher.js';
@@ -12,6 +13,24 @@ import {
1213
type TemporalFieldKind,
1314
} from './memory-temporal.js';
1415

16+
/**
17+
* [#4436] A filter this driver cannot COMPILE — see the twin in
18+
* `driver-sql`'s `unsupportedFilterError`, which carries the full rationale.
19+
*
20+
* Kept in lockstep with driver-sql deliberately: #3948 made the two backends
21+
* AGREE that an uncompilable filter is a refusal rather than a silent
22+
* match-everything, and the refusal's wire envelope has to agree too. A test
23+
* suite that swaps the memory driver for SQL must see the same `400
24+
* INVALID_FILTER`, not a coded refusal on one backend and a bare `{error}` on
25+
* the other.
26+
*/
27+
function unsupportedFilterError(message: string): Error {
28+
const err = new Error(message) as Error & { code?: string; status?: number };
29+
err.code = StandardErrorCode.enum.INVALID_FILTER;
30+
err.status = 400;
31+
return err;
32+
}
33+
1534
/**
1635
* Persistence adapter interface.
1736
* Matches the PersistenceAdapterSchema contract from @objectstack/spec.
@@ -764,8 +783,8 @@ export class InMemoryDriver implements IDataDriver {
764783
// matches EVERY record. An unapplied filter must not look like a
765784
// satisfied one. #3948.
766785
if (lower !== 'and' && lower !== 'or') {
767-
throw new Error(
768-
`[driver-memory] Unrecognized filter operator "${item}" in a comparison triple. ` +
786+
throw unsupportedFilterError(
787+
`Unrecognized filter operator "${item}" in a comparison triple. ` +
769788
`A filter array is either a logical node (["and"|"or", …]) or nested ` +
770789
`conditions ([[field, op, value], …]); a bare [field, op, value] only ` +
771790
`reaches the driver when its operator is outside @objectstack/spec ` +
@@ -785,8 +804,8 @@ export class InMemoryDriver implements IDataDriver {
785804
const cond = this.convertConditionToMongo(field, operator, value, object);
786805
if (cond) logicGroups[logicGroups.length - 1].conditions.push(cond);
787806
} else {
788-
throw new Error(
789-
`[driver-memory] Unrecognized filter element of type ` +
807+
throw unsupportedFilterError(
808+
`Unrecognized filter element of type ` +
790809
`"${item === null ? 'null' : typeof item}" — expected a logical keyword ` +
791810
`("and"/"or") or a condition array. Filter was: ${JSON.stringify(filters)}`,
792811
);
@@ -874,16 +893,16 @@ export class InMemoryDriver implements IDataDriver {
874893
: { $gte: store(value[0]), $lte: store(value[1]) },
875894
};
876895
}
877-
throw new Error(
878-
`[driver-memory] "between" on field "${field}" needs a two-element array, got ` +
896+
throw unsupportedFilterError(
897+
`"between" on field "${field}" needs a two-element array, got ` +
879898
`${JSON.stringify(value)}. Returning no predicate would silently match every record.`,
880899
);
881900
default:
882901
// Was `return null`, which the caller dropped — so an operator this
883902
// driver cannot express narrowed nothing instead of erroring. driver-sql
884903
// already threw on the same input; the two backends disagreed. #3948.
885-
throw new Error(
886-
`[driver-memory] Unsupported filter operator "${operator}" on field "${field}". ` +
904+
throw unsupportedFilterError(
905+
`Unsupported filter operator "${operator}" on field "${field}". ` +
887906
`Supported operators: =, !=, <, <=, >, >=, in, nin, between, contains, ` +
888907
`not_contains, starts_with, ends_with (see @objectstack/spec VALID_AST_OPERATORS).`,
889908
);

packages/plugins/driver-sql/src/sql-driver.ts

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { parseAutonumberFormat, renderAutonumber, missingFieldValues, isTenancyD
1212
import { STRUCTURED_JSON_TYPES, FILE_REFERENCE_TYPES, MULTI_OPTION_TYPES, NUMERIC_VALUE_TYPES } from '@objectstack/spec/data';
1313
import { canonicalAstOperator } from '@objectstack/spec/data';
1414
import type { IDataDriver } from '@objectstack/spec/contracts';
15+
import { StandardErrorCode } from '@objectstack/spec/api';
1516
import { StorageNameMapping } from '@objectstack/spec/system';
1617
import { ExternalSchemaModeViolationError } from '@objectstack/spec/shared';
1718
import { resolveMultiOrgEnabled } from '@objectstack/types';
@@ -378,6 +379,40 @@ function canonicalTimeOfDay(value: unknown): unknown {
378379
*/
379380
const SQLITE_TIME_EXPR_REFS = 8;
380381

382+
/**
383+
* [#4436] A filter this driver cannot COMPILE — the caller sent an operator (or
384+
* an operand shape) outside what the backend can express.
385+
*
386+
* This is a refusal the request caused, and #4209/#4029 already made it a
387+
* refusal rather than a silent match-everything. What was missing is the wire
388+
* IDENTITY of that refusal: the thrown `Error` carried no `code`, so
389+
* `mapDataError`'s default branch served `{ "error": "<message>" }` with no
390+
* `code` at all — breaking the ADR-0112 contract that `error.code` is the
391+
* schema-enforced SCREAMING_SNAKE vocabulary every sibling rejection on this
392+
* route already speaks (`INVALID_FIELD`, `INVALID_FILTER`, `RECORD_NOT_FOUND`).
393+
*
394+
* `INVALID_FILTER` is the catalogued code for the condition, and the SAME one
395+
* `metadata-protocol` emits for a filter that fails to parse upstream
396+
* (`malformedFilterArrayError` / `unusableFilterError`): one condition — "this
397+
* filter cannot run" — has one wire code however the caller reached it.
398+
*
399+
* `status: 400` makes `@objectstack/rest`'s `sendError` pass the message
400+
* through instead of routing it to the SQL-leak heuristic, and puts the
401+
* rejection on the `isExpectedQueryRejection` list so a client mistake stops
402+
* being logged as an unhandled server error.
403+
*
404+
* The `[sql-driver]` prefix these messages used to carry is GONE from the text:
405+
* it is driver-internal wording, and shipping it to clients is exactly what the
406+
* #3867 sanitiser exists to stop. The operator/field/vocabulary detail — the
407+
* part a caller can act on — stays.
408+
*/
409+
function unsupportedFilterError(message: string): Error {
410+
const err = new Error(message) as Error & { code?: string; status?: number };
411+
err.code = StandardErrorCode.enum.INVALID_FILTER;
412+
err.status = 400;
413+
return err;
414+
}
415+
381416
// ── Introspection Types ──────────────────────────────────────────────────────
382417

383418
export interface IntrospectedColumn {
@@ -5259,7 +5294,7 @@ export class SqlDriver implements IDataDriver {
52595294
case 'between': {
52605295
const arr = Array.isArray(coerced) ? coerced : [];
52615296
if (arr.length !== 2) {
5262-
throw new Error(`[sql-driver] operator "between" on field "${field}" requires a [min, max] value array.`);
5297+
throw unsupportedFilterError(`Operator "between" on field "${field}" requires a [min, max] value array.`);
52635298
}
52645299
builder[join === 'or' ? 'orWhereBetween' : 'whereBetween'](field, arr as [any, any]);
52655300
return;
@@ -5298,8 +5333,8 @@ export class SqlDriver implements IDataDriver {
52985333
builder[whereNotNull](field);
52995334
return;
53005335
default:
5301-
throw new Error(
5302-
`[sql-driver] Unsupported filter operator "${op}" on field "${field}". Supported operators: ` +
5336+
throw unsupportedFilterError(
5337+
`Unsupported filter operator "${op}" on field "${field}". Supported operators: ` +
53035338
`=, !=, <, <=, >, >=, in, nin, between, contains, not_contains, starts_with, ends_with, ` +
53045339
`is_null, is_not_null (see @objectstack/spec VALID_AST_OPERATORS).`,
53055340
);
@@ -5472,8 +5507,8 @@ export class SqlDriver implements IDataDriver {
54725507
: (logicalOp === 'or' ? 'orWhereNotNull' : 'whereNotNull')](field);
54735508
break;
54745509
default:
5475-
throw new Error(
5476-
`[sql-driver] Unsupported filter operator "${op}" on field "${field}". Supported operators: ` +
5510+
throw unsupportedFilterError(
5511+
`Unsupported filter operator "${op}" on field "${field}". Supported operators: ` +
54775512
`$eq, $ne, $gt, $gte, $lt, $lte, $in, $nin, $between, $contains, $notContains, ` +
54785513
`$startsWith, $endsWith, $regex, $null, $exists.`,
54795514
);

0 commit comments

Comments
 (0)