From f6e9f0c50e84992f9070d9d9f890d8e10f489334 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 15:18:53 +0000 Subject: [PATCH 1/3] fix(objectql): a plugin driver's request-shaped 4xx read refusal answers 400 with its message Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_017ihL2fT4drj3TCsV9G9CPh --- .../objectql/src/external-driver-errors.ts | 58 +++++++++++++++++-- .../src/external-driver-floor.test.ts | 58 ++++++++++++++++++- .../objectql/src/external-driver-floor.ts | 19 ++++-- packages/objectql/src/index.ts | 5 +- 4 files changed, 130 insertions(+), 10 deletions(-) diff --git a/packages/objectql/src/external-driver-errors.ts b/packages/objectql/src/external-driver-errors.ts index f82f8ed439b..90d66511eb1 100644 --- a/packages/objectql/src/external-driver-errors.ts +++ b/packages/objectql/src/external-driver-errors.ts @@ -1,10 +1,10 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * [#27277] The two refusals the engine's floor over a plugin driver raises + * [#27277] The refusals the engine's floor over a plugin driver raises * (`external-driver-floor.ts`, ADR-0015 Tier A, epic #27261 P1). * - * Both are 4xx: the caller can change the request and try again, and the + * All are 4xx: the caller can change the request and try again, and the * message is theirs to read. That is what keeps them apart from the third * outcome of the same floor, a remote that failed or did not answer, which is * `ERR_DATASOURCE_UNAVAILABLE` / 503 (`DatasourceUnavailableError`, reason @@ -13,12 +13,15 @@ * Each declares its `status` on the thrown error (the convergent HTTP-exit * convention `driver-connect-errors.ts` and `concurrent-update-error.ts` * follow), so every door answers the same status and keeps the `code`. - * ⛔ Neither is an ADR-0015 gate refusal: `EXTERNAL_WRITE_FORBIDDEN` (403) is + * ⛔ None is an ADR-0015 gate refusal: `EXTERNAL_WRITE_FORBIDDEN` (403) is * the platform's own write switch being off, fixed by turning it on; these are * fixed by changing the payload or the filter. They are registered under this - * package in `ERROR_CODE_LEDGER`, beside the gate codes' own owner. + * package in `ERROR_CODE_LEDGER`, beside the gate codes' own owner; the read + * refusal carries the standard `INVALID_QUERY` and needs no row. */ +import { StandardErrorCode } from '@objectstack/spec/api'; + /** The `code` {@link ExternalWriteRefusedError} carries. */ export const EXTERNAL_WRITE_REFUSED_CODE = 'EXTERNAL_WRITE_REFUSED' as const; @@ -50,6 +53,53 @@ export type ExternalWriteOperation = */ export const EXTERNAL_WRITE_REFUSED_STATUSES: ReadonlySet = new Set([400, 404, 409, 412, 422]); +/** + * [#27471] The statuses a plugin driver's `find` throw is read as a refusal of + * THIS read with: the remote (or the driver, before it asked — the guide's + * `refuse()`) judged the filter, the column or the resource, and a changed + * request may pass. Answered {@link ExternalReadRefusedError} with the driver's + * message. Every other status, and a throw that declares none, is a remote + * failure (`ERR_DATASOURCE_UNAVAILABLE`, 503, text withheld), for the reasons + * {@link EXTERNAL_WRITE_REFUSED_STATUSES} gives. + */ +export const EXTERNAL_READ_REFUSED_STATUSES: ReadonlySet = new Set([400, 404, 422]); + +/** + * [#27471] A plugin driver refused a read with a request-shaped 4xx + * ({@link EXTERNAL_READ_REFUSED_STATUSES}): an operator, a combinator or a + * column the remote cannot run, a resource it does not map. 400 with the + * standard `INVALID_QUERY`: the caller's query is what has to change, so the + * message is the driver's own and a retry of the same read never clears it. + * `find`, `findOne` and `count` all read through `find`, so `operation` is + * always `find`. + */ +export class ExternalReadRefusedError extends Error { + readonly code = StandardErrorCode.enum.INVALID_QUERY; + readonly status = 400; + readonly httpStatus = 400; + readonly details: { + datasource: string; + object: string; + operation: 'find'; + /** The status the driver declared on its throw. */ + remoteStatus: number; + }; + + constructor( + datasource: string, + object: string, + remoteStatus: number, + remoteMessage: string, + options?: { cause?: unknown }, + ) { + const said = remoteMessage.trim() || `it answered ${remoteStatus} with no message`; + super(`The plugin driver of datasource '${datasource}' refused this read on '${object}': ${said}`); + this.name = 'ExternalReadRefusedError'; + if (options && 'cause' in options) (this as Error & { cause?: unknown }).cause = options.cause; + this.details = { datasource, object, operation: 'find', remoteStatus }; + } +} + /** * The remote system behind a plugin driver refused a write with a request-shaped * 4xx ({@link EXTERNAL_WRITE_REFUSED_STATUSES}) — its validation failed, the diff --git a/packages/objectql/src/external-driver-floor.test.ts b/packages/objectql/src/external-driver-floor.test.ts index 5c2019eb3ad..05d2e7ab612 100644 --- a/packages/objectql/src/external-driver-floor.test.ts +++ b/packages/objectql/src/external-driver-floor.test.ts @@ -13,7 +13,9 @@ import { describe, it, expect } from 'vitest'; import type { IDataDriver } from '@objectstack/spec/contracts'; import { ObjectQL } from './engine'; -import { EXTERNAL_ROW_CAP, ExternalRowCapExceededError, ExternalWriteRefusedError } from './external-driver-errors.js'; +import { + EXTERNAL_ROW_CAP, ExternalReadRefusedError, ExternalRowCapExceededError, ExternalWriteRefusedError, +} from './external-driver-errors.js'; import { DatasourceUnavailableError } from './driver-connect-errors.js'; import { isExternalDriverFloor, needsExternalDriverFloor, unwrapExternalDriverFloor } from './external-driver-floor.js'; @@ -233,6 +235,60 @@ describe('[#27277] reads: the engine applies the query', () => { }); }); +describe('[#27471] a read the driver refuses with a request-shaped 4xx is 400 with its message', () => { + const refusingRead = (error: unknown) => makeEngine({ extra: { async find() { throw error; } } }); + /** The virtual-objects guide's `refuse()`: a by-name refusal carrying the status the floor reads. */ + const refuse = (what: string) => + Object.assign(new Error(`tree driver: the remote cannot run ${what}`), { status: 400 }); + + it('the guide\'s refuse() answers INVALID_QUERY / 400 naming what the remote cannot run, on every read', async () => { + const { engine } = refusingRead(refuse('the operator $gte on \'weight\'')); + for (const read of [ + () => engine.find('tree_node', { where: { weight: { $gte: 2 } } }), + () => engine.findOne('tree_node', { where: { weight: { $gte: 2 } } }), + () => engine.count('tree_node', { where: { weight: { $gte: 2 } } }), + ]) { + const err = await read().then(() => undefined, (e) => e); + expect(err).toBeInstanceOf(ExternalReadRefusedError); + expect(err.code).toBe('INVALID_QUERY'); + expect(err.status).toBe(400); + expect(err.message).toContain("the remote cannot run the operator $gte on 'weight'"); + expect(err.details).toEqual({ datasource: 'tree', object: 'tree_node', operation: 'find', remoteStatus: 400 }); + } + }); + + for (const status of [400, 404, 422]) { + it(`${status} (as \`status\` or \`statusCode\`) is INVALID_QUERY / 400 carrying the driver's message`, async () => { + for (const key of ['status', 'statusCode']) { + const { engine } = refusingRead(Object.assign(new Error(`remote said ${status}`), { [key]: status })); + const err = await engine.find('tree_node', {}).then(() => undefined, (e) => e); + expect(err).toBeInstanceOf(ExternalReadRefusedError); + expect(err.status).toBe(400); + expect(err.message).toContain(`remote said ${status}`); + expect(err.details.remoteStatus).toBe(status); + } + }); + } + + for (const status of [401, 403, 407, 429, 500, 502, 503, 409, 412]) { + it(`${status} stays ERR_DATASOURCE_UNAVAILABLE / 503, its text withheld`, async () => { + const { engine } = refusingRead(Object.assign(new Error('token sk_live_123 rejected'), { status })); + const err = await engine.find('tree_node', {}).then(() => undefined, (e) => e); + expect(err).toBeInstanceOf(DatasourceUnavailableError); + expect(err.code).toBe('ERR_DATASOURCE_UNAVAILABLE'); + expect(err.kind).toBe('remote'); + expect(err.message).not.toContain('sk_live_123'); + }); + } + + it('a throw that declares no status stays ERR_DATASOURCE_UNAVAILABLE / 503, its text withheld', async () => { + const { engine } = refusingRead(new Error('tree driver: the remote cannot run the combinator $or')); + const err = await engine.find('tree_node', {}).then(() => undefined, (e) => e); + expect(err).toBeInstanceOf(DatasourceUnavailableError); + expect(err.message).not.toContain('$or'); + }); +}); + describe('[#27277] writes pass through; a remote refusal is 422', () => { it('a write the remote accepts is the remote\'s answer', async () => { const { engine } = makeEngine({ diff --git a/packages/objectql/src/external-driver-floor.ts b/packages/objectql/src/external-driver-floor.ts index 4c831fc3954..1957bd34d07 100644 --- a/packages/objectql/src/external-driver-floor.ts +++ b/packages/objectql/src/external-driver-floor.ts @@ -38,10 +38,13 @@ * - Writes pass through, by id: `updateMany` / `deleteMany` are hidden too, so * no `where` the engine cannot re-check reaches a write. A write the remote * refuses with a request-shaped 4xx (`EXTERNAL_WRITE_REFUSED_STATUSES`) is - * {@link ExternalWriteRefusedError} (422, the remote's message); any other - * failure of a read or a write — an auth or rate-limit 4xx included — is - * `ERR_DATASOURCE_UNAVAILABLE` (503, reason `remote`), with the cause in the - * server log and never on the wire. + * {@link ExternalWriteRefusedError} (422, the remote's message); a `find` + * the driver refuses with one (`EXTERNAL_READ_REFUSED_STATUSES`, #27471) is + * {@link ExternalReadRefusedError} (400 `INVALID_QUERY`, the driver's + * message, cut at the engine boundary like any driver error). Any other + * failure of a read or a write — an auth or rate-limit 4xx, or a throw that + * declares no status, included — is `ERR_DATASOURCE_UNAVAILABLE` (503, + * reason `remote`), with the cause in the server log and never on the wire. * - Everything else (`connect`, `supports`, `syncSchema`, `introspectSchema`, * `beginTransaction`, …) is the driver's own, unchanged. * @@ -60,8 +63,10 @@ import { applyDriverQueryInMemory, assertDriverWhereEvaluableInMemory } from './ import { declaredFieldClasses, declaredJsonStoredFields } from './having-filter.js'; import { DatasourceUnavailableError } from './driver-connect-errors.js'; import { + EXTERNAL_READ_REFUSED_STATUSES, EXTERNAL_ROW_CAP, EXTERNAL_WRITE_REFUSED_STATUSES, + ExternalReadRefusedError, ExternalRowCapExceededError, ExternalWriteRefusedError, type ExternalWriteOperation, @@ -172,6 +177,12 @@ export function createExternalDriverFloor(driver: IDataDriver, host: ExternalDri try { rows = await inner.find(object, sent, options as Parameters[2]); } catch (error) { + const status = declaredStatus(error); + if (status !== undefined && EXTERNAL_READ_REFUSED_STATUSES.has(status)) { + throw new ExternalReadRefusedError( + host.datasource, object, status, error instanceof Error ? error.message : String(error), { cause: error }, + ); + } throw remoteFailure(object, 'find', error); } if (!Array.isArray(rows)) { diff --git a/packages/objectql/src/index.ts b/packages/objectql/src/index.ts index 1bdeb934f67..ed12d094d2b 100644 --- a/packages/objectql/src/index.ts +++ b/packages/objectql/src/index.ts @@ -301,16 +301,19 @@ export type { } from './driver-connect-errors.js'; // [#27277] The engine's floor over a plugin driver on an external datasource // (ADR-0015 Tier A): the in-memory reading of a driver query it applies, and -// the two 422 refusals it raises (registered in ERROR_CODE_LEDGER). +// the two 422 refusals it raises (registered in ERROR_CODE_LEDGER), plus +// [#27471] the 400 read refusal (the standard INVALID_QUERY). export { applyDriverQueryInMemory } from './driver-query-in-memory.js'; export type { DriverQueryInMemory, DriverQueryInMemoryOptions } from './driver-query-in-memory.js'; export { ExternalWriteRefusedError, + ExternalReadRefusedError, ExternalRowCapExceededError, EXTERNAL_WRITE_REFUSED_CODE, EXTERNAL_ROW_CAP_EXCEEDED_CODE, EXTERNAL_ROW_CAP, EXTERNAL_WRITE_REFUSED_STATUSES, + EXTERNAL_READ_REFUSED_STATUSES, } from './external-driver-errors.js'; export type { ExternalWriteOperation } from './external-driver-errors.js'; export { needsExternalDriverFloor, unwrapExternalDriverFloor } from './external-driver-floor.js'; From 9773776be4f7b5d326b700c19fc50ec2f83f89d0 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 15:23:30 +0000 Subject: [PATCH 2/3] docs(virtual-objects): refuse() sets the status the engine reads; the failure paragraph names each answer Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_017ihL2fT4drj3TCsV9G9CPh --- .changeset/27471-external-read-refusal-400.md | 12 +++++++++ .../docs/data-modeling/virtual-objects.mdx | 27 ++++++++++++------- 2 files changed, 29 insertions(+), 10 deletions(-) create mode 100644 .changeset/27471-external-read-refusal-400.md diff --git a/.changeset/27471-external-read-refusal-400.md b/.changeset/27471-external-read-refusal-400.md new file mode 100644 index 00000000000..bc0c34a7c5d --- /dev/null +++ b/.changeset/27471-external-read-refusal-400.md @@ -0,0 +1,12 @@ +--- +'@objectstack/objectql': patch +--- + +fix(objectql): a plugin driver's request-shaped read refusal answers 400 INVALID_QUERY with the driver's message, not 503 "retry later" + +Clause-②: no +Metadata-protocol: none + +On a datasource served by a plugin driver (a non-managed `schemaMode` on a driver that is not built in), a `find` the driver throws with a request-shaped status (`status` or `statusCode` 400, 404 or 422) now answers `400 INVALID_QUERY` carrying the driver's message: an operator, a combinator, a column or a resource the remote cannot run, which a changed query fixes and a retry never does. Before, every `find` throw answered `503 ERR_DATASOURCE_UNAVAILABLE` ("It may clear; retry later.") with the driver's reason only in the server log. `find`, `findOne` and `count` all read this way. + +Everything else keeps today's answer: a 5xx, an unreachable remote, a throw that declares no status, and 401, 403, 407 and 429 (a credential or rate-limit condition whose text may name the credential) stay `503 ERR_DATASOURCE_UNAVAILABLE`, the text withheld. The refusal is exported as `ExternalReadRefusedError`, with its statuses as `EXTERNAL_READ_REFUSED_STATUSES`. The virtual-objects guide's `refuse()` helper now sets `status: 400`, and its failure paragraph says what the caller sees for each status. diff --git a/content/docs/data-modeling/virtual-objects.mdx b/content/docs/data-modeling/virtual-objects.mdx index 99b4c970ce3..dab995279af 100644 --- a/content/docs/data-modeling/virtual-objects.mdx +++ b/content/docs/data-modeling/virtual-objects.mdx @@ -90,8 +90,9 @@ const RESOURCES: Record const OPERATORS: Record = { $eq: 'eq', $ne: 'ne', $in: 'in' }; +/** A refusal of this request, by name. `status: 400` is what the engine reads to tell it from an outage. */ function refuse(what: string): never { - throw new Error(`port_schedule driver: the remote cannot run ${what}`); + throw Object.assign(new Error(`port_schedule driver: the remote cannot run ${what}`), { status: 400 }); } function resourceOf(object: string) { @@ -191,15 +192,19 @@ trust a plugin driver to have applied the query: it re-applies it of an `$or` narrows the result. The skeleton above takes the stricter road and refuses instead, which is also safe. - **Fail when the remote fails.** Throw; ⛔ never return an empty array, which reads - exactly like "no rows match". A read that throws (a refusal by name included) answers - the caller `503 ERR_DATASOURCE_UNAVAILABLE` with `reason: 'remote'`. Its message says - the remote could not answer and to retry; your error's own text goes to the server log - at `warn` and never to the caller. A write the remote refuses for the request's own - sake (400, 404, 409, 412 or 422) is answered `422 EXTERNAL_WRITE_REFUSED` carrying your - error's message, so set `status` (or `statusCode`) on the error you throw. A write - that fails any other way is `503`, as a read is. That includes 401, 403, 407 and 429: - an expired credential or a rate limit is the operator's to fix, and its text may name - the credential. + exactly like "no rows match". What the caller sees depends on the `status` (or + `statusCode`) on the error you throw: + - A read refused for the request's own sake (400, 404 or 422), which is what + `refuse()` above throws, answers `400 INVALID_QUERY` carrying your error's message, + so the caller reads which operator or column to drop. A retry of the same read never + clears it. + - A write refused for the request's own sake (400, 404, 409, 412 or 422) answers + `422 EXTERNAL_WRITE_REFUSED` carrying your error's message. + - Anything else answers `503 ERR_DATASOURCE_UNAVAILABLE` with `reason: 'remote'`: a + 5xx, an unreachable remote, a throw with no `status`, and also 401, 403, 407 and 429, + because an expired credential or a rate limit is the operator's to fix and its text + may name the credential. The message says the remote could not answer and to retry; + your error's own text goes to the server log at `warn` and never to the caller. - **Send the remote only the columns it owns.** `update` receives the patch plus any field a hook stamped, plus `id`, `updated_at` and `updated_by`, which the engine adds on its own and no remote column declares. @@ -561,6 +566,8 @@ describe('port_schedule driver — the shapes the engine sends, and what the rem const remote = recordingTransport(); const driver = createPortScheduleDriver(remote); await expect(driver.find('berth_slot', { where: { $or: [{ archived: false }, { planner: 'u1' }] } } as never)).rejects.toThrow(/\$or/); + // The status the engine reads: a refusal of this request answers 400, not "retry later". + await expect(driver.find('berth_slot', { where: { $or: [{ archived: false }] } } as never)).rejects.toMatchObject({ status: 400 }); await expect(driver.find('berth_slot', { where: { start_date: { $gte: '2026-11-01' } } } as never)).rejects.toThrow(/\$gte/); // The default sharing model asks for ownership columns no remote row carries. await expect(driver.findOne('berth_slot', { where: { $and: [{ id: 's_1' }, { created_by: 'u1' }] } } as never)).rejects.toThrow(/created_by/); From e9672b771a25cc357584c0ae6e4373e7646e22ee Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 16:23:57 +0000 Subject: [PATCH 3/3] fix(objectql): a read refusal keeps the driver's registered code, and withholds its text when the where carries a policy predicate Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_017ihL2fT4drj3TCsV9G9CPh --- .changeset/27471-external-read-refusal-400.md | 4 +- .../docs/data-modeling/virtual-objects.mdx | 12 +++- .../objectql/src/external-driver-errors.ts | 47 ++++++++++---- .../src/external-driver-floor.test.ts | 59 ++++++++++++++++-- .../objectql/src/external-driver-floor.ts | 61 +++++++++++++++++-- 5 files changed, 154 insertions(+), 29 deletions(-) diff --git a/.changeset/27471-external-read-refusal-400.md b/.changeset/27471-external-read-refusal-400.md index bc0c34a7c5d..61ed9dc96ec 100644 --- a/.changeset/27471-external-read-refusal-400.md +++ b/.changeset/27471-external-read-refusal-400.md @@ -2,11 +2,11 @@ '@objectstack/objectql': patch --- -fix(objectql): a plugin driver's request-shaped read refusal answers 400 INVALID_QUERY with the driver's message, not 503 "retry later" +fix(objectql): a plugin driver's request-shaped read refusal answers 400 with its own code and message, not 503 "retry later" Clause-②: no Metadata-protocol: none -On a datasource served by a plugin driver (a non-managed `schemaMode` on a driver that is not built in), a `find` the driver throws with a request-shaped status (`status` or `statusCode` 400, 404 or 422) now answers `400 INVALID_QUERY` carrying the driver's message: an operator, a combinator, a column or a resource the remote cannot run, which a changed query fixes and a retry never does. Before, every `find` throw answered `503 ERR_DATASOURCE_UNAVAILABLE` ("It may clear; retry later.") with the driver's reason only in the server log. `find`, `findOne` and `count` all read this way. +On a datasource served by a plugin driver (a non-managed `schemaMode` on a driver that is not built in), a `find` the driver throws with a request-shaped status (`status` or `statusCode` 400, 404 or 422) now answers `400`: an operator, a combinator, a column or a resource the remote cannot run, which a changed query fixes and a retry never does. The code is the one the driver declared when it is a registered code (`INVALID_FILTER`, for example), otherwise `INVALID_QUERY`; `details.remoteCode` names what the driver declared. The driver's message reaches the caller when the read's filter is the caller's own. When the read carries a predicate the caller did not write (a sharing or row-level security scope), the message is withheld, the answer names the datasource and object and points at the server log, and the driver's text is logged at `warn`. Before, every `find` throw answered `503 ERR_DATASOURCE_UNAVAILABLE` ("It may clear; retry later.") with the driver's reason only in the server log. `find`, `findOne` and `count` all read this way. Everything else keeps today's answer: a 5xx, an unreachable remote, a throw that declares no status, and 401, 403, 407 and 429 (a credential or rate-limit condition whose text may name the credential) stay `503 ERR_DATASOURCE_UNAVAILABLE`, the text withheld. The refusal is exported as `ExternalReadRefusedError`, with its statuses as `EXTERNAL_READ_REFUSED_STATUSES`. The virtual-objects guide's `refuse()` helper now sets `status: 400`, and its failure paragraph says what the caller sees for each status. diff --git a/content/docs/data-modeling/virtual-objects.mdx b/content/docs/data-modeling/virtual-objects.mdx index dab995279af..6781a8ed81b 100644 --- a/content/docs/data-modeling/virtual-objects.mdx +++ b/content/docs/data-modeling/virtual-objects.mdx @@ -195,9 +195,15 @@ trust a plugin driver to have applied the query: it re-applies it exactly like "no rows match". What the caller sees depends on the `status` (or `statusCode`) on the error you throw: - A read refused for the request's own sake (400, 404 or 422), which is what - `refuse()` above throws, answers `400 INVALID_QUERY` carrying your error's message, - so the caller reads which operator or column to drop. A retry of the same read never - clears it. + `refuse()` above throws, answers `400`. The code is `INVALID_QUERY`, or the code you + set on the error when it is a registered one (`INVALID_FILTER`, say). A retry of the + same read never clears it. The caller reads your error's message, so they know which + operator or column to drop. Write it for them: ⛔ no remote URL, host or credential + in it. The message is withheld, and goes to the server log, when the read carries a + predicate the caller did not write, such as a sharing or row-level security scope, + because it may name that predicate's column or value. A remote `404` on a list + endpoint often means a wrong base path or resource mapping in your driver, not a bad + query. - A write refused for the request's own sake (400, 404, 409, 412 or 422) answers `422 EXTERNAL_WRITE_REFUSED` carrying your error's message. - Anything else answers `503 ERR_DATASOURCE_UNAVAILABLE` with `reason: 'remote'`: a diff --git a/packages/objectql/src/external-driver-errors.ts b/packages/objectql/src/external-driver-errors.ts index 90d66511eb1..474ea7a68fa 100644 --- a/packages/objectql/src/external-driver-errors.ts +++ b/packages/objectql/src/external-driver-errors.ts @@ -17,10 +17,11 @@ * the platform's own write switch being off, fixed by turning it on; these are * fixed by changing the payload or the filter. They are registered under this * package in `ERROR_CODE_LEDGER`, beside the gate codes' own owner; the read - * refusal carries the standard `INVALID_QUERY` and needs no row. + * refusal carries the driver's registered code or the standard `INVALID_QUERY` + * and needs no row. */ -import { StandardErrorCode } from '@objectstack/spec/api'; +import { ErrorCode, StandardErrorCode } from '@objectstack/spec/api'; /** The `code` {@link ExternalWriteRefusedError} carries. */ export const EXTERNAL_WRITE_REFUSED_CODE = 'EXTERNAL_WRITE_REFUSED' as const; @@ -57,8 +58,8 @@ export const EXTERNAL_WRITE_REFUSED_STATUSES: ReadonlySet = new Set([400 * [#27471] The statuses a plugin driver's `find` throw is read as a refusal of * THIS read with: the remote (or the driver, before it asked — the guide's * `refuse()`) judged the filter, the column or the resource, and a changed - * request may pass. Answered {@link ExternalReadRefusedError} with the driver's - * message. Every other status, and a throw that declares none, is a remote + * request may pass. Answered {@link ExternalReadRefusedError}, with the driver's + * message when the read's `where` is the caller's own. Every other status, and a throw that declares none, is a remote * failure (`ERR_DATASOURCE_UNAVAILABLE`, 503, text withheld), for the reasons * {@link EXTERNAL_WRITE_REFUSED_STATUSES} gives. */ @@ -67,14 +68,24 @@ export const EXTERNAL_READ_REFUSED_STATUSES: ReadonlySet = new Set([400, /** * [#27471] A plugin driver refused a read with a request-shaped 4xx * ({@link EXTERNAL_READ_REFUSED_STATUSES}): an operator, a combinator or a - * column the remote cannot run, a resource it does not map. 400 with the - * standard `INVALID_QUERY`: the caller's query is what has to change, so the - * message is the driver's own and a retry of the same read never clears it. + * column the remote cannot run, a resource it does not map. 400: the caller's + * query is what has to change, and a retry of the same read never clears it. * `find`, `findOne` and `count` all read through `find`, so `operation` is * always `find`. + * + * - **Code.** A driver that declared a registered ADR-0112 code (`ErrorCode`, + * the membership test the HTTP door applies to a thrown `code`) keeps it, as + * it would on a datasource the engine does not floor; any other spelling, or + * none, answers the standard `INVALID_QUERY`. `details.remoteCode` carries + * what the driver declared either way. + * - **Message.** The driver's own sentence, when the floor passes it + * (`remoteMessage` a string). `null` withholds it: the read's `where` was not + * attested as the caller's own (`filter-subtree-provenance.ts`), so the + * sentence may name a policy subtree's column or comparand. The answer is + * then the same refusal with a fixed sentence pointing at the server log. */ export class ExternalReadRefusedError extends Error { - readonly code = StandardErrorCode.enum.INVALID_QUERY; + readonly code: ErrorCode; readonly status = 400; readonly httpStatus = 400; readonly details: { @@ -83,20 +94,30 @@ export class ExternalReadRefusedError extends Error { operation: 'find'; /** The status the driver declared on its throw. */ remoteStatus: number; + /** The `code` the driver declared on its throw, kept or not; absent when it declared none. */ + remoteCode?: string; }; constructor( datasource: string, object: string, remoteStatus: number, - remoteMessage: string, - options?: { cause?: unknown }, + remoteMessage: string | null, + options?: { cause?: unknown; remoteCode?: string }, ) { - const said = remoteMessage.trim() || `it answered ${remoteStatus} with no message`; - super(`The plugin driver of datasource '${datasource}' refused this read on '${object}': ${said}`); + const head = `The plugin driver of datasource '${datasource}' refused this read on '${object}'`; + const said = remoteMessage?.trim() || `it answered ${remoteStatus} with no message`; + super(remoteMessage === null ? `${head}; its reason is in the server log.` : `${head}: ${said}`); this.name = 'ExternalReadRefusedError'; if (options && 'cause' in options) (this as Error & { cause?: unknown }).cause = options.cause; - this.details = { datasource, object, operation: 'find', remoteStatus }; + const remoteCode = options?.remoteCode; + this.code = remoteCode !== undefined && ErrorCode.safeParse(remoteCode).success + ? remoteCode as ErrorCode + : StandardErrorCode.enum.INVALID_QUERY; + this.details = { + datasource, object, operation: 'find', remoteStatus, + ...(remoteCode !== undefined ? { remoteCode } : {}), + }; } } diff --git a/packages/objectql/src/external-driver-floor.test.ts b/packages/objectql/src/external-driver-floor.test.ts index 05d2e7ab612..007028a5796 100644 --- a/packages/objectql/src/external-driver-floor.test.ts +++ b/packages/objectql/src/external-driver-floor.test.ts @@ -12,6 +12,7 @@ import { describe, it, expect } from 'vitest'; import type { IDataDriver } from '@objectstack/spec/contracts'; +import { markFilterSubtreeProvenance } from '@objectstack/spec/data'; import { ObjectQL } from './engine'; import { EXTERNAL_ROW_CAP, ExternalReadRefusedError, ExternalRowCapExceededError, ExternalWriteRefusedError, @@ -235,18 +236,20 @@ describe('[#27277] reads: the engine applies the query', () => { }); }); -describe('[#27471] a read the driver refuses with a request-shaped 4xx is 400 with its message', () => { +describe('[#27471] a read the driver refuses with a request-shaped 4xx is 400', () => { const refusingRead = (error: unknown) => makeEngine({ extra: { async find() { throw error; } } }); /** The virtual-objects guide's `refuse()`: a by-name refusal carrying the status the floor reads. */ const refuse = (what: string) => Object.assign(new Error(`tree driver: the remote cannot run ${what}`), { status: 400 }); + /** A `where` a merge boundary vouched for as the caller's own. */ + const authored = (where: T): T => markFilterSubtreeProvenance(where, 'author'); it('the guide\'s refuse() answers INVALID_QUERY / 400 naming what the remote cannot run, on every read', async () => { const { engine } = refusingRead(refuse('the operator $gte on \'weight\'')); for (const read of [ - () => engine.find('tree_node', { where: { weight: { $gte: 2 } } }), - () => engine.findOne('tree_node', { where: { weight: { $gte: 2 } } }), - () => engine.count('tree_node', { where: { weight: { $gte: 2 } } }), + () => engine.find('tree_node', { where: authored({ weight: { $gte: 2 } }) }), + () => engine.findOne('tree_node', { where: authored({ weight: { $gte: 2 } }) }), + () => engine.count('tree_node', { where: authored({ weight: { $gte: 2 } }) }), ]) { const err = await read().then(() => undefined, (e) => e); expect(err).toBeInstanceOf(ExternalReadRefusedError); @@ -258,11 +261,12 @@ describe('[#27471] a read the driver refuses with a request-shaped 4xx is 400 wi }); for (const status of [400, 404, 422]) { - it(`${status} (as \`status\` or \`statusCode\`) is INVALID_QUERY / 400 carrying the driver's message`, async () => { + it(`${status} (as \`status\` or \`statusCode\`) is 400 carrying the driver's message`, async () => { for (const key of ['status', 'statusCode']) { const { engine } = refusingRead(Object.assign(new Error(`remote said ${status}`), { [key]: status })); const err = await engine.find('tree_node', {}).then(() => undefined, (e) => e); expect(err).toBeInstanceOf(ExternalReadRefusedError); + expect(err.code).toBe('INVALID_QUERY'); expect(err.status).toBe(400); expect(err.message).toContain(`remote said ${status}`); expect(err.details.remoteStatus).toBe(status); @@ -270,6 +274,51 @@ describe('[#27471] a read the driver refuses with a request-shaped 4xx is 400 wi }); } + it('a registered code the driver declared is kept (ADR-0112), and remoteCode names it', async () => { + const { engine } = refusingRead( + Object.assign(new Error('tree driver: $regex is not a filter'), { status: 400, code: 'INVALID_FILTER' }), + ); + const err = await engine.find('tree_node', {}).then(() => undefined, (e) => e); + expect(err).toBeInstanceOf(ExternalReadRefusedError); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.details.remoteCode).toBe('INVALID_FILTER'); + }); + + it('a code that is not registered answers INVALID_QUERY, and remoteCode still names it', async () => { + const { engine } = refusingRead( + Object.assign(new Error('tree driver: no such filter'), { status: 400, code: 'TREE_BAD_FILTER' }), + ); + const err = await engine.find('tree_node', {}).then(() => undefined, (e) => e); + expect(err.code).toBe('INVALID_QUERY'); + expect(err.details.remoteCode).toBe('TREE_BAD_FILTER'); + }); + + it('a where no boundary vouched for is refused 400 with its reason withheld and logged', async () => { + const warned: Array | undefined> = []; + const { engine } = refusingRead(refuse("a filter on 'region' with comparand EMEA")); + (engine as unknown as { logger: { warn: (m: string, meta?: Record) => void } }).logger.warn = + (_m, meta) => { warned.push(meta); }; + const err = await engine.find('tree_node', { where: { kind: 'leaf' } }).then(() => undefined, (e) => e); + expect(err).toBeInstanceOf(ExternalReadRefusedError); + expect(err.status).toBe(400); + expect(err.message).not.toContain('EMEA'); + expect(err.message).toContain('server log'); + expect(warned.some((meta) => String(meta?.error).includes('EMEA'))).toBe(true); + }); + + it('a policy arm under an author root withholds the reason (#7929 fail-closed)', async () => { + const { engine } = refusingRead(refuse("a filter on 'region' with comparand EMEA")); + const where = authored({ + $and: [authored({ kind: 'leaf' }), markFilterSubtreeProvenance({ region: 'EMEA' }, 'policy')], + }); + const err = await engine.find('tree_node', { where }).then(() => undefined, (e) => e); + expect(err).toBeInstanceOf(ExternalReadRefusedError); + expect(err.status).toBe(400); + expect(err.message).not.toContain('EMEA'); + expect(err.details.remoteStatus).toBe(400); + }); + for (const status of [401, 403, 407, 429, 500, 502, 503, 409, 412]) { it(`${status} stays ERR_DATASOURCE_UNAVAILABLE / 503, its text withheld`, async () => { const { engine } = refusingRead(Object.assign(new Error('token sk_live_123 rejected'), { status })); diff --git a/packages/objectql/src/external-driver-floor.ts b/packages/objectql/src/external-driver-floor.ts index 1957bd34d07..49812cf4b92 100644 --- a/packages/objectql/src/external-driver-floor.ts +++ b/packages/objectql/src/external-driver-floor.ts @@ -40,8 +40,10 @@ * refuses with a request-shaped 4xx (`EXTERNAL_WRITE_REFUSED_STATUSES`) is * {@link ExternalWriteRefusedError} (422, the remote's message); a `find` * the driver refuses with one (`EXTERNAL_READ_REFUSED_STATUSES`, #27471) is - * {@link ExternalReadRefusedError} (400 `INVALID_QUERY`, the driver's - * message, cut at the engine boundary like any driver error). Any other + * {@link ExternalReadRefusedError} (400, the driver's registered code or + * `INVALID_QUERY`; the driver's message only when the sent `where` is + * attested as the caller's own, else a fixed sentence and the message in the + * server log). Any other * failure of a read or a write — an auth or rate-limit 4xx, or a throw that * declares no status, included — is `ERR_DATASOURCE_UNAVAILABLE` (503, * reason `remote`), with the cause in the server log and never on the wire. @@ -58,7 +60,7 @@ */ import type { DriverQuery, IDataDriver } from '@objectstack/spec/contracts'; -import { resolveDriverId } from '@objectstack/spec/data'; +import { filterSubtreeProvenanceOf, resolveDriverId } from '@objectstack/spec/data'; import { applyDriverQueryInMemory, assertDriverWhereEvaluableInMemory } from './driver-query-in-memory.js'; import { declaredFieldClasses, declaredJsonStoredFields } from './having-filter.js'; import { DatasourceUnavailableError } from './driver-connect-errors.js'; @@ -164,6 +166,29 @@ export function createExternalDriverFloor(driver: IDataDriver, host: ExternalDri return refusal; }; + /** + * [#27471] A `find` the driver refused with a request-shaped 4xx. Its sentence + * reaches the caller only when every predicate it can be about is the + * caller's own ({@link whereIsCallersOwn}); otherwise it goes to the log, as + * a remote failure's cause does, and the caller reads a fixed sentence. + */ + const readRefusal = (object: string, where: unknown, status: number, error: unknown): Error => { + const message = error instanceof Error ? error.message : String(error); + const disclosed = whereIsCallersOwn(where); + if (!disclosed) { + host.logger.warn( + `[ObjectQL] the plugin driver of datasource '${host.datasource}' refused a find on '${object}'; ` + + 'the caller was answered without this reason, because the read carries a predicate the caller did not write', + { datasource: host.datasource, object, operation: 'find', status, error: message }, + ); + } + const { code } = (error ?? {}) as { code?: unknown }; + return new ExternalReadRefusedError(host.datasource, object, status, disclosed ? message : null, { + cause: error, + ...(typeof code === 'string' ? { remoteCode: code } : {}), + }); + }; + const readCapped = async (object: string, query: DriverQuery | undefined, options: unknown) => { assertDriverWhereEvaluableInMemory(query?.where); const routed = (query as { object?: unknown } | undefined)?.object; @@ -179,9 +204,7 @@ export function createExternalDriverFloor(driver: IDataDriver, host: ExternalDri } catch (error) { const status = declaredStatus(error); if (status !== undefined && EXTERNAL_READ_REFUSED_STATUSES.has(status)) { - throw new ExternalReadRefusedError( - host.datasource, object, status, error instanceof Error ? error.message : String(error), { cause: error }, - ); + throw readRefusal(object, sent.where, status, error); } throw remoteFailure(object, 'find', error); } @@ -256,6 +279,32 @@ function withoutRouting(query: DriverQuery): DriverQuery { return rest as DriverQuery; } +/** + * [#27471] Is every predicate in `where` the caller's own, so a driver's refusal + * of it may be quoted back? Yes when there is no `where`, or when its root is + * marked `'author'` and no node under it is marked `'policy'`. Anything else — + * an unmarked root (no merge boundary vouched for it), a policy arm under an + * author root — answers no, the fail-closed direction + * `filter-subtree-provenance.ts` requires. Walks plain objects and arrays, and + * stops on a cycle. + */ +function whereIsCallersOwn(where: unknown): boolean { + if (where === undefined) return true; + if (filterSubtreeProvenanceOf(where) !== 'author') return false; + const onPath = new Set(); + const carriesPolicy = (node: unknown): boolean => { + if (node === null || typeof node !== 'object' || onPath.has(node)) return false; + if (filterSubtreeProvenanceOf(node) === 'policy') return true; + onPath.add(node); + try { + return Object.values(node as Record).some(carriesPolicy); + } finally { + onPath.delete(node); + } + }; + return !carriesPolicy(where); +} + /** The HTTP status a thrown error declares, read as the repo's HTTP exits read it: `status`, then `statusCode`. */ function declaredStatus(error: unknown): number | undefined { const { status, statusCode } = (error ?? {}) as { status?: unknown; statusCode?: unknown };