diff --git a/.changeset/datasource-availability-observability.md b/.changeset/datasource-availability-observability.md new file mode 100644 index 0000000000..1efc409e22 --- /dev/null +++ b/.changeset/datasource-availability-observability.md @@ -0,0 +1,79 @@ +--- +"@objectstack/service-datasource": minor +"@objectstack/objectql": minor +"@objectstack/rest": patch +"@objectstack/runtime": patch +--- + +feat(datasource): a datasource that is down is visible, and says why when queried (#3827, #3828) + +#3816 made an explicitly-bound datasource that cannot connect refuse the boot. Two +gaps survived that fix, both in the cases that still boot — a policy denial, an +`autoConnect` datasource, or any failure the operator waved through with +`OS_ALLOW_DRIVER_CONNECT_FAILURE`: + +- **It was invisible.** `DatasourceSummary.status` was the literal `'unvalidated'` + for every row — the contract declared three states and the implementation only + ever emitted one — so a dead datasource looked exactly like a healthy-untested + one. `checkDriversHealth()` could not help either: it iterates registered + drivers, and a datasource that never connected was never registered, so it is + *absent* from the probe rather than unhealthy. The only trace was a warning + that scrolled past at boot, which made the diagnostic procedure "restart the + server and re-read the logs". +- **The query-time error said nothing.** `getDriver()` answered four different + situations with one sentence, `Datasource 'x' is not registered.`: refused by + policy, failed to connect under the escape hatch, a misspelled name, and + `active: false`. Only the third is an authoring bug, so the other three sent + the reader hunting for a typo that does not exist. + +Both come from the same root: `connect()` already produced a `ConnectResult` for +every attempt and every caller threw it away. + +- **`DatasourceConnectionService` retains the last verdict per datasource**, with a + coarse `availability` (`available` / `blocked` / `failed` / `unattempted`) beside + the raw status. New `getConnectionState(name)` / `listConnectionStates()`. + `disconnect()` drops it, so a removed pool stops explaining itself. +- **`DatasourceSummary.status` tells the truth**: `ok` | `error` | `blocked` | + `unvalidated`, with a new operator-facing `statusReason`. `blocked` is new and + deliberate — a policy denial is a decision, not a fault, and will not clear on + its own. Reported in **Setup → Datasources**, `GET /api/v1/datasources`, and the + summary returned from create/update, so a "Save" whose pool failed to open is no + longer presented as success. +- **`ERR_DATASOURCE_UNAVAILABLE` (HTTP 503)**: new `DatasourceUnavailableError` + from `@objectstack/objectql`, thrown by `getDriver()` when the connection layer + recorded *why* a declared datasource has no driver. An undeclared name keeps the + original message — there is genuinely nothing to add. 503 rather than 500/400: + nothing about the request is wrong, and the state may clear. +- **A privileged/public split for the reason.** The error **never** carries the + underlying cause — connect failures routinely contain hosts, ports and DSNs, and + a policy's `reason` is written for operators. Those stay in the logs and the + (admin-gated) datasource list. `DatasourceConnectDecision` gains an opt-in + `publicReason` for hosts that want to tell tenants something specific + (e.g. `'External datasources require the Scale plan.'`); it is the only string + that reaches an end user. +- **Readiness is deliberately not gated on this.** `/ready` still reflects + registered-driver health only: an optional datasource being down must not pull an + otherwise-working replica out of the load balancer. + +Also lands a drift guard for **#3826**, and corrects ADR-0062's status while doing +it. The ADR claimed D1 ("exactly one definition → live driver path") as +implemented; only the *construction* half converged. The `default` driver is still +registered as a `driver.*` kernel service and connected by `ObjectQLEngine.init()`, +with its own failure verdict, pool teardown, and no connect policy. What blocks the +merge is an input-shape mismatch, not ordering: `connect()` takes a datasource +*definition* and builds the driver, while `default` arrives pre-built, and routing +it through the service would make `ObjectQLPlugin`'s boot depend on an optional +higher-layer service. Until that is designed, `degraded-boot-parity.test.ts` pins +both paths to the same operator-visible contract (fail-fast by default, identical +`OS_ALLOW_DRIVER_CONNECT_FAILURE` parsing, `DEGRADED BOOT` on stderr) so a change +to one that forgets the other fails CI — #3741 → #3758 was exactly that miss, and +it cost three months and a second bug report. + +**Migration.** Additive. `DatasourceSummary.status` gains a `'blocked'` member: a +consumer exhaustively switching on it needs a case (the admin UI shows it as a +distinct state). Nothing that was `'ok'` or `'error'` changes meaning; rows that +were reported `'unvalidated'` now report their real state. Query-time errors for a +datasource the connection layer recorded change from a generic `Error` to +`DatasourceUnavailableError` (503 instead of the previous catch-all status); +matching on the old `is not registered` text still works for the undeclared-name +case, which is the only one that was ever accurate. diff --git a/content/docs/api/error-catalog.mdx b/content/docs/api/error-catalog.mdx index dba8b340e9..018058e5ef 100644 --- a/content/docs/api/error-catalog.mdx +++ b/content/docs/api/error-catalog.mdx @@ -13,7 +13,7 @@ ObjectStack uses a structured error system with **9 error categories** and **51 -**Spec vs. wire format:** The codes below are `StandardErrorCode` — the standardized, lowercase snake_case contract that plugin/hook authors should throw with (see [Server-Side Error Handling](/docs/api/error-handling-server)). The kernel REST server (`@objectstack/rest`) that serves `/api/v1/data/*` today emits a flatter envelope with SCREAMING_SNAKE_CASE codes instead — e.g. `VALIDATION_FAILED`, `PERMISSION_DENIED`, `RECORD_NOT_FOUND`, `CONCURRENT_UPDATE` — not the codes in this catalog. See the [API Overview](/docs/api#error-handling) for both wire formats in use and [Wire Format](/docs/api/wire-format#7-error-response-format) for JSON examples. +**Spec vs. wire format:** The codes below are `StandardErrorCode` — the standardized, lowercase snake_case contract that plugin/hook authors should throw with (see [Server-Side Error Handling](/docs/api/error-handling-server)). The kernel REST server (`@objectstack/rest`) that serves `/api/v1/data/*` today emits a flatter envelope with SCREAMING_SNAKE_CASE codes instead — e.g. `VALIDATION_FAILED`, `PERMISSION_DENIED`, `RECORD_NOT_FOUND`, `CONCURRENT_UPDATE`, `ERR_DATASOURCE_UNAVAILABLE` — not the codes in this catalog. See the [API Overview](/docs/api#error-handling) for both wire formats in use and [Wire Format](/docs/api/wire-format#7-error-response-format) for JSON examples. --- diff --git a/content/docs/api/wire-format.mdx b/content/docs/api/wire-format.mdx index 55db92e101..e004be0b44 100644 --- a/content/docs/api/wire-format.mdx +++ b/content/docs/api/wire-format.mdx @@ -380,6 +380,28 @@ Returned when an `If-Match` / `expectedVersion` token no longer matches the stor } ``` +### Datasource Unavailable — `503 Service Unavailable` + +Returned when the object's declared `datasource` has no live driver: the host's +connect policy refused it, or it failed to connect at startup and the server was +started with `OS_ALLOW_DRIVER_CONNECT_FAILURE`. Nothing about the request is +wrong, and the state may clear — so this is a `503`, not a `400` or a `500`. + +`reason` is the class (`blocked` | `failed`). The underlying cause is +deliberately **not** included — it routinely contains hosts, ports and DSNs; it +stays in the server logs and **Setup → Datasources**. See +[External Datasources](/docs/data-modeling/external-datasources#what-a-query-against-an-unusable-datasource-says). + +```json +{ + "error": "[ObjectQL] Datasource 'analytics' configured for object 'visit' is declared but not connected: the host's datasource connect policy refused it. See the startup logs or Setup → Datasources for the cause.", + "code": "ERR_DATASOURCE_UNAVAILABLE", + "datasource": "analytics", + "reason": "blocked", + "object": "visit" +} +``` + **Error Codes:** See the [Error Catalog](/docs/api/error-catalog) for the complete list of error codes and their meanings. diff --git a/content/docs/data-modeling/external-datasources.mdx b/content/docs/data-modeling/external-datasources.mdx index e05f628e39..d1de069528 100644 --- a/content/docs/data-modeling/external-datasources.mdx +++ b/content/docs/data-modeling/external-datasources.mdx @@ -149,11 +149,66 @@ one failed start reports *all* the misconfigured ones. [engine-level driver-connect guard](/docs/data-modeling/drivers#startup-a-driver-that-cannot-connect-aborts-the-boot) — in an explicitly degraded state announced by a `DEGRADED BOOT` banner. The datasource stays unconnected for the process lifetime: nothing re-runs the -connect, so queries against its objects keep failing with -`Datasource 'x' is not registered` even after the database comes back. Do not set -it in production. +connect, so queries against its objects keep failing until the server is +restarted. Do not set it in production. +### Seeing the state of a datasource + +Every connect attempt's verdict is retained, so a datasource that is down no +longer has to be diagnosed by restarting the server and re-reading boot logs +([#3827](https://github.com/objectstack-ai/objectstack/issues/3827)). + +**Setup → Datasources** and `GET /api/v1/datasources` report a `status` per +datasource: + +| `status` | Meaning | +|:---|:---| +| `ok` | A live driver is registered and routable. | +| `error` | A connect was attempted and failed. `statusReason` carries the cause. | +| `blocked` | The host's connect policy refused it — a decision, not a fault. It will not clear on its own. | +| `unvalidated` | No connect attempted: a `managed` datasource left metadata-only by the gate above, or a runtime row nobody has tested. | + +`statusReason` is **operator-facing** and may name hosts, ports, or internal +plans — this surface is already admin-gated. + +Note that `checkDriversHealth()` (and therefore `/ready`) cannot see these: a +datasource that never connected was never registered as a driver, so it is +*absent* from the health probe rather than reported unhealthy. Readiness is +deliberately **not** gated on them — an optional datasource being down must not +pull an otherwise-working replica out of the load balancer. + +### What a query against an unusable datasource says + +An object bound to a datasource with no live driver fails with +`ERR_DATASOURCE_UNAVAILABLE` (HTTP **503**), and the message says which situation +it is ([#3828](https://github.com/objectstack-ai/objectstack/issues/3828)): +refused by policy, or a connect that failed under +`OS_ALLOW_DRIVER_CONNECT_FAILURE`. A datasource name that was never declared +still gets the original `is not registered` — that one really is an authoring +bug, and there is nothing to add. + +The error **never carries the underlying cause**: connect errors routinely +contain hosts, ports and DSNs, and a policy's `reason` is written for operators. +Both stay in the logs and the admin list. A host that wants to tell tenants +something specific sets `publicReason` on its connect decision — opt-in, and the +only string that reaches an end user: + +```typescript +const policy: DatasourceConnectPolicy = { + canConnect: (ds) => + allowedFor(tenant, ds) + ? { allow: true } + : { + allow: false, + // operator-facing: logs + Setup → Datasources only + reason: `egress allow-list miss for ${ds.name} (${tenant.id}, plan=${tenant.plan})`, + // tenant-facing: appended to the query-time error + publicReason: 'External datasources require the Scale plan.', + }, +}; +``` + ## 4. Credentials Never inline a password. Put a reference in `external.credentialsRef` and store the diff --git a/docs/adr/0062-external-datasource-runtime.md b/docs/adr/0062-external-datasource-runtime.md index 282073a7b4..38074ea2e4 100644 --- a/docs/adr/0062-external-datasource-runtime.md +++ b/docs/adr/0062-external-datasource-runtime.md @@ -1,6 +1,6 @@ # ADR-0062: External Datasource Runtime — connection lifecycle, credentials, visibility & query completeness -**Status**: Accepted (2026-06-22) — D1–D8 implemented (`service-datasource` connection service + opt-in-safe gate + fail-closed connect policy; native-SQL declines external per D6; D7 lint in `validate-expressions.ts`). +**Status**: Accepted (2026-06-22) — D2–D8 implemented (`service-datasource` connection service + opt-in-safe gate + fail-closed connect policy; native-SQL declines external per D6; D7 lint in `validate-expressions.ts`). **D1 partially implemented**: declared datasources auto-connect through the one service, but the `default` driver still has its own connect + failure path — see the status correction under D1 (#3826). **Supersedes the runtime portions of**: ADR-0015 §18 addendum (kept as the historical record). ADR-0015 remains the canonical spec/binding decision; this ADR is the canonical *runtime* decision. @@ -53,6 +53,21 @@ R2/R3/R8 are the same change seen from three angles — you cannot "auto-connect Introduce a single service that, given a datasource definition, builds a driver via the **injected** driver factory (the same `createDefaultDatasourceDriverFactory` used by the runtime-admin path), connects it, and registers it into the ObjectQL engine under the datasource name. `app-plugin` calls it for every declared datasource (in addition to today's `registerInMemory` for visibility); `standalone-stack`'s single-`default`-driver bootstrap is refactored to go through the same service so there is exactly one "definition → live driver" code path. `engine.registerDriver` remains the sink. The `onEnable` bridge becomes unnecessary for the common case (kept as an escape hatch — D8). +> **Status correction (#3826) — the `default` refactor is NOT done; the "exactly one code path" claim is half-true.** The *construction* half converged: `standalone-stack` builds the `default` driver through `createDefaultDatasourceDriverFactory` (`standalone-stack.ts`, "the SAME `create({driver,config})` used for declared/runtime datasources"). The *connect + failure-verdict* half did not, and this ADR's header has been claiming D1 as implemented while two independent implementations coexist: +> +> | | `default` | declared datasource | +> |:---|:---|:---| +> | build | shared factory ✅ | shared factory ✅ | +> | register | `DriverPlugin` → a `driver.*` kernel service, discovered in `ObjectQLPlugin.start()` | `engine.registerDriver()` inside `connect()` | +> | connect | `ObjectQLEngine.init()` | `DatasourceConnectionService.connect()` | +> | failure verdict | `DriverConnectError`, aggregate (#3741) | `handleFailure()` (#3758) | +> | pool teardown | kernel shutdown via `DriverPlugin` | `DatasourceConnectionService.disconnect()` | +> | connect policy | not consulted | `DatasourceConnectPolicy` | +> +> **What actually blocks the merge is an input-shape mismatch, not ordering.** The kernel's init-all-then-start-all means the connection service *does* exist by `ObjectQLPlugin.start()`, so timing is available. The obstacles are: (1) `DatasourceConnectionService.connect()` takes a datasource *definition* and **builds** the driver, while `default` arrives as an already-constructed driver instance published as a `driver.*` kernel service — there is no "adopt this driver" entry point; and (2) routing `default` through the service would make `ObjectQLPlugin`'s boot depend on an **optional service from a higher layer** (`service-datasource`), inverting the layering for the one driver every app needs. Closing this means either adding an `adoptDriver()` seam to the connection service, or making the standalone `default` a real declared datasource definition — a design decision, not a mechanical move, and still the riskiest single step per §Risk. +> +> Until then the divergence is guarded rather than assumed: `packages/runtime/src/degraded-boot-parity.test.ts` pins both paths to the same operator-visible contract (fail-fast by default, identical `OS_ALLOW_DRIVER_CONNECT_FAILURE` parsing, `DEGRADED BOOT` on stderr), so a change to one that forgets the other fails CI instead of shipping. #3741 → #3758 was exactly that miss, and it cost three months and a second bug report. + ### D2 — Connect is opt-in-safe: existing managed apps are byte-for-byte unchanged Auto-connect must not change apps that today declare datasources that are *decorative* or routed via `datasourceMapping` (e.g. `examples/app-crm`'s `crm_primary`/`crm_analytics`). Gate auto-connect so a declared datasource is only connected when it is meaningfully addressed: **(a)** it is `external` (`schemaMode !== 'managed'`), or **(b)** an object/`datasourceMapping` actually routes to it, or **(c)** it sets an explicit `autoConnect: true`. A managed datasource that nothing routes to stays metadata-only (today's behavior). The `default` datasource keeps its current dedicated bootstrap. This is the load-bearing backward-compat decision. diff --git a/packages/objectql/src/core.ts b/packages/objectql/src/core.ts index c170652f29..81a447169f 100644 --- a/packages/objectql/src/core.ts +++ b/packages/objectql/src/core.ts @@ -42,8 +42,13 @@ export type { ObjectQLHostContext, HookHandler, HookEntry, OperationContext, Eng // Boot guard: thrown by `ObjectQL.init()` when a registered driver's connect() // fails (framework#3741). Embedders that boot the engine themselves can catch // it to render their own "database unreachable" message. -export { DriverConnectError } from './driver-connect-errors.js'; -export type { DriverConnectFailure, DriverHealth } from './driver-connect-errors.js'; +export { DriverConnectError, DatasourceUnavailableError } from './driver-connect-errors.js'; +export type { + DriverConnectFailure, + DriverHealth, + DatasourceUnavailableInfo, + DatasourceUnavailableKind, +} from './driver-connect-errors.js'; // In-memory aggregation fallback export { applyInMemoryAggregation, bucketDateValue } from './in-memory-aggregation.js'; diff --git a/packages/objectql/src/datasource-unavailable.test.ts b/packages/objectql/src/datasource-unavailable.test.ts new file mode 100644 index 0000000000..80751a4540 --- /dev/null +++ b/packages/objectql/src/datasource-unavailable.test.ts @@ -0,0 +1,116 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// framework#3828: `getDriver()` answered four different situations with one +// sentence — `Datasource 'x' is not registered.` A policy denial, a boot +// connect failure the operator waved through, a misspelled name, and an +// `active: false` row all read identically, so the reader goes hunting for a +// typo that isn't there. The connection layer knows which one it is; the engine +// just had no way to be told. + +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from './engine.js'; +import { DatasourceUnavailableError } from './driver-connect-errors.js'; + +/** An engine holding one object bound to a datasource that has no driver. */ +function engineWithBoundObject() { + const engine = new ObjectQL({ + logger: { debug() {}, info() {}, warn() {}, error() {} }, + } as any); + engine.registerObject({ + name: 'visit', + label: 'Visit', + datasource: 'analytics', + fields: { id: { type: 'text' } }, + } as any); + return engine; +} + +/** Query the bound object and hand back whatever it threw. */ +async function queryError(engine: ObjectQL): Promise { + return engine.find('visit').then( + () => { throw new Error('find() resolved but should have thrown'); }, + (e: unknown) => e, + ); +} + +describe('ObjectQL.getDriver — a declared datasource with no driver (framework#3828)', () => { + it('keeps the original message when nothing ever tried to connect the name', async () => { + // The authoring-bug case: a typo, or a datasource nobody declared. There is + // genuinely nothing to add, so the wording is unchanged. + const err = await queryError(engineWithBoundObject()); + expect(err.message).toContain("Datasource 'analytics'"); + expect(err.message).toContain('is not registered'); + expect(err).not.toBeInstanceOf(DatasourceUnavailableError); + }); + + it('says the policy refused it, when the connection layer recorded a denial', async () => { + const engine = engineWithBoundObject(); + engine.markDatasourceUnavailable({ name: 'analytics', kind: 'blocked' }); + + const err = await queryError(engine); + expect(err).toBeInstanceOf(DatasourceUnavailableError); + expect(err.code).toBe('ERR_DATASOURCE_UNAVAILABLE'); + expect(err.datasource).toBe('analytics'); + expect(err.objectName).toBe('visit'); + expect(err.kind).toBe('blocked'); + expect(err.message).toContain('connect policy refused it'); + expect(err.message).not.toContain('is not registered'); + }); + + it('appends the host-authored publicReason when one was opted into', async () => { + const engine = engineWithBoundObject(); + engine.markDatasourceUnavailable({ + name: 'analytics', + kind: 'blocked', + publicDetail: 'External datasources require the Scale plan.', + }); + expect((await queryError(engine)).message).toContain('require the Scale plan'); + }); + + it('says the connect failed and a restart is needed, for the degraded-boot case', async () => { + const engine = engineWithBoundObject(); + engine.markDatasourceUnavailable({ name: 'analytics', kind: 'failed' }); + + const err = await queryError(engine); + expect(err.kind).toBe('failed'); + expect(err.message).toContain('OS_ALLOW_DRIVER_CONNECT_FAILURE'); + // The durable fact: the client may reconnect, but nothing re-runs the + // connect, so waiting it out does not help (#3759). + expect(err.message).toContain('restarted'); + }); + + it('never carries the underlying cause — that is what leaks hosts and DSNs', async () => { + const engine = engineWithBoundObject(); + // The connection layer deliberately hands over only the class, so there is + // no path for a cause to reach here even if a caller wanted one. + engine.markDatasourceUnavailable({ name: 'analytics', kind: 'failed' }); + const err = await queryError(engine); + expect(err.message).not.toMatch(/ECONNREFUSED|\d+\.\d+\.\d+\.\d+|postgres:\/\//); + expect(err.message).toContain('startup logs'); + }); + + it('clears the explanation once the datasource connects', async () => { + const engine = engineWithBoundObject(); + engine.markDatasourceUnavailable({ name: 'analytics', kind: 'failed' }); + expect(engine.listUnavailableDatasources()).toEqual([{ name: 'analytics', kind: 'failed' }]); + + engine.clearDatasourceUnavailable('analytics'); + expect(engine.listUnavailableDatasources()).toEqual([]); + // …and the message falls back to the undeclared-name wording. + expect((await queryError(engine)).message).toContain('is not registered'); + }); + + it('lists unavailable datasources — which checkDriversHealth structurally cannot', async () => { + const engine = engineWithBoundObject(); + engine.markDatasourceUnavailable({ name: 'analytics', kind: 'failed' }); + engine.markDatasourceUnavailable({ name: 'billing', kind: 'blocked', publicDetail: 'nope' }); + + // A datasource that never connected was never registered as a driver, so + // the health probe cannot report it at all (framework#3827). + expect(await engine.checkDriversHealth()).toEqual([]); + expect(engine.listUnavailableDatasources()).toEqual([ + { name: 'analytics', kind: 'failed' }, + { name: 'billing', kind: 'blocked', publicDetail: 'nope' }, + ]); + }); +}); diff --git a/packages/objectql/src/driver-connect-errors.ts b/packages/objectql/src/driver-connect-errors.ts index 85407a1362..c944243ea5 100644 --- a/packages/objectql/src/driver-connect-errors.ts +++ b/packages/objectql/src/driver-connect-errors.ts @@ -84,6 +84,67 @@ export class DriverConnectError extends Error { } } +/** Why a declared datasource has no live driver (framework#3828). */ +export type DatasourceUnavailableKind = 'blocked' | 'failed'; + +/** What the connection layer recorded about a datasource it could not connect. */ +export interface DatasourceUnavailableInfo { + kind: DatasourceUnavailableKind; + /** + * Tenant-safe detail, opt-in by the host. The operator-facing reason is + * deliberately NOT carried here — see `DatasourceConnectDecision.publicReason`. + */ + publicDetail?: string; +} + +/** + * Thrown by `getDriver()` when an object's `datasource` was **declared** but has + * no live driver, and the connection layer knows why (framework#3828). + * + * Before this, all four of these produced the same sentence — `Datasource 'x' + * is not registered.`: + * + * 1. the host's connect policy refused it (plan / egress isolation), + * 2. it failed to connect at boot and `OS_ALLOW_DRIVER_CONNECT_FAILURE` let + * the server start anyway, + * 3. the app misspelled the datasource name, and + * 4. it is declared `active: false`. + * + * (3) is an authoring bug; (1) and (2) are states of the deployment. Answering + * all of them identically sends the reader hunting for a typo that isn't there. + * Cases the connection layer never recorded keep the original message — there is + * genuinely nothing more to say about a name nobody declared. + * + * **The message never carries the underlying cause.** A connect failure's text + * routinely contains the host, port, or DSN, and a policy's `reason` is written + * for operators; neither is safe to hand to whoever is browsing a record. The + * cause stays in the startup logs and the datasource-admin list, and this error + * says which of those to read. A host that wants to tell tenants something + * specific sets `publicReason` on its connect decision. + */ +export class DatasourceUnavailableError extends Error { + readonly code = 'ERR_DATASOURCE_UNAVAILABLE' as const; + + constructor( + public readonly datasource: string, + public readonly objectName: string, + public readonly kind: DatasourceUnavailableKind, + publicDetail?: string, + ) { + const head = + `[ObjectQL] Datasource '${datasource}' configured for object '${objectName}' is declared but not connected`; + const why = + kind === 'blocked' + ? `: the host's datasource connect policy refused it.` + : `: it failed to connect at startup and the server was started with ` + + `OS_ALLOW_DRIVER_CONNECT_FAILURE. Nothing re-runs the connect — the server must be ` + + `restarted once the datasource is reachable.`; + const detail = publicDetail ? ` ${publicDetail}` : ''; + super(`${head}${why}${detail} See the startup logs or Setup → Datasources for the cause.`); + this.name = 'DatasourceUnavailableError'; + } +} + /** * The degraded-boot banner now lives in `@objectstack/types` because * `DatasourceConnectionService` owes the operator the same banner for the same diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 403da15d28..9de3d887fb 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -30,7 +30,15 @@ import { resolveFilterTokens, } from '@objectstack/core'; import { SummaryRecomputeError, type SummaryRecomputeFailure } from './summary-errors.js'; -import { DriverConnectError, emitDegradedBootBanner, type DriverConnectFailure, type DriverHealth } from './driver-connect-errors.js'; +import { + DriverConnectError, + DatasourceUnavailableError, + emitDegradedBootBanner, + type DriverConnectFailure, + type DriverHealth, + type DatasourceUnavailableInfo, + type DatasourceUnavailableKind, +} from './driver-connect-errors.js'; import { resolveAllowDriverConnectFailure } from '@objectstack/types'; /** @@ -355,6 +363,12 @@ export class ObjectQL implements IDataEngine { // registerDatasourceDef. Absent entry ⇒ treated as managed (default DB). private datasourceDefs = new Map(); + // Declared-but-unusable datasources, keyed by name (framework#3828). Written + // by the datasource connection layer via markDatasourceUnavailable; read only + // by getDriver, to explain a missing driver instead of blaming a typo. Empty + // is the normal state — an entry here means a connect was refused or failed. + private unavailableDatasources = new Map(); + // Per-object hooks with priority support private hooks: Map = new Map([ ['beforeFind', []], ['afterFind', []], @@ -1481,6 +1495,41 @@ export class ObjectQL implements IDataEngine { this.datasourceDefs.set(def.name, { schemaMode: def.schemaMode, external: def.external }); } + /** + * Record that a **declared** datasource has no live driver, and why + * (framework#3828). Called by `DatasourceConnectionService` when a connect is + * refused by the host policy or fails while the operator has opted into a + * degraded boot. + * + * The engine only needs this to answer a query well: without it + * {@link getDriver} cannot tell a refused datasource from a misspelled one and + * says `is not registered` to both. Deliberately carries no operator-facing + * cause — see {@link DatasourceUnavailableError}. + */ + markDatasourceUnavailable(info: { name: string; kind: DatasourceUnavailableKind; publicDetail?: string }): void { + if (!info?.name) return; + this.unavailableDatasources.set(info.name, { + kind: info.kind, + ...(info.publicDetail ? { publicDetail: info.publicDetail } : {}), + }); + } + + /** Drop a {@link markDatasourceUnavailable} record (successful (re)connect / pool removal). */ + clearDatasourceUnavailable(name: string): void { + this.unavailableDatasources.delete(name); + } + + /** + * Datasources that were declared but are NOT usable, with the reason class. + * + * Distinct from {@link checkDriversHealth}, which answers "can a REGISTERED + * driver serve a query right now" and therefore cannot see these at all — a + * datasource that never connected was never registered (framework#3827). + */ + listUnavailableDatasources(): Array<{ name: string } & DatasourceUnavailableInfo> { + return Array.from(this.unavailableDatasources, ([name, info]) => ({ name, ...info })); + } + /** * Write gate — Gate 3 of ADR-0015 §5.3. * @@ -1761,6 +1810,21 @@ export class ObjectQL implements IDataEngine { if (this.drivers.has(object.datasource)) { return this.drivers.get(object.datasource)!; } + // The datasource layer may have recorded WHY this one has no driver — + // refused by the host policy, or failed to connect under + // OS_ALLOW_DRIVER_CONNECT_FAILURE (framework#3828). Saying so beats + // sending the reader hunting for a typo that isn't there. + const unavailable = this.unavailableDatasources.get(object.datasource); + if (unavailable) { + throw new DatasourceUnavailableError( + object.datasource, + objectName, + unavailable.kind, + unavailable.publicDetail, + ); + } + // No record: nothing ever tried to connect this name, so it is genuinely + // undeclared (or misspelled). Unchanged message — there is nothing to add. throw new Error(`[ObjectQL] Datasource '${object.datasource}' configured for object '${objectName}' is not registered.`); } diff --git a/packages/objectql/src/index.ts b/packages/objectql/src/index.ts index 2d7351c383..0ce2e983e0 100644 --- a/packages/objectql/src/index.ts +++ b/packages/objectql/src/index.ts @@ -38,8 +38,13 @@ export type { SummaryRecomputeFailure } from './summary-errors.js'; // Boot guard: thrown by `ObjectQL.init()` when a registered driver's connect() // fails (framework#3741). Hosts that boot the engine themselves can catch it to // render their own "database unreachable" message. -export { DriverConnectError } from './driver-connect-errors.js'; -export type { DriverConnectFailure, DriverHealth } from './driver-connect-errors.js'; +export { DriverConnectError, DatasourceUnavailableError } from './driver-connect-errors.js'; +export type { + DriverConnectFailure, + DriverHealth, + DatasourceUnavailableInfo, + DatasourceUnavailableKind, +} from './driver-connect-errors.js'; export type { InsertManyRowOutcome } from './engine.js'; // Export in-memory aggregation fallback (used by engine.aggregate when the diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 10838cd4f3..8e5da1d5c6 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -97,6 +97,26 @@ export function mapDataError(error: any, object?: string): { status: number; bod }, }; } + // A declared datasource that is refused by the host policy, or failed to + // connect under OS_ALLOW_DRIVER_CONNECT_FAILURE → 503 (framework#3828). + // Handled before the catch-alls because nothing about the REQUEST is wrong: + // the deployment cannot serve this object right now. 503 (not 500) is the + // honest answer — it is a dependency outage or a policy state, it may clear, + // and it tells a caller/proxy that retrying elsewhere or later is sensible. + // The message is already sanitised at the throw site (no DSN, host, or + // operator-facing policy reason), so it is safe to pass through verbatim. + if (error?.code === 'ERR_DATASOURCE_UNAVAILABLE') { + return { + status: 503, + body: { + error: error?.message ?? 'The datasource for this object is not available', + code: 'ERR_DATASOURCE_UNAVAILABLE', + ...(error?.datasource ? { datasource: error.datasource } : {}), + ...(error?.kind ? { reason: error.kind } : {}), + ...(object ? { object } : {}), + }, + }; + } // Validation failures → 400 with per-field envelope. Handled FIRST // because the validator throws a typed error before any SQL ever // runs, and we want callers to differentiate "your payload was diff --git a/packages/rest/src/rest.test.ts b/packages/rest/src/rest.test.ts index 1ff62afea3..7374d2a074 100644 --- a/packages/rest/src/rest.test.ts +++ b/packages/rest/src/rest.test.ts @@ -2080,6 +2080,41 @@ describe('mapDataError — schema/constraint envelopes', () => { // errors through mapDataError (not sendError's `.status` passthrough), so // the code needs its own branch — without it the 403 degraded to the // catch-all 400 (caught live in the runtime smoke test). + // #3828: an object whose declared datasource is refused by the host policy, + // or failed to connect under OS_ALLOW_DRIVER_CONNECT_FAILURE. Nothing about + // the REQUEST is wrong, so 4xx lies; and it is a dependency/policy state that + // may clear, so 503 is more honest (and more actionable to a proxy) than 500. + it('maps ERR_DATASOURCE_UNAVAILABLE → 503 with the datasource and reason class', () => { + const r = mapDataError( + Object.assign( + new Error( + "[ObjectQL] Datasource 'analytics' configured for object 'visit' is declared but not connected: " + + "the host's datasource connect policy refused it. External datasources require the Scale plan. " + + 'See the startup logs or Setup → Datasources for the cause.', + ), + { code: 'ERR_DATASOURCE_UNAVAILABLE', datasource: 'analytics', kind: 'blocked' }, + ), + 'visit', + ); + expect(r.status).toBe(503); + expect(r.body.code).toBe('ERR_DATASOURCE_UNAVAILABLE'); + expect(r.body.datasource).toBe('analytics'); + expect(r.body.reason).toBe('blocked'); + expect(r.body.object).toBe('visit'); + // The message is sanitised at the throw site, so it passes through as-is. + expect(String(r.body.error)).toContain('require the Scale plan'); + }); + + it('does not downgrade ERR_DATASOURCE_UNAVAILABLE to the generic 400 catch-all', () => { + const r = mapDataError( + Object.assign(new Error("Datasource 'x' ... is declared but not connected"), { + code: 'ERR_DATASOURCE_UNAVAILABLE', + kind: 'failed', + }), + ); + expect(r.status).toBe(503); + }); + it('maps FEEDS_DISABLED → 403 with the gated target object', () => { const r = mapDataError( Object.assign(new Error("Comments are disabled for object 'gate_probe' (enable.feeds: false)"), { diff --git a/packages/runtime/src/datasource-autoconnect.test.ts b/packages/runtime/src/datasource-autoconnect.test.ts index bd857dd255..99505205f8 100644 --- a/packages/runtime/src/datasource-autoconnect.test.ts +++ b/packages/runtime/src/datasource-autoconnect.test.ts @@ -272,4 +272,60 @@ describe('ADR-0062 connect policy seam', () => { try { await (kernel as any)?.stop?.(); } catch { /* noop */ } } }, BOOT_TIMEOUT); + + // framework#3828 — the denial is deliberate and stays non-fatal, but the + // tenant used to be told `Datasource 'autoconn_ext' is not registered`, which + // reads like "you misconfigured your app" rather than "your plan blocks this". + it('a query against a denied datasource explains WHY, without leaking the operator reason', async () => { + const denyExternal: DatasourceConnectPolicy = { + canConnect: (ds) => + ds.schemaMode === 'external' + ? { + allow: false, + reason: 'egress allow-list miss for warehouse.internal:5432 (org_42, plan=free)', + publicReason: 'External datasources require the Scale plan.', + } + : { allow: true }, + }; + const kernel = await boot({ connectPolicy: denyExternal }); + try { + const engine = kernel.getService<{ find(o: string): Promise }>('data'); + const err: any = await engine.find('ext_note').then( + () => { throw new Error('find() resolved but should have thrown'); }, + (e: unknown) => e, + ); + expect(err.code).toBe('ERR_DATASOURCE_UNAVAILABLE'); + expect(err.message).toContain('connect policy refused it'); + expect(err.message).toContain('require the Scale plan'); + // The operator-facing reason must never cross into a tenant-visible error. + expect(err.message).not.toContain('warehouse.internal'); + expect(err.message).not.toContain('org_42'); + } finally { + try { await (kernel as any)?.stop?.(); } catch { /* noop */ } + } + }, BOOT_TIMEOUT); + + // framework#3827 — the admin list is the one place an operator can see this + // without redeploying and re-reading boot logs. + it('surfaces the denial in the datasource-admin list, with the operator reason', async () => { + const denyExternal: DatasourceConnectPolicy = { + canConnect: (ds) => + ds.schemaMode === 'external' + ? { allow: false, reason: 'egress allow-list miss for warehouse.internal:5432' } + : { allow: true }, + }; + const kernel = await boot({ connectPolicy: denyExternal }); + try { + const admin = kernel.getService<{ listDatasources(): Promise }>('datasource-admin'); + const list = await admin.listDatasources(); + const ext = list.find((d) => d.name === 'autoconn_ext')!; + expect(ext.status).toBe('blocked'); + // Admin-gated surface: the raw reason is the useful answer here. + expect(ext.statusReason).toContain('warehouse.internal:5432'); + // A datasource nothing tried to connect stays honestly unknown. + expect(list.find((d) => d.name === 'decorative')!.status).toBe('unvalidated'); + } finally { + try { await (kernel as any)?.stop?.(); } catch { /* noop */ } + } + }, BOOT_TIMEOUT); }); diff --git a/packages/runtime/src/degraded-boot-parity.test.ts b/packages/runtime/src/degraded-boot-parity.test.ts new file mode 100644 index 0000000000..914cda4d36 --- /dev/null +++ b/packages/runtime/src/degraded-boot-parity.test.ts @@ -0,0 +1,147 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// framework#3826 — DRIFT GUARD for the two "a datasource would not connect" +// implementations. +// +// ADR-0062 D1 asks for exactly one "definition → live driver" path. The +// *construction* half converged (both build through the shared datasource driver +// factory), but the *connect + failure verdict* half did not: +// +// - the `default` driver is registered as a `driver.*` kernel service and +// connected by `ObjectQLEngine.init()` → DriverConnectError (#3741) +// - declared datasources are connected by +// `DatasourceConnectionService.connect()` → handleFailure (#3758) +// +// They agree today only because two separate pieces of code were each written +// correctly — and the last time they disagreed, the gap survived three months +// and a second bug report (#3741 fixed one layer, #3758 was the other). Until +// the paths are actually merged, this test is what notices a divergence: +// it pins the operator-visible contract both of them owe. +// +// This lives in `runtime` because it is the only package that depends on both. + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { DatasourceConnectionService } from '@objectstack/service-datasource'; + +const ENV = 'OS_ALLOW_DRIVER_CONNECT_FAILURE'; + +/** Minimal driver stub whose `connect()` rejects — the engine-side failure. */ +function failingDriver(name: string) { + return { + name, + version: '0.0.0', + supports: {}, + connected: false, + async connect() { throw new Error('connect ECONNREFUSED 127.0.0.1:5432'); }, + async disconnect() {}, + async checkHealth() { return false; }, + async execute() { return null; }, + async find() { return []; }, + findStream() { throw new Error('ni'); }, + async findOne() { return null; }, + async create(_o: string, d: Record) { return { id: 'r_1', ...d }; }, + async update(_o: string, id: string, d: Record) { return { ...d, id }; }, + async delete() { return true; }, + async count() { return 0; }, + async bulkCreate() { return []; }, + async bulkUpdate() { return []; }, + async bulkDelete() {}, + async beginTransaction() { return { __trx: true, commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + } as any; +} + +/** The engine path: one boot-registered driver that cannot connect. */ +async function bootEngine(): Promise { + const engine = new ObjectQL({ + logger: { debug() {}, info() {}, warn() {}, error() {} }, + } as any); + engine.registerDriver(failingDriver('sql'), true); + return engine.init().then(() => undefined, (e: Error) => e); +} + +/** + * The datasource path: a declared datasource with an object bound to it that + * cannot connect — the #3758 shape, i.e. the one with no fallback, so the two + * paths are being asked the same question ("a datasource I need is down"). + */ +async function bootDatasource(): Promise { + const drivers = new Map(); + const service = new DatasourceConnectionService({ + factory: () => ({ + supports: () => true, + create: async () => ({ + driver: { name: 'd' }, + connect: async () => { throw new Error('connect ECONNREFUSED 127.0.0.1:5432'); }, + }), + }) as any, + engine: () => ({ + registerDriver: (d: any) => drivers.set(d.name, d), + getDriverByName: (n: string) => drivers.get(n), + }), + logger: { warn() {} }, + }); + return service + .connect( + { name: 'analytics', driver: 'sqlite', schemaMode: 'managed', config: {} }, + { objects: ['visit'], context: { trigger: 'declared-auto' } }, + ) + .then(() => undefined, (e: Error) => e); +} + +const PATHS: Array<{ label: string; boot: () => Promise }> = [ + { label: 'ObjectQLEngine.init()', boot: bootEngine }, + { label: 'DatasourceConnectionService.connect()', boot: bootDatasource }, +]; + +describe('degraded-boot parity between the two connect paths (framework#3826)', () => { + let saved: string | undefined; + beforeEach(() => { saved = process.env[ENV]; delete process.env[ENV]; }); + afterEach(() => { + if (saved === undefined) delete process.env[ENV]; + else process.env[ENV] = saved; + }); + + for (const { label, boot } of PATHS) { + describe(label, () => { + it('refuses the boot by default', async () => { + expect(await boot()).toBeInstanceOf(Error); + }); + + // The flag is the operator's single lever over both paths. If either one + // ever re-reads the env inline instead of going through + // `resolveAllowDriverConnectFailure()`, these two cases diverge — which is + // precisely the drift this file exists to catch. + for (const truthy of ['1', 'true', 'on', 'yes', 'YES', ' True ']) { + it(`boots degraded when ${ENV}=${JSON.stringify(truthy)}`, async () => { + process.env[ENV] = truthy; + expect(await boot()).toBeUndefined(); + }); + } + + for (const falsy of ['0', 'false', 'off', 'no', '']) { + it(`still refuses the boot when ${ENV}=${JSON.stringify(falsy)}`, async () => { + process.env[ENV] = falsy; + expect(await boot()).toBeInstanceOf(Error); + }); + } + + it('announces the degraded state on stderr, which `os serve` boot-quiet cannot swallow', async () => { + process.env[ENV] = '1'; + const written: string[] = []; + const realWrite = process.stderr.write; + (process.stderr as { write: unknown }).write = (chunk: any) => { + written.push(String(chunk)); + return true; + }; + try { + await boot(); + } finally { + (process.stderr as { write: unknown }).write = realWrite; + } + expect(written.join('')).toContain('DEGRADED BOOT'); + }); + }); + } +}); diff --git a/packages/services/service-datasource/src/__tests__/datasource-admin-service.test.ts b/packages/services/service-datasource/src/__tests__/datasource-admin-service.test.ts index 29e8723d00..454f3209eb 100644 --- a/packages/services/service-datasource/src/__tests__/datasource-admin-service.test.ts +++ b/packages/services/service-datasource/src/__tests__/datasource-admin-service.test.ts @@ -316,3 +316,86 @@ describe('getDatasource', () => { expect(await service.getDatasource('missing')).toBeUndefined(); }); }); + +// framework#3827 — `status` was the literal 'unvalidated' for every row, so a +// datasource that died at boot looked exactly like a healthy-but-untested one. +describe('listDatasources — status reflects the last connect verdict (framework#3827)', () => { + function harnessWithStates( + states: ReadonlyArray<{ + name: string; + availability: 'available' | 'blocked' | 'failed' | 'unattempted'; + reason?: string; + }>, + ) { + const config: DatasourceAdminServiceConfig = { + probe: async () => ({ ok: true }), + listDatasourceRecords: async () => [ + { name: 'live', driver: 'sqlite', origin: 'code' }, + { name: 'dead', driver: 'postgres', origin: 'code' }, + { name: 'denied', driver: 'postgres', schemaMode: 'external', origin: 'code' }, + { name: 'decorative', driver: 'sqlite', origin: 'code' }, + ], + getDatasourceRecord: async () => undefined, + putDatasourceRecord: async () => {}, + deleteDatasourceRecord: async () => {}, + writeSecret: async () => 'ref', + countBoundObjects: async () => 0, + connectionStates: () => states, + }; + return new DatasourceAdminService(config); + } + + it('maps each availability class onto a distinguishable status', async () => { + const service = harnessWithStates([ + { name: 'live', availability: 'available' }, + { name: 'dead', availability: 'failed', reason: 'connect ECONNREFUSED 10.0.0.4:5432' }, + { name: 'denied', availability: 'blocked', reason: 'plan=free; egress allow-list miss' }, + // 'decorative' has no state at all — the D2 gate never attempted it. + ]); + const byName = Object.fromEntries((await service.listDatasources()).map((d) => [d.name, d])); + + expect(byName.live!.status).toBe('ok'); + expect(byName.dead!.status).toBe('error'); + expect(byName.denied!.status).toBe('blocked'); + expect(byName.decorative!.status).toBe('unvalidated'); + }); + + it('carries the operator-facing reason for error/blocked only', async () => { + const service = harnessWithStates([ + { name: 'live', availability: 'available', reason: 'should not be surfaced' }, + { name: 'dead', availability: 'failed', reason: 'connect ECONNREFUSED 10.0.0.4:5432' }, + { name: 'denied', availability: 'blocked', reason: 'plan=free; egress allow-list miss' }, + ]); + const byName = Object.fromEntries((await service.listDatasources()).map((d) => [d.name, d])); + + // This surface is admin-gated, so the raw cause is the useful answer here — + // it is the END-USER error that must stay sanitised (#3828). + expect(byName.dead!.statusReason).toContain('ECONNREFUSED'); + expect(byName.denied!.statusReason).toContain('egress allow-list'); + // A healthy datasource has nothing to explain. + expect(byName.live!.statusReason).toBeUndefined(); + expect(byName.decorative!.statusReason).toBeUndefined(); + }); + + it('an `unattempted` verdict reads the same as no verdict — nobody tried, nothing is known', async () => { + const service = harnessWithStates([{ name: 'live', availability: 'unattempted', reason: 'no driver factory' }]); + const live = (await service.listDatasources()).find((d) => d.name === 'live')!; + expect(live.status).toBe('unvalidated'); + expect(live.statusReason).toBeUndefined(); + }); + + it('falls back to `unvalidated` throughout when no connection service is wired', async () => { + const config: DatasourceAdminServiceConfig = { + probe: async () => ({ ok: true }), + listDatasourceRecords: async () => [{ name: 'x', driver: 'sqlite', origin: 'code' }], + getDatasourceRecord: async () => undefined, + putDatasourceRecord: async () => {}, + deleteDatasourceRecord: async () => {}, + writeSecret: async () => 'ref', + countBoundObjects: async () => 0, + // no `connectionStates` + }; + const list = await new DatasourceAdminService(config).listDatasources(); + expect(list[0]!.status).toBe('unvalidated'); + }); +}); diff --git a/packages/services/service-datasource/src/__tests__/datasource-connection-service.test.ts b/packages/services/service-datasource/src/__tests__/datasource-connection-service.test.ts index ffa1b02838..d7f8d484e7 100644 --- a/packages/services/service-datasource/src/__tests__/datasource-connection-service.test.ts +++ b/packages/services/service-datasource/src/__tests__/datasource-connection-service.test.ts @@ -10,15 +10,28 @@ import { import type { IDatasourceDriverFactory } from '../contracts/datasource-driver-factory.js'; import type { DatasourceConnectPolicy } from '../contracts/connect-policy.js'; -/** A fake engine recording driver registration + schema syncs. */ +/** One `markDatasourceUnavailable` call, as the engine would receive it. */ +type UnavailableCall = { name: string; kind: 'blocked' | 'failed'; publicDetail?: string }; + +/** A fake engine recording driver registration, schema syncs + availability records. */ function fakeEngine() { const drivers = new Map(); const defs: Array<{ name: string; schemaMode?: string }> = []; const synced: string[] = []; - const engine: ConnectionEngineLike & { drivers: typeof drivers; defs: typeof defs; synced: string[] } = { + const unavailable = new Map(); + const cleared: string[] = []; + const engine: ConnectionEngineLike & { + drivers: typeof drivers; + defs: typeof defs; + synced: string[]; + unavailable: typeof unavailable; + cleared: string[]; + } = { drivers, defs, synced, + unavailable, + cleared, registerDriver: (driver: any) => { if (drivers.has(driver.name)) return; // mirror engine's skip-if-present drivers.set(driver.name, driver); @@ -30,6 +43,13 @@ function fakeEngine() { syncObjectSchema: async (name) => { synced.push(name); }, + markDatasourceUnavailable: (info) => { + unavailable.set(info.name, info); + }, + clearDatasourceUnavailable: (name) => { + unavailable.delete(name); + cleared.push(name); + }, }; return engine; } @@ -486,3 +506,150 @@ describe('DatasourceConnectionService.connectDeclared', () => { } }); }); + +// framework#3827 / #3828 — every connect already produced a verdict and every +// caller threw it away, so a datasource that died at boot was invisible for the +// rest of the process and a query against it could only say "is not registered". +describe('retained connection state (framework#3827)', () => { + const analytics: ConnectableDatasource = { + name: 'analytics', + driver: 'sqlite', + schemaMode: 'managed', + config: {}, + }; + + it('records `available` on a successful connect and clears any stale unavailability', async () => { + const { service, engine } = svc(); + await service.connect(externalDs); + expect(service.getConnectionState('warehouse')).toMatchObject({ + name: 'warehouse', + status: 'connected', + availability: 'available', + }); + // A previously-failed datasource that later connects must stop explaining + // itself as broken. + expect(engine!.cleared).toContain('warehouse'); + expect(engine!.unavailable.has('warehouse')).toBe(false); + }); + + it('records `available` for the onEnable escape hatch (already-registered counts as usable)', async () => { + const { service, engine } = svc(); + engine!.drivers.set('warehouse', { name: 'warehouse' }); + await service.connect(externalDs); + expect(service.getConnectionState('warehouse')?.availability).toBe('available'); + }); + + it('records `failed` with the cause, and tells the engine — without leaking the cause to it', async () => { + const { service, engine } = svc({ factory: fakeFactory({ connectThrows: true }) }); + await service.connect(analytics, { context: { trigger: 'runtime-admin' } }); + + const state = service.getConnectionState('analytics'); + expect(state).toMatchObject({ status: 'failed-degraded', availability: 'failed' }); + expect(state!.reason).toContain('connection refused'); // operator-facing, retained + + // The engine gets the CLASS, never the raw cause: its copy is what reaches + // an end user, and a connect error routinely names hosts/ports/DSNs. + expect(engine!.unavailable.get('analytics')).toEqual({ name: 'analytics', kind: 'failed' }); + }); + + it('records `blocked` for a policy denial and passes ONLY the opt-in publicReason on', async () => { + const policy: DatasourceConnectPolicy = { + canConnect: () => ({ + allow: false, + reason: 'tenant plan=free; egress allow-list miss for warehouse.internal:5432', + publicReason: 'External datasources require the Scale plan.', + }), + }; + const { service, engine } = svc({ policy }); + await service.connect(externalDs); + + const state = service.getConnectionState('warehouse'); + expect(state).toMatchObject({ status: 'skipped-policy', availability: 'blocked' }); + // The privileged reason is retained for the admin list… + expect(state!.reason).toContain('warehouse.internal:5432'); + // …but only the tenant-safe string crosses into the engine. + expect(engine!.unavailable.get('warehouse')).toEqual({ + name: 'warehouse', + kind: 'blocked', + publicDetail: 'External datasources require the Scale plan.', + }); + }); + + it('omits publicDetail entirely when the policy did not opt in', async () => { + const policy: DatasourceConnectPolicy = { + canConnect: () => ({ allow: false, reason: 'internal: quota exceeded for org_42' }), + }; + const { service, engine } = svc({ policy }); + await service.connect(externalDs); + expect(engine!.unavailable.get('warehouse')).toEqual({ name: 'warehouse', kind: 'blocked' }); + }); + + it('does not carry a publicReason from one connect into the next', async () => { + let first = true; + const policy: DatasourceConnectPolicy = { + canConnect: () => { + const decision = first + ? { allow: false, reason: 'r1', publicReason: 'Only for the first one.' } + : { allow: false, reason: 'r2' }; + first = false; + return decision; + }, + }; + const { service, engine } = svc({ policy }); + await service.connect({ ...externalDs, name: 'ds_a' }); + await service.connect({ ...externalDs, name: 'ds_b' }); + expect(engine!.unavailable.get('ds_a')?.publicDetail).toBe('Only for the first one.'); + expect(engine!.unavailable.get('ds_b')?.publicDetail).toBeUndefined(); + }); + + it('records the verdict even when the D5 fail-fast throws', async () => { + const ENV = 'OS_ALLOW_DRIVER_CONNECT_FAILURE'; + const saved = process.env[ENV]; + delete process.env[ENV]; + try { + const { service, engine } = svc({ factory: fakeFactory({ connectThrows: true }) }); + await expect( + service.connect(analytics, { objects: ['visit'], context: { trigger: 'declared-auto' } }), + ).rejects.toThrow(/fail-fast/); + // The boot is aborting, but a host that catches the throw must not be left + // with a datasource whose state claims it was never attempted. + expect(service.getConnectionState('analytics')?.availability).toBe('failed'); + expect(engine!.unavailable.get('analytics')?.kind).toBe('failed'); + } finally { + if (saved === undefined) delete process.env[ENV]; + else process.env[ENV] = saved; + } + }); + + it('leaves `unattempted` (not "broken") when there is no factory/engine to try with', async () => { + const service = new DatasourceConnectionService({ + factory: () => undefined, + engine: () => fakeEngine(), + }); + await service.connect(externalDs); + expect(service.getConnectionState('warehouse')).toMatchObject({ + status: 'skipped-no-infra', + availability: 'unattempted', + }); + }); + + it('drops the verdict on disconnect — a removed pool must not keep explaining itself', async () => { + const { service, engine } = svc({ factory: fakeFactory({ connectThrows: true }) }); + await service.connect(analytics, { context: { trigger: 'runtime-admin' } }); + expect(service.getConnectionState('analytics')).toBeDefined(); + + await service.disconnect('analytics'); + expect(service.getConnectionState('analytics')).toBeUndefined(); + expect(engine!.unavailable.has('analytics')).toBe(false); + }); + + it('lists every retained verdict for the admin surface', async () => { + const { service } = svc({ factory: fakeFactory({ connectThrows: true }) }); + await service.connect({ ...analytics, name: 'a' }, { context: { trigger: 'runtime-admin' } }); + await service.connect({ ...analytics, name: 'b' }, { context: { trigger: 'runtime-admin' } }); + expect(service.listConnectionStates().map((s) => [s.name, s.availability])).toEqual([ + ['a', 'failed'], + ['b', 'failed'], + ]); + }); +}); diff --git a/packages/services/service-datasource/src/contracts/connect-policy.ts b/packages/services/service-datasource/src/contracts/connect-policy.ts index cf933e7cfe..cfaf1470ea 100644 --- a/packages/services/service-datasource/src/contracts/connect-policy.ts +++ b/packages/services/service-datasource/src/contracts/connect-policy.ts @@ -34,8 +34,28 @@ export interface DatasourceConnectContext { /** A policy verdict. `allow:false` leaves the datasource unconnected (metadata-only). */ export interface DatasourceConnectDecision { allow: boolean; - /** Human-readable reason, surfaced in logs when a connect is denied. */ + /** + * Human-readable reason for the operator, surfaced in **logs and the + * datasource-admin list** when a connect is denied. + * + * Treat this as PRIVILEGED: it is written for whoever runs the host and may + * name internal plans, quotas, allow-lists or hostnames. It is never included + * in the error an end user sees when they query an object bound to the denied + * datasource — use {@link publicReason} for that (framework#3828). + */ reason?: string; + /** + * Optional tenant-safe explanation, opt-in. When set, it is appended to the + * `ERR_DATASOURCE_UNAVAILABLE` error thrown at query time, so an end user + * hitting an object bound to this datasource is told *why* instead of the + * bare "is not registered" (framework#3828). + * + * Opt-in on purpose: a policy's {@link reason} is written for operators and + * assuming it is safe to echo to tenants would leak host internals the moment + * a host writes a candid one. Say here, explicitly, what a tenant may read — + * e.g. `'External datasources require the Scale plan.'` + */ + publicReason?: string; } /** The minimal datasource shape a policy inspects (never a secret). */ diff --git a/packages/services/service-datasource/src/contracts/datasource-admin-service.ts b/packages/services/service-datasource/src/contracts/datasource-admin-service.ts index 5614d351f8..ad0412abed 100644 --- a/packages/services/service-datasource/src/contracts/datasource-admin-service.ts +++ b/packages/services/service-datasource/src/contracts/datasource-admin-service.ts @@ -73,8 +73,30 @@ export interface DatasourceSummary { schemaMode: 'managed' | 'external' | 'validate-only'; origin: DatasourceOrigin; active: boolean; - /** Validation health: `unvalidated` until the first validate/test runs. */ - status: 'ok' | 'error' | 'unvalidated'; + /** + * Current availability, taken from the last connect attempt (framework#3827): + * + * - `ok` — a live driver is registered and routable. + * - `error` — a connect was attempted and failed (unreachable, bad + * credential, unsupported driver). See {@link statusReason}. + * - `blocked` — the host's connect policy refused it. A decision, not a + * fault; it will not clear on its own. + * - `unvalidated` — no connect attempted. Includes a `managed` datasource + * left metadata-only by the ADR-0062 D2 gate, and a runtime + * row nobody has tested yet. + * + * This was hardcoded to `unvalidated` for every row, which made a dead + * datasource indistinguishable from a healthy-but-untested one — the reason a + * failed boot connect stayed invisible for the rest of the process. + */ + status: 'ok' | 'error' | 'blocked' | 'unvalidated'; + /** + * Operator-facing detail behind `error` / `blocked`. PRIVILEGED: it is the raw + * connect error or the policy's `reason`, so it can name hosts, ports and + * internal plans. This surface is already admin-gated; the end-user + * query-time error deliberately carries none of it (framework#3828). + */ + statusReason?: string; /** Package id that defines a code-origin datasource (omitted for runtime). */ definedIn?: string; /** True when a runtime row is shadowed by a code definition of the same name. */ diff --git a/packages/services/service-datasource/src/datasource-admin-plugin.ts b/packages/services/service-datasource/src/datasource-admin-plugin.ts index 36186bd2b8..14b1151595 100644 --- a/packages/services/service-datasource/src/datasource-admin-plugin.ts +++ b/packages/services/service-datasource/src/datasource-admin-plugin.ts @@ -304,6 +304,12 @@ export class DatasourceAdminServicePlugin implements Plugin { await this.connection?.disconnect(name); }, + // The admin list's `status` reads the connection service's retained + // verdicts (framework#3827). Resolved lazily per call: the service exists + // by the end of this init(), but the verdicts only appear once boot + // auto-connect (AppPlugin.start) and any runtime pool writes have run. + connectionStates: () => this.connection?.listConnectionStates() ?? [], + logger, }; diff --git a/packages/services/service-datasource/src/datasource-admin-service.ts b/packages/services/service-datasource/src/datasource-admin-service.ts index 97ad89c50b..426eaf2ec3 100644 --- a/packages/services/service-datasource/src/datasource-admin-service.ts +++ b/packages/services/service-datasource/src/datasource-admin-service.ts @@ -90,9 +90,38 @@ export interface DatasourceAdminServiceConfig { registerPool?: (record: StoredDatasource) => Promise | void; /** Tear down a runtime datasource's pool on remove. */ unregisterPool?: (name: string) => Promise | void; + /** + * Last connect verdict per datasource, from `DatasourceConnectionService` + * (framework#3827). Absent (a host without the connection service) means the + * list reports `unvalidated` throughout — the pre-#3827 behavior, and honest: + * with nothing attempting connects there is genuinely no verdict to report. + */ + connectionStates?: () => ReadonlyArray<{ + name: string; + availability: 'available' | 'blocked' | 'failed' | 'unattempted'; + reason?: string; + }>; logger?: Logger; } +/** Map a connection verdict onto the admin list's `status` field. */ +function summaryStatus( + availability: 'available' | 'blocked' | 'failed' | 'unattempted' | undefined, +): DatasourceSummary['status'] { + switch (availability) { + case 'available': + return 'ok'; + case 'blocked': + return 'blocked'; + case 'failed': + return 'error'; + // `unattempted` and "no record at all" are the same answer to the only + // question this field asks: nobody has tried, so nothing is known. + default: + return 'unvalidated'; + } +} + export class DatasourceAdminService implements IDatasourceAdminService { constructor(private readonly config: DatasourceAdminServiceConfig) {} @@ -113,10 +142,19 @@ export class DatasourceAdminService implements IDatasourceAdminService { byName.set(rec.name, slot); } + // Last connect verdict per datasource (framework#3827). Without this the + // `status` below was a constant, so a datasource that died at boot looked + // exactly like one nobody had tested. + const states = new Map( + (this.config.connectionStates?.() ?? []).map((s) => [s.name, s]), + ); + const summaries: DatasourceSummary[] = []; for (const [name, slot] of byName) { const effective = slot.code ?? slot.runtime; if (!effective) continue; + const state = states.get(name); + const status = summaryStatus(state?.availability); summaries.push({ name, label: effective.label, @@ -124,7 +162,10 @@ export class DatasourceAdminService implements IDatasourceAdminService { schemaMode: effective.schemaMode ?? 'managed', origin: slot.code ? 'code' : 'runtime', active: effective.active ?? true, - status: 'unvalidated', + status, + ...(status !== 'ok' && status !== 'unvalidated' && state?.reason + ? { statusReason: state.reason } + : {}), ...(slot.code?.definedIn ? { definedIn: slot.code.definedIn } : {}), ...(slot.code && slot.runtime ? { conflictsWithCode: true } : {}), }); @@ -292,6 +333,12 @@ export class DatasourceAdminService implements IDatasourceAdminService { } private toSummary(record: StoredDatasource): DatasourceSummary { + // Returned from create/update, i.e. right after `tryRegisterPool` — so the + // connect verdict for this write is already recorded and worth reporting: + // a "Save" that silently failed to open the pool is exactly the case the + // wizard must not present as success (framework#3827). + const state = this.config.connectionStates?.().find((s) => s.name === record.name); + const status = summaryStatus(state?.availability); return { name: record.name, label: record.label, @@ -299,7 +346,10 @@ export class DatasourceAdminService implements IDatasourceAdminService { schemaMode: record.schemaMode ?? 'managed', origin: record.origin ?? 'runtime', active: record.active ?? true, - status: 'unvalidated', + status, + ...(status !== 'ok' && status !== 'unvalidated' && state?.reason + ? { statusReason: state.reason } + : {}), }; } diff --git a/packages/services/service-datasource/src/datasource-connection-service.ts b/packages/services/service-datasource/src/datasource-connection-service.ts index 3c459e7dd7..17a3a04107 100644 --- a/packages/services/service-datasource/src/datasource-connection-service.ts +++ b/packages/services/service-datasource/src/datasource-connection-service.ts @@ -36,6 +36,7 @@ import { allowAllConnectPolicy, type DatasourceConnectPolicy, type DatasourceConnectContext, + type DatasourceConnectDecision, } from './contracts/connect-policy.js'; import type { Logger } from './logger.js'; @@ -84,6 +85,23 @@ export interface ConnectionEngineLike { * `onEnable` bridge does manually). */ syncObjectSchema?: (objectName: string) => Promise; + /** + * Tell the engine a datasource was *declared* but is not connected, and why + * (framework#3828). Without this the engine cannot distinguish "the app + * misspelled a datasource name" from "the host's policy refused it" from "it + * failed to connect and the operator set OS_ALLOW_DRIVER_CONNECT_FAILURE" — + * all three used to surface as the same bare `is not registered`. + * + * `publicDetail` is the only part safe to echo to an end user; the operator + * -facing reason stays in the logs and the datasource-admin list. + */ + markDatasourceUnavailable?: (info: { + name: string; + kind: 'blocked' | 'failed'; + publicDetail?: string; + }) => void; + /** Drop a previous {@link markDatasourceUnavailable} record (reconnect / removal). */ + clearDatasourceUnavailable?: (name: string) => void; } /** Secret dereference surface (the `SecretBinder.resolve`, Phase 2 / D3). */ @@ -119,6 +137,44 @@ export interface ConnectResult { reason?: string; } +/** + * How a {@link ConnectStatus} reads to someone asking "can I use this + * datasource right now?" (framework#3827). + * + * - `available` — a live driver is registered (`connected`, or + * `already-registered` via the D8 `onEnable` escape hatch). + * - `blocked` — the host connect policy refused it. A decision, not a + * fault: never fail-fast, and it is expected to persist. + * - `failed` — a connect was attempted and did not produce a usable + * driver (unreachable, bad credential, unsupported driver). + * - `unattempted` — no verdict: the D2 gate left it metadata-only, or there + * was no factory/engine to try with. NOT the same as + * healthy, and NOT the same as broken. + */ +export type DatasourceAvailability = 'available' | 'blocked' | 'failed' | 'unattempted'; + +/** The retained outcome of the last connect attempt for one datasource. */ +export interface DatasourceConnectionState extends ConnectResult { + availability: DatasourceAvailability; +} + +/** Map a raw {@link ConnectStatus} onto its coarse availability class. */ +export function availabilityOf(status: ConnectStatus): DatasourceAvailability { + switch (status) { + case 'connected': + case 'already-registered': + return 'available'; + case 'skipped-policy': + return 'blocked'; + case 'skipped-unsupported': + case 'failed-credentials': + case 'failed-degraded': + return 'failed'; + case 'skipped-no-infra': + return 'unattempted'; + } +} + /** * ADR-0062 D2 — is this declared datasource "meaningfully addressed", such that * auto-connecting it is safe and intended? @@ -160,12 +216,34 @@ export class DatasourceConnectionService { private readonly policy: DatasourceConnectPolicy; private readonly logger?: Logger; + /** + * Last connect verdict per datasource (framework#3827). + * + * Every `connect()` already produced a {@link ConnectResult} and every caller + * threw it away, which is why a datasource that failed at boot was invisible + * for the rest of the process: the admin list reported a hardcoded + * `'unvalidated'` for everything, and `checkDriversHealth()` cannot see a + * driver that was never registered. Retaining the verdict is what lets both + * the admin surface and the query-time error say something true. + */ + private readonly states = new Map(); + constructor(cfg: DatasourceConnectionServiceConfig) { this.cfg = cfg; this.policy = cfg.policy ?? allowAllConnectPolicy; this.logger = cfg.logger; } + /** The last connect verdict for one datasource, or `undefined` if never attempted. */ + getConnectionState(name: string): DatasourceConnectionState | undefined { + return this.states.get(name); + } + + /** Every retained connect verdict, in first-attempt order. */ + listConnectionStates(): DatasourceConnectionState[] { + return Array.from(this.states.values()); + } + /** * Auto-connect the declared (code-defined) datasources that pass the D2 gate. * Called from `AppPlugin.start()` with the app bundle's datasources + objects. @@ -221,10 +299,61 @@ export class DatasourceConnectionService { * explicitly bound by objects. Everything else degrades with a warning so an * optional replica's connectivity blip never bricks boot. See * {@link handleFailure}. + * + * Whatever the outcome — including the fail-fast throw — it is retained in + * {@link getConnectionState} and, when the datasource ends up unusable, + * reported to the engine so a query against a bound object can say *why* + * instead of a bare "is not registered" (framework#3827 / #3828). */ async connect( record: ConnectableDatasource, opts: { objects?: readonly string[]; context?: DatasourceConnectContext } = {}, + ): Promise { + try { + const result = await this.attemptConnect(record, opts); + this.recordState(result, this.lastPublicDetail); + return result; + } catch (err) { + // A D5 fail-fast verdict. The boot is about to abort, but a host that + // catches it (tests, an embedder, a plugin that degrades on its own) must + // not be left with a datasource whose state says "never attempted". + this.recordState( + { name: record.name, status: 'failed-degraded', reason: errMsg(err) }, + undefined, + ); + throw err; + } finally { + this.lastPublicDetail = undefined; + } + } + + /** + * Policy-supplied, tenant-safe detail for the connect currently in flight. + * Threaded through an instance field rather than the {@link ConnectResult} so + * the public result shape stays the operator-facing one: `reason` is + * privileged, and only this opt-in string may reach an end user (#3828). + */ + private lastPublicDetail?: string; + + /** Retain the verdict and mirror an unusable datasource into the engine. */ + private recordState(result: ConnectResult, publicDetail: string | undefined): void { + const availability = availabilityOf(result.status); + this.states.set(result.name, { ...result, availability }); + const engine = this.cfg.engine(); + if (availability === 'available' || availability === 'unattempted') { + engine?.clearDatasourceUnavailable?.(result.name); + return; + } + engine?.markDatasourceUnavailable?.({ + name: result.name, + kind: availability === 'blocked' ? 'blocked' : 'failed', + ...(publicDetail ? { publicDetail } : {}), + }); + } + + private async attemptConnect( + record: ConnectableDatasource, + opts: { objects?: readonly string[]; context?: DatasourceConnectContext } = {}, ): Promise { const name = record.name; const engine = this.cfg.engine(); @@ -237,7 +366,7 @@ export class DatasourceConnectionService { } // Policy gate (fail-closed on throw). - let decision; + let decision: DatasourceConnectDecision; try { decision = await this.policy.canConnect( { name, driver: record.driver, schemaMode: record.schemaMode, external: record.external }, @@ -248,6 +377,9 @@ export class DatasourceConnectionService { } if (!decision.allow) { this.logger?.info?.(`datasource '${name}': connect denied by policy${decision.reason ? ` (${decision.reason})` : ''}`); + // `reason` is operator-facing and stays in logs + the admin list; + // only the opt-in `publicReason` may reach a tenant (#3828). + this.lastPublicDetail = decision.publicReason; return { name, status: 'skipped-policy', reason: decision.reason }; } @@ -341,7 +473,8 @@ export class DatasourceConnectionService { /** Gracefully disconnect a previously-registered datasource pool. */ async disconnect(name: string): Promise { - const driver = this.cfg.engine()?.getDriverByName?.(name) as { disconnect?: () => Promise } | undefined; + const engine = this.cfg.engine(); + const driver = engine?.getDriverByName?.(name) as { disconnect?: () => Promise } | undefined; if (typeof driver?.disconnect === 'function') { try { await driver.disconnect(); @@ -349,6 +482,11 @@ export class DatasourceConnectionService { this.logger?.warn?.(`datasource '${name}': disconnect failed: ${errMsg(err)}`); } } + // A removed/updated pool has no verdict any more. Leaving a stale `failed` + // behind would make the admin list — and the query-time error — describe a + // datasource that no longer exists in that state. + this.states.delete(name); + engine?.clearDatasourceUnavailable?.(name); } /** @@ -418,9 +556,10 @@ export class DatasourceConnectionService { } const banner = `⚠️ DEGRADED BOOT: ${msg} (${why}), but OS_ALLOW_DRIVER_CONNECT_FAILURE is set — starting ` + - `anyway. Queries against the objects bound to it fail with "Datasource '${record.name}' is ` + - `not registered" until it is reachable AND the server is restarted: nothing re-runs the ` + - `connect. Unset OS_ALLOW_DRIVER_CONNECT_FAILURE to restore fail-fast boot.`; + `anyway. Queries against the objects bound to it fail with ERR_DATASOURCE_UNAVAILABLE (HTTP ` + + `503) until it is reachable AND the server is restarted: nothing re-runs the connect. ` + + `Its state shows as 'error' in Setup → Datasources. Unset OS_ALLOW_DRIVER_CONNECT_FAILURE ` + + `to restore fail-fast boot.`; this.logger?.warn?.(banner); // …and again on a channel the host cannot silence — see the helper's note // on `os serve`'s boot-quiet stdout capture.