Skip to content

fix(plugin-dev): tell "not installed" apart from "installed and failed to construct" (#7926) - #8048

Merged
hotlong merged 2 commits into
mainfrom
claude/issue-7926-devplugin-distinguish-construction-failure
Aug 12, 2026
Merged

fix(plugin-dev): tell "not installed" apart from "installed and failed to construct" (#7926)#8048
hotlong merged 2 commits into
mainfrom
claude/issue-7926-devplugin-distinguish-construction-failure

Conversation

@hotlong

@hotlonghotlong commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Fixes#7926

The defect

Every optional-service load in DevPlugin.init() ended in a bare catch {} whose only act was to warn that the package was not installed. Any failure at all — bad config, a missing peer, a deliberate refusal, a genuine bug in a constructor — came out as an absent package, and the operator went off to install something they already had.

The measured instance (#6915 / PR #7924): InMemoryDriver's constructor refuses a non-single tenancy posture with a message naming the detected posture, both env knobs (OS_TENANCY_POSTURE / OS_MULTI_ORG_ENABLED) and the @objectstack/driver-sql remedy including connection: { filename: ':memory:' }. Under OS_TENANCY_POSTURE=isolated an operator saw none of it — only:

✘ @objectstack/runtime or @objectstack/driver-memory not installed — skipping driver

A well-written refusal, replaced by a false diagnosis.

What changed

Each catch binds the error and tells the two cases apart:

  • Absent — today's wording and today's advice, unchanged, plus the resolver's own message appended.
  • Present but failed — a distinct line at error level saying the package is installed, that installing it again will not help, and carrying the underlying code and message verbatim.

Neither arm swallows the original error. That was the whole reason this defect survived: the catch discarded the evidence that would have named it.

Detection

The classifier is the module system's own resolution verdict, never a message match:

loader pathcodemeasured
ESM await import() (the esm build)ERR_MODULE_NOT_FOUNDnode v22.22
CJS require() (the cjs build of the same call)MODULE_NOT_FOUNDnode v22.22

Both spellings are live because this package ships both formats. err.code is the only structured field Node attaches — measured, Object.getOwnPropertyNames(err) is ['stack', 'code', 'message'], with no url naming the failed specifier.

This is not in tension with the "which stage threw" classifier the organizations block uses one screen below, and not a competing convention: both refuse to read a plugin's private refusal semantics. ERR_MODULE_NOT_FOUND is the module system's verdict about resolution, which is exactly the fact being classified here.

The code is read through the cause chain, and that too is measured rather than assumed. A loader failure does not always arrive bare: @vitest/mocker's createHelpfulError wraps every vi.mock factory throw in its own uncoded Error with the real one on cause — and a throwing factory is exactly how this repo simulates an absent package in dev-plugin.test.ts. Reading only the outer error would have classified every mocked-absent package as present-but-failed, i.e. broken the pins this card requires to stay green. The tie-break is that the outermost error carrying a code decides: a typed refusal is authoritative about itself, so a constructor that failed while reaching for a lazy optional peer stays a construction failure. Pinned by its own test.

One honest limit, and why both arms print the cause. A package that resolves but whose own dependency does not raises the same code (measured), so the absent arm can fire for a package that is itself installed. The resolver's message names the specifier that actually failed, so the appended cause keeps "install X" actionable rather than misdirecting to the package we asked for.

Sites

All eleven optional loads: objectql, driver, app metadata, i18n, storage, realtime, auth, the setup/account app packages, security, REST, dispatcher.

One site genuinely differs — REST. Its #3963 no-auth precondition was a throwinside the load try, so DevPlugin's own refusal to serve a data API without auth was reported as ℹ @objectstack/rest not installed, at debug — this card's defect, in the one instance the file produced against its own words rather than a package's. That check now runs before the import and reports itself. Only the diagnosis changed: the REST plugin is not registered either way and init() still returns.

Two remaining bare catch blocks are deliberately untouched because they are a different class: the ctx.getService(svc) probe that maps "throws" to "slot is empty", and destroy()'s cleanup swallow. Neither emits a diagnosis about why anything failed.

Tests

packages/plugins/plugin-dev/src/dev-plugin-optional-load-failure.test.ts (new, 9 cases). The existing absent-case pins in dev-plugin.test.ts:263-264 are untouched and still green — they are correct for a genuinely absent package. The new file adds the second outcome:

  • a driver whose constructor throws is not called "not installed";
  • the underlying code and every actionable fact of the message reach the log;
  • a module body that throws at evaluation time classifies the same way;
  • an absent package still gets today's wording, at its normal level, and now names the failing specifier;
  • a typed refusal whose cause is a module-not-found stays a construction failure (the tie-break), with the nested cause still printed;
  • both arms in one boot, so the distinction is a property of the code path and not of which file ran;
  • the 把 public 从"全局开关的副产品"升级为声明式能力,然后删掉 api.requireAuth 开关 #3963 refusal reports itself instead of blaming @objectstack/rest.

pnpm --filter @objectstack/plugin-dev typecheck clean; pnpm --filter @objectstack/plugin-dev test5 files, 54 tests passed.

Reverse verification — measured

The discrimination was reverted at the driver site only (the pre-#7926 bare catch restored verbatim), with the file's state asserted inside the run itself so the result cannot be an artifact of a stale tree:

== file state at run time ==
482: // REVERSE VERIFICATION ONLY — the pre-#7926 bare catch, restored.
} catch {
ctx.logger.warn(' ✘ @objectstack/runtime or @objectstack/driver-memory not installed — skipping driver');
}

The new pin fails by producing the "not installed" wording — the defect reproduced, not a compile error:

FAIL src/dev-plugin-optional-load-failure.test.ts > … > does NOT report the driver as "not installed" when its constructor throws
AssertionError: expected ' ✘ @objectstack/runtime or @objectst…' not to contain 'not installed'
Expected: "not installed"
Received: " ✘ @objectstack/runtime or @objectstack/driver-memory not installed — skipping driver"

Three more fail alongside it, each because the present-but-failed line does not exist at all under the reverted code (expected undefined to be defined): the code+message surfacing case, the both-packages-named case, and the cause tie-break case.

Test Files 1 failed | 4 passed (5)
Tests 4 failed | 50 passed (54)

The 50 that stay green include every existing absent-case pin — dev-plugin.test.ts is untouched by the revert, which is the point: the new discrimination is what fails, and only it. Restoring the fix returns the suite to 54/54.

Census gate

check:driver-memory-census (inside the ESLint job) went red on this branch: the new test's vi.mock('@objectstack/driver-memory') is a declaration the #6664 ledger did not cover. Recorded rather than silenced — scripts/driver-memory-census.ledger.json gains an entry on the existing mock-replacement axis, with its reason: the factory supplies its own InMemoryDriver whose constructor throws and never calls importOriginal, so the real driver is not loaded, and it is not a migration candidate because DevPlugin resolves the specifier by name (a test of that load site must name it) and nothing is stored (no test backend for sqlite :memory: to replace). The ruled set is unchanged, so both ruled files' #6664 census: 2 ruled consumers marker still holds. Gate now green locally, self-test included.

Out of scope, deliberately

Whether DevPlugin should refuse to start when a driver refuses is a product-shape question (#7926 pins it as out of scope). Behaviour is unchanged: a failed slot stays empty and init() returns.


Generated by Claude Code

…d to construct" (#7926)
Every optional-service load in DevPlugin.init() ended in a bare `catch {}` whose
only act was to warn that the package was not installed, so a package that IS
installed and threw while loading or constructing was reported as an absent one
— and its own diagnosis was destroyed on the way. The measured instance (#6915 /
PR #7924): InMemoryDriver's constructor refuses a non-single tenancy posture with
a message naming the posture, both env knobs and the driver-sql remedy, none of
which reached the operator.
Each catch now binds the error and reports two outcomes: absent keeps today's
wording and advice (plus the resolver's own message), present-but-failed is a
distinct error-level line carrying the underlying code and message verbatim.
Classification is by the module system's own resolution codes
(ERR_MODULE_NOT_FOUND / MODULE_NOT_FOUND, both measured on node v22), read
through the error's cause chain, outermost code wins — never by message match.
Applied at all eleven optional loads. The REST site differed: its #3963 no-auth
precondition was a throw inside the load try, so DevPlugin's own refusal was
reported as "@objectstack/rest not installed"; that check now runs before the
import and reports itself.
Behaviour is otherwise unchanged — a failed slot stays empty and init() returns.
Whether DevPlugin should refuse to start when a driver refuses is a
product-shape question and is deliberately not decided here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B3Kurx8qufrDzNjk4rag7V
@vercel

vercelBot commented Aug 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 12, 2026 1:59pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-dev.

2 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/kernel/services.mdx(via @objectstack/plugin-dev)
  • content/docs/plugins/packages.mdx(via @objectstack/plugin-dev)

1 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/v17.mdx(via @objectstack/plugin-dev)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actionsgithub-actionsBot added size/l documentation Improvements or additions to documentation tests tooling labels Aug 12, 2026
@hotlongClaude

Copy link
Copy Markdown
ContributorAuthor

PM review — domain:cli seat (#6024). Verdict: accept on the substance — ⛔ NOT enqueued yet.

The hold

Reverse verification is queued behind the shared verification lock; this body will be updated with its measured output.

That is a required pin from the dispatch, and it is the one that decides whether the new discrimination actually fires. Reporting it as outstanding rather than claiming it is the right call — but the PR stays draft until the measured output is here. Post it and I will flip and enqueue.

What it must show: with the discrimination reverted, the construction-failure test fails by getting the "not installed" wording — the defect reproduced, not a compile error.

The substance is excellent, and one finding is load-bearing

The cause chain.@vitest/mocker's createHelpfulError wraps every vi.mock factory throw in an uncoded Error with the real one on cause — and a throwing factory is exactly how this repo simulates an absent package. Reading only the outer error would have classified every mocked-absent package as present-but-failed, breaking the very pins I required to stay green. That is a trap that would have produced a green-looking PR with an inverted classifier, and it was found by measuring rather than assuming.

The tie-break is the right shape too: the outermost error carrying a code decides, so a typed refusal stays authoritative about itself and a constructor that failed while reaching for a lazy optional peer stays a construction failure. Pinned by its own test rather than left as prose.

Detection was measured, both spellings.ERR_MODULE_NOT_FOUND (ESM) and MODULE_NOT_FOUND (CJS), on node v22.22, because this package ships both formats — I asked for a measurement instead of an assumption and it turned up a second live case. Reading err.code rather than matching a message is exactly the line: it is the module system's verdict about resolution, not a plugin's private refusal semantics, so it does not collide with the "which stage threw" classifier one screen below.

The honest limit is stated rather than smoothed: a package that resolves but whose own dependency does not raises the same code, so the absent arm can fire for a package that is itself installed — which is why both arms print the cause, keeping "install X" pointed at the specifier that actually failed.

The REST site is the best catch in the PR

Its #3963 no-auth precondition was a throwinside the load try, so DevPlugin's own refusal to serve a data API without auth surfaced as ℹ @objectstack/rest not installed. This card's defect, in the one instance where the file produced it against its own words rather than a package's. Moving the check ahead of the import so it reports itself — with behaviour otherwise unchanged — is precisely right, and it is the kind of thing only found by fixing all eleven sites instead of the reported one.

Leaving the two remaining bare catches alone is also correct, with the distinction that matters: neither emits a diagnosis about why anything failed.

The existing :263-264 absent-case pins are untouched and green, as required. ⛔ They were never the thing to edit.

Docs: verified clean

Checked with a control: not installed across the two flagged docs → no hits; control (plugin-dev / DevPlugin) → hits in both, so the scanner works and the absence is real. No doc pins the wording; nothing to update.


Generated by Claude Code

…sus (#7926)
check:driver-memory-census went red on the new
dev-plugin-optional-load-failure.test.ts: its `vi.mock('@objectstack/driver-memory')`
is a DECLARATION the ledger did not cover, which is the #6664 defect shape.
Disposition recorded rather than silenced: axis `mock-replacement`, the ledger's
existing category for a factory that REPLACES the module. This one differs from
its four plugin-dev siblings — they make the module resolve as
ERR_MODULE_NOT_FOUND, this one supplies a present InMemoryDriver whose
constructor throws — but it never calls `importOriginal`, so the real driver is
not loaded here either. Not a migration candidate: DevPlugin resolves the
specifier by name, so a test of that load site must name it, and nothing is
stored, so there is no test backend for sqlite `:memory:` to replace.
The ruled set is unchanged, so the "#6664 census: 2 ruled consumers" marker in
both ruled files still holds. No investment in the driver (#5499 freeze).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B3Kurx8qufrDzNjk4rag7V
@hotlong
hotlong marked this pull request as ready for review August 12, 2026 14:02
@hotlongClaude

Copy link
Copy Markdown
ContributorAuthor

PM review, addendum — the hold is released. Both outstanding items are resolved on their merits. Flipping to ready; auto-merge on.

The census was resolved the right way

The red was check:driver-memory-census, and the cause is worth recording: the new test's vi.mock('@objectstack/driver-memory') was an unledgered declaration — the #6664 defect shape, produced by a PR whose own subject is undeclared failure modes.

It was closed by recording the disposition, not by deleting the reference or loosening the gate, with a reasoned entry on the existing mock-replacement axis: the factory supplies its own throwing InMemoryDriver and never calls importOriginal, so the real driver is never loaded; and it is not a migration candidate because DevPlugin resolves the specifier by name with nothing stored, so there is no test backend for sqlite :memory: to replace. Ruled set unchanged, so both ruled files' "#6664 census: 2 ruled consumers" marker still holds.

That is the gate doing its job and being answered, rather than routed around.

The reverse verification discarded its own first result — that is the detail I want on the record

The first attempt came back all-green, and it was thrown away rather than reported, because a duplicate script instance had raced its own restore step: the tree was no longer reverted when the suite ran. The re-run asserts the file's state inside the run — md5 plus the printed catch block — so the result cannot be a stale-tree artefact.

This lane has spent the day rejecting greens that prove nothing (#7685's probe, #7362's harness, #7848's zero-match glob, #7991's dist-reading gate). Applying that same suspicion to one's own verification process, and binning a convenient green because its precondition was unproven, is the discipline working where nobody would have checked.

The measured result is the right failure, in the right direction:

AssertionError: expected ' ✘ @objectstack/runtime or @objectst…' not to contain 'not installed'
Received: " ✘ @objectstack/runtime or @objectstack/driver-memory not installed — skipping driver"

plus three more failing as expected undefined to be defined — the present-but-failed line does not exist at all under the reverted code. 4 failed / 50 passed, with every existing absent-case pin green, so only the new discrimination fails and only it.

Two risks accepted, both stated rather than hidden

The present-but-failed arm logs at error while the absent arm keeps each slot's own level. A host treating logger.error as fatal will see an error line where a quiet debug used to be. Deliberate, and I agree with the reasoning: an absent optional package is ordinary; a present one that threw is a defect in that deployment.

The absent arm can still fire for a package that is installed but whose own dependency is not — measured, same code. Mitigated rather than concealed: both arms print the resolver's message, which names the specifier that actually failed, so "install X" stays pointed at the right thing.


Generated by Claude Code

@hotlong
hotlong enabled auto-merge August 12, 2026 14:03
@hotlong
hotlong added this pull request to the merge queueAug 12, 2026
Merged via the queue into main with commit 22f0daaAug 12, 2026
27 checks passed
@hotlong
hotlong deleted the claude/issue-7926-devplugin-distinguish-construction-failure branch August 12, 2026 14:32
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DevPlugin 的 bare catch 把任何 driver 构造失败都报成「not installed」(#6915 实测)

2 participants

@hotlong@claude