Repository navigation
Commit ea08ce5
fix(service-datasource): a missing database client (pg, mysql2, better-sqlite3) is refused with its install, not the unbuilt-workspace build advice (#24933)
Fixes #24909
Clause-②: no
## What was wrong
When a boot-critical datasource could not connect because a module could
not be resolved, `connectFailureRemedy()` always ended the refusal on
`UNBUILT_WORKSPACE_REMEDY`: "… That is a build precondition … Run `pnpm
install && pnpm build`, then start again." That is right for the case it
was written for, a monorepo worktree whose driver `dist/` is missing. It
is wrong for a project that points at PostgreSQL or MySQL without
installing the database client. `pg` and `mysql2` are optional peers of
`@objectstack/driver-sql`, and `better-sqlite3` is an optional
dependency of it. The project installs them itself, so neither command
adds them, and a reader who followed the advice was refused again with
the same text.
## What changed
All of the code change is in
`packages/services/service-datasource/src/connect-failure-remedy.ts`. It
is a message branch only.
- The chooser reads WHICH module could not be resolved: the first
`Cannot find module|package 'X'` in the error text.
- If X is exactly one of the closed list
`OPTIONAL_DRIVER_CLIENT_PACKAGES` (`pg`, `mysql2`, `better-sqlite3`),
the refusal ends on `missingDriverClientRemedy(X)`. That list is the
clients the factory's knex arms load. The remedy names the install of
that client once (`npm install X`, with `pnpm add X / yarn add X` beside
it). It names no build step, no environment variable and no
configuration key.
- `isUnbuiltWorkspaceFailure()` no longer answers true for that case.
- Everything else is unchanged. A path, any `@objectstack/…` package, a
subpath and any other bare name keep `UNBUILT_WORKSPACE_REMEDY`. Every
cause that is not a module-not-found keeps
`GENERIC_CONNECT_FAILURE_REMEDY` byte for byte. When the boot is
refused, the thrown error type, the retained connection state and the
`OS_ALLOW_DRIVER_CONNECT_FAILURE` degraded boot do not change.
- `packages/types/**`, `packages/spec/**` and `packages/drivers/**` are
untouched. `isModuleNotFoundError` is still the gate in front of the new
read.
The outside suggestion in comment 6292687799 proposed the same split:
keep the unbuilt remedy for missing build output, add an install remedy
for a missing optional peer, pin both, and keep `ECONNREFUSED`
unchanged. This PR adopts that idea. Its point about keeping the
original thrown error long enough to classify it was already true on
this path, because `handleFailure` receives the thrown value. The
measurement below shows the specifier exists only in the text anyway.
## Measurements (dispatch assumptions H1 to H4)
Taken on the branch base `b42c621c05`. Between the
`@objectstack/service-datasource@18.0.0` tag and that base,
`connect-failure-remedy.ts` and `packages/types/src/module-not-found.ts`
are byte-identical (empty `git diff`), so the readings hold for the
published release.
- **H1, confirmed.** The base chooser was `isModuleNotFoundError(cause)
? UNBUILT : GENERIC`. Nothing on the path read the specifier.
- **H2, measured.** The real factory was run
(`createDefaultDatasourceDriverFactory({ dev: false })`, built ESM and
CJS dist), with the client made unresolvable through Node's own
resolver. postgres/`pg`, mysql/`mysql2` and sqlite/`better-sqlite3` all
throw knex's wrapper `Knex: run` / `$ npm install X --save` / `Cannot
find module 'X'` / `Require stack: …`, with `code` undefined. The signal
is in the text only, the same as for the sqlite-wasm and mongodb arms.
Node's own errors for a bare specifier carry no field naming it either:
CJS has `code` and `requireStack`, and ESM `Cannot find package` has
`code` only.
- **H3, measured on every arm, ESM and CJS.**
- Package not installed: a bare specifier (`Cannot find package
'@objectstack/driver-…'` under ESM, `Cannot find module
'@objectstack/driver-…'` under CJS).
- Installed but unbuilt: an absolute path into `…/dist/index.mjs` or
`…/dist/index.cjs`.
- Missing client: a bare third-party name.
The split holds on every arm. A missing `@objectstack/driver-turso` (and
mongodb, sqlite-wasm) stays on the unbuilt remedy in this PR; see open
question 2.
- **H4.** The server cannot know the package manager. The chosen wording
is one command, `npm install X`, with the pnpm and yarn spellings of the
same command beside it. That is how the factory's optional-driver
messages in the same package already spell an install. See open question
1.
## The card's own path: a published 18.0.0 scaffold outside any
workspace
`npm create objectstack@18.0.0 my-app -- --skip-skills` (installed with
pnpm, no `pg`), then `npx os dev --no-watch -p PORT -d
postgres://postgres@127.0.0.1:1/db`:
- **Before** (published code): exit 1, ending "… Run `pnpm install &&
pnpm build`, then start again." This is the card's refusal, reproduced.
- **After**: this PR's compiled remedy functions were transplanted into
that scaffold's copy of the published bundle. Exit 1 (the boot is still
refused), ending "The database client package `pg` is not installed, so
no connection was attempted. … Install it next to the server that opens
this datasource — `npm install pg` (pnpm add pg / yarn add pg) — then
start again." `-d mysql://…` gives the same ending with `mysql2`.
- **Following the remedy**: `pnpm add pg` (exit 0), then start again.
The boot gets past the client and is refused for the next real fault,
`connect ECONNREFUSED 127.0.0.1:1`, ending on the generic sentence
unchanged.
## Tests (head `649414d3b8`)
- `pnpm --filter @objectstack/service-datasource typecheck`: exit 0.
`tsc --listFiles` confirms it compiles both changed test files.
- `pnpm --filter @objectstack/service-datasource exec vitest run
--maxWorkers=2`: 49 files, 883 tests passed.
- Unit pins (`datasource-connection-service.test.ts`):
- Each measured knex client shape (`pg`, `mysql2`, `better-sqlite3`),
and ESM `Cannot find package 'pg'`, ends on that client's install. The
remedy contains `npm install X` exactly once and never `pnpm build`. No
`OS_` variable or `--database` appears.
- The unbuilt-workspace pins stay unchanged and green. The new boundary
cases also keep `UNBUILT_WORKSPACE_REMEDY`: `@objectstack/driver-sql`
not installed, unbuilt `dist`, a file inside `pg`, the `pg/lib/native`
subpath, and `knex` missing.
- `ECONNREFUSED` and `28P01` (with their codes) still end on the generic
sentence.
- Real boot-refusal path (new
`missing-driver-client-boot-refusal.test.ts`):
- The real factory and the real `DatasourceConnectionService` connect
the host's boot-critical `default` under `declared-auto`. The client is
made unresolvable through Node's own resolver for that one specifier, so
the error is Node's and knex's, not the test's.
- For pg, mysql2 and better-sqlite3 (sqlite with `dev: false`), the boot
is still refused with a plain `Error`. The message carries `Cannot find
module 'X'`, ends on that client's install and never names `pnpm build`.
The verdict is retained (`failed-degraded`, availability `failed`).
- The escape hatch still boots degraded.
- Control: with the client present, the same harness is refused for
`ECONNREFUSED` and ends on the generic sentence.
- The SQL driver is loaded at module top, so no case's clock pays the
cold load. The pg case went from 9496 ms to 12 ms.
- Ablation: two legs, one per changed behaviour, both through `node
scripts/ablation-replace.mjs` (anchor hit 1 to 0 on disk, blob changed).
The subject is imported from `src`, so no rebuild is involved. Each
restore was proven by blob equal to HEAD (`78e1c14f40fb`) and an empty
`git diff HEAD`.
1. Deleting the client branch in `connectFailureRemedy`: 7 red, exactly
the predicted set (the 3 knex unit cases, the ESM `pg` case and the 3
real-path arms). The 83 others stayed green.
2. Deleting the client exclusion in `isUnbuiltWorkspaceFailure`: 1 red,
the classifier pin. The 89 others stayed green.
## Gates
Local, on head `649414d3b8`. The list was derived with `node
scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` (no paths; merge base `74fcec1ed`). Each
exit code was captured before any pipe.
- 66 commands derived. 65 exited 0.
- 1 is NOT MEASURED: `pnpm check:dual-build-cjs-loads` exited 3 with
PREREQUISITE NOT MET. That gate reads every package's built output, and
this worktree built only the service-datasource closure. CI runs it.
- `--ran` reconciliation, with each exit recorded: "66 derived
famil(ies) accounted for — 65 run, 1 NOT-MEASURED (1 DERIVED from a
recorded exit 3)".
- The 65 include `check:test-source-alias`,
`check:cross-package-test-inputs`, `check:engine-double-contract`,
`check:doc-authoring`, `check:nul-bytes`, `check:type-check-coverage`,
`check:type-check-debt`, `check-adr-0087-registration --base
origin/main` and `check-empty-changeset --base origin/main`.
- Not run locally, left to CI:
- the `test:repo` vitest project (the diff touches none of its tests);
- repo-wide scans, `pnpm lint` first among them;
- the local-only families `check:dev-prereqs` and
`check:skill-frame-freshness`, which nothing in the diff triggers (no
`packages/spec` build and no skill-frame file).
- The PR's check-runs had not finished when this PR was opened; the dev
report cites them.
## Open questions
1. **Install wording (H4).** A: one command, `npm install X` only. B:
the module name and a neutral sentence ("add `X` with your package
manager"). C (chosen): `npm install X` with `pnpm add X / yarn add X`
beside it.
- Real need: the scaffolder picks pnpm when it is on PATH and npm
otherwise (`detect-package-manager.ts`), and the card's reader used `npm
create` and then pnpm. So both npm and pnpm projects are real. A serves
only one of them; B and C serve both.
- Long-term fit: C is the spelling the factory's optional-driver
messages in this package already use. B would add a second spelling for
the same situation.
- Harder to get wrong: C gives a copy-pasteable command for each package
manager, and an agent can pick the one matching the lockfile. A invites
`npm install` inside a pnpm project. B leaves the reader to work out the
command.
- Scope: none of the three adds a mechanism (no package-manager
detection on the server).
- Recommendation: C.
2. **A missing optional `@objectstack/driver-*` package (turso, mongodb,
sqlite-wasm), reachable today.** Measured through `os dev` on the
published 18.0.0 scaffold: a declared non-default `turso` datasource
with a bound object, and no `@objectstack/driver-turso` installed. The
refusal names two fixes. The factory's own text says `npm install
@objectstack/driver-turso`, and the appended remedy says "That is a
build precondition … Run `pnpm install && pnpm build`". This PR leaves
that case on the unbuilt remedy, because the bare specifier means two
things. In a scaffold it is a missing install. In the workspace it is a
stale install, which `pnpm install` repairs, and `npm install` inside a
pnpm workspace would be wrong. The #7385 factory pins also assert that
these texts classify as unbuilt.
- A: leave it as it is.
- B: for a bare specifier naming one of the three optional peers this
package declares, end on a sentence that defers to the install the
factory text already names, and keep paths on the unbuilt remedy. The
cost is small: the same file, plus revising the three #7385 classifier
pins to path-shaped causes.
- C: as B, but for `@objectstack/driver-turso` only. The CLI and runtime
depend on mongodb and sqlite-wasm outright, so a scaffold normally has
them.
- Real need: only C's case is measured as reachable from a scaffold.
Long-term fit: B is the one rule, "a bare optional install is an
install". Harder to get wrong: B and C give one fix per refusal, A gives
two. Scope: all are small.
- Recommendation: B, with the workspace stale-install trade-off stated
for the maintainer to accept or reject.
## Acceptance notes
- `content/docs/getting-started/your-first-project.mdx` (the package
table and the paragraph under it) says `@objectstack/runtime` "brings
the database drivers with it — you do not install one separately". It
also says to "point `OS_DATABASE_URL` at Postgres / MySQL / MongoDB for
production — no code changes". It never says that `pg` or `mysql2` must
be installed. This is the step the card's reproduction followed. Not
changed here: it does not quote the refusal. It is reported to the seat
as a finding.
- `drivers.mdx` already says `pg` and `mysql2` are optional peers to
install yourself. No page quotes the old refusal for the missing-client
case, so no doc changed.
- A workspace contributor with a stale install that lacks `pg` would now
be told `npm install pg (pnpm add pg / …)` rather than `pnpm install`.
pnpm links `pg` into the workspace as a devDependency of `driver-sql` on
every install, so this needs an install that predates `pg`. The module
note says so.
- No `CHANGELOG.md` or `content/docs/releases/` edit. The changeset
`.changeset/24909-missing-driver-client-remedy.md` is a `patch` for
`@objectstack/service-datasource`.
> **Seat's append** (`domain:services` seat 1,
`session_011TdZ8GjixNZFLz1xJD9zzK`): the dev's round-2 markdown from
os-dev report `6296337366` on #24909, verbatim. The dev writes the body
once and never PATCHes it. The finding above about
`your-first-project.mdx` is now fixed in this PR, by claim amendment
`6295994725`.
## Round 2: the getting-started page (claim amendment 6295994725)
`content/docs/getting-started/your-first-project.mdx`: only the package
table's `@objectstack/runtime` row and the paragraph under it changed.
No new section.
- The row no longer says "you do not install one separately". It now
says that SQLite and MongoDB work as installed, and that PostgreSQL and
MySQL also need their client package.
- The paragraph keeps the dependency sentence, which was measured true,
and adds that the runtime installs `driver-mongodb` as an optional
dependency.
- SQLite (the default) needs nothing more.
- MongoDB needs only `OS_DATABASE_URL`.
- Postgres and MySQL need their client installed first (`npm install pg`
/ `npm install mysql2`, the page's npm spelling). It is an optional peer
of `driver-sql`, and without it the boot is refused.
- The bare "no code changes" link is now "No code changes either way —
see [Drivers](/docs/data-modeling/drivers)".
Measured on a fresh published `create-objectstack@18.0.0` scaffold
(pnpm-installed, outside any workspace) and on the 18.0.0 npm manifests:
- `@objectstack/runtime@18.0.0`: dependencies `driver-sql`,
`driver-memory`, `driver-sqlite-wasm`; optionalDependencies
`driver-mongodb`; optional peer `driver-turso`. The workspace head has
the same edges.
- `@objectstack/cli@18.0.0`: dependencies `driver-sql`, `driver-memory`,
`driver-mongodb`, `driver-sqlite-wasm`; optional peer `driver-turso`.
- `@objectstack/driver-mongodb@18.0.0` depends on `mongodb`.
`@objectstack/driver-sql@18.0.0` has optional peers `pg` and `mysql2`
and the optional dependency `better-sqlite3`.
- In the scaffold, `driver-mongodb`, `driver-sqlite-wasm` and
`driver-sql` resolve from service-datasource, and `mongodb` resolves
from `driver-mongodb`. `pg` and `mysql2` do not resolve.
- `os dev` with no database flag: "Server is ready" on
`SqlDriver(better-sqlite3)`, and `GET /api/v1/discovery` answered 200.
The server was stopped afterwards.
- `os dev -d mongodb://127.0.0.1:1/db`: exit 1 on `connect ECONNREFUSED
127.0.0.1:1` with the generic sentence. The driver and client loaded;
only the absent server refused.
- Postgres and MySQL without their client: refused with exit 1, as
measured in round 1.
A correction to round 1: open question 2 said "the CLI and runtime
depend on mongodb and sqlite-wasm outright". That holds for the CLI. The
runtime carries `driver-mongodb` as an optionalDependency, which is
installed by default and skipped under `--omit=optional`.
The changeset is unchanged: `content/docs/**` publishes nothing, and the
existing `patch` changeset covers the code.
Gates on head `907bd42fdb`, re-derived: 95 commands (the 66 from round 1
plus 29 docs families). The 29 added ones and 8 docs-reading round-1
families (`check:doc-authoring` among them) were re-run, each under the
lock. All exit 0 once the `lint`, `formula` and `client-react` closures
were built. Before those builds, 4 had exited 3 (prerequisite). The
derived link family is `check:doc-anchors`, exit 0; CI's lychee lane is
`Check Documentation Links`. `--ran`: 95 accounted, 94 run, 1 NOT
MEASURED (`check:dual-build-cjs-loads`, which needs every package
built).
---
_Generated by [Claude
Code](https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent e7befa7 commit ea08ce5
5 files changed
Lines changed: 451 additions & 7 deletions
File tree
- .changeset
- content/docs/getting-started
- packages/services/service-datasource/src
- __tests__
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
242 | 242 | | |
243 | 243 | | |
244 | 244 | | |
245 | | - | |
| 245 | + | |
246 | 246 | | |
247 | 247 | | |
248 | 248 | | |
249 | 249 | | |
250 | 250 | | |
251 | 251 | | |
252 | | - | |
253 | | - | |
254 | | - | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
255 | 259 | | |
256 | 260 | | |
257 | 261 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| 15 | + | |
15 | 16 | | |
| 17 | + | |
| 18 | + | |
16 | 19 | | |
17 | 20 | | |
18 | 21 | | |
| |||
589 | 592 | | |
590 | 593 | | |
591 | 594 | | |
| 595 | + | |
| 596 | + | |
| 597 | + | |
| 598 | + | |
| 599 | + | |
| 600 | + | |
| 601 | + | |
| 602 | + | |
| 603 | + | |
| 604 | + | |
| 605 | + | |
| 606 | + | |
| 607 | + | |
| 608 | + | |
| 609 | + | |
| 610 | + | |
| 611 | + | |
| 612 | + | |
| 613 | + | |
| 614 | + | |
| 615 | + | |
| 616 | + | |
| 617 | + | |
| 618 | + | |
| 619 | + | |
| 620 | + | |
| 621 | + | |
| 622 | + | |
| 623 | + | |
| 624 | + | |
| 625 | + | |
| 626 | + | |
| 627 | + | |
| 628 | + | |
| 629 | + | |
| 630 | + | |
| 631 | + | |
| 632 | + | |
| 633 | + | |
| 634 | + | |
| 635 | + | |
| 636 | + | |
| 637 | + | |
| 638 | + | |
| 639 | + | |
| 640 | + | |
| 641 | + | |
| 642 | + | |
| 643 | + | |
| 644 | + | |
| 645 | + | |
| 646 | + | |
| 647 | + | |
| 648 | + | |
| 649 | + | |
| 650 | + | |
| 651 | + | |
| 652 | + | |
| 653 | + | |
| 654 | + | |
| 655 | + | |
| 656 | + | |
| 657 | + | |
| 658 | + | |
| 659 | + | |
| 660 | + | |
| 661 | + | |
| 662 | + | |
| 663 | + | |
| 664 | + | |
| 665 | + | |
| 666 | + | |
| 667 | + | |
| 668 | + | |
| 669 | + | |
| 670 | + | |
| 671 | + | |
| 672 | + | |
| 673 | + | |
592 | 674 | | |
593 | 675 | | |
594 | 676 | | |
| |||
679 | 761 | | |
680 | 762 | | |
681 | 763 | | |
| 764 | + | |
| 765 | + | |
| 766 | + | |
| 767 | + | |
| 768 | + | |
| 769 | + | |
| 770 | + | |
| 771 | + | |
| 772 | + | |
| 773 | + | |
| 774 | + | |
| 775 | + | |
| 776 | + | |
| 777 | + | |
| 778 | + | |
| 779 | + | |
| 780 | + | |
| 781 | + | |
| 782 | + | |
| 783 | + | |
| 784 | + | |
| 785 | + | |
| 786 | + | |
| 787 | + | |
| 788 | + | |
| 789 | + | |
| 790 | + | |
| 791 | + | |
682 | 792 | | |
683 | 793 | | |
| 794 | + | |
| 795 | + | |
| 796 | + | |
| 797 | + | |
684 | 798 | | |
685 | 799 | | |
686 | 800 | | |
| |||
0 commit comments