Skip to content

Commit 37a7d13

Browse files
dmealingclaude
andcommitted
fix(cli): stop leaking a mutated Error.prepareStackTrace from config loading
loadMetaobjectsConfig permanently mutated the process-global Error.prepareStackTrace. When Bun's native import of a config fails (a config whose module body throws, say), jiti falls back to its bundled Babel transformer, whose rewrite-stack-trace installs a prepareStackTrace wrapper delegating to whatever it captured — and never restores it. On Node the captured value is undefined, so Babel's lenient fallback runs and nothing is harmed. On Bun it is Bun's NATIVE default, which throws `TypeError: First argument must be an Error object` for any target that is not a real ErrorInstance. Once leaked, every later legacy-constructor error in the process throws that TypeError WHILE BEING CONSTRUCTED, destroying its real message. libsql's SqliteError is exactly that shape (ES5 constructor + Error.captureStackTrace), so a genuine "CHECK constraint failed: …" surfaced as "First argument must be an Error object". The loader now snapshots and restores the hook (and stackTraceLimit); restoring is safe because Babel's installer self-neuters after its first call. How it hid is the more useful part: the damage is cross-package, and every CI lane runs `bun test` PER PACKAGE, so cli and migrate-ts never shared a process. Only a workspace-wide `bun test` — the run the contributor docs invite — showed it, as four migrate-ts real-engine gates failing on their error-MESSAGE assertions while the migrate engine behaved correctly. No migration-correctness defect was masked: the constraints fired and every apply → introspect → re-diff-EMPTY loop completed. Also hardens three assertions in sqlite-uuid-pk-kitchen-sink that were not merely weak but VACUOUS: `expect(p).rejects.toThrow()` without `await` returns a promise and never gates the test, so three "VALUE SEMANTICS" checks asserted nothing and passed throughout the pollution. Now awaited and matched on /CHECK/i. The workspace-wide suite is green for the first time: 5763 pass, 24 skip, 0 fail across 652 files (was 4 fail). Fix and regression test are mutation-verified — both tests fail with the restore removed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KTGT5ksntpcJDZVJ5VyXHS
1 parent b852475 commit 37a7d13

4 files changed

Lines changed: 117 additions & 3 deletions

File tree

CHANGELOG.md

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,39 @@ this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
77

88
## [Unreleased]
99

10+
### Fixed — loading a `metaobjects.config.ts` could permanently corrupt error reporting for the rest of the process (npm)
11+
12+
`loadMetaobjectsConfig` leaked a mutation of the process-global `Error.prepareStackTrace`.
13+
When Bun's native import of the config fails — a config whose module body throws, for
14+
instance — jiti falls back to its bundled Babel transformer, whose `rewrite-stack-trace`
15+
**permanently** installs a `prepareStackTrace` wrapper delegating to whatever value it
16+
captured. On Node that value is `undefined`, so Babel's own lenient fallback runs and
17+
nothing is harmed. On Bun it is Bun's **native default**, which throws
18+
`TypeError: First argument must be an Error object` for any target that is not a real
19+
`ErrorInstance`.
20+
21+
Once that wrapper leaks, every subsequent *legacy-constructor* error in the process throws
22+
that `TypeError` **while being constructed**, destroying its real message. libsql's
23+
`SqliteError` is exactly that shape (an ES5-style constructor calling
24+
`Error.captureStackTrace`), so a genuine `CHECK constraint failed: …` surfaced as
25+
`First argument must be an Error object` instead.
26+
27+
The loader now snapshots and restores `Error.prepareStackTrace` (and `stackTraceLimit`)
28+
around the jiti call. Restoring is safe — Babel's installer self-neuters after its first
29+
call, so dropping the wrapper costs only cosmetic frame-hiding in later Babel diagnostics.
30+
31+
**How it stayed hidden, which is the more useful part:** the damage is cross-package, and
32+
every CI lane runs `bun test` **per package** (`scripts/ci-local.sh`, `conformance.yml`,
33+
`integration-tests.yml`), so `cli` and `migrate-ts` never shared a process. In a
34+
workspace-wide `bun test` — the run the contributor docs invite — four `migrate-ts`
35+
real-engine gates failed on their error-*message* assertions while the migrate engine was
36+
behaving correctly. Sibling tests using a bare, pattern-less `rejects.toThrow()` passed
37+
throughout, because the substituted `TypeError` still satisfies them.
38+
39+
No migration-correctness defect was masked: in every case the constraint fired and the
40+
apply → introspect → re-diff-EMPTY loop completed. Gated by a loader-seam regression test
41+
that pins both the hook identity and the end-to-end consequence.
42+
1043
## [0.21.3] — npm `0.21.3` · PyPI `0.21.3` · NuGet `0.21.3` · Maven `7.21.3`
1144

1245
Coordinated PATCH. **Changed product code: Maven and npm.** PyPI and NuGet are version-parity

server/typescript/packages/cli/src/lib/load-metaobjects-config.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,19 @@ export async function loadMetaobjectsConfig(projectRoot: string): Promise<Metaob
209209
alias: aliasMap,
210210
});
211211

212+
// Global-hooks hygiene. When Bun's native import of the config fails (e.g. a config
213+
// whose module body throws), jiti falls back to its bundled Babel transformer, whose
214+
// rewrite-stack-trace PERMANENTLY replaces `Error.prepareStackTrace` with a wrapper
215+
// delegating to whatever it captured. On Node that captured value is `undefined`, so
216+
// Babel's own lenient fallback runs and nothing is harmed. On Bun it is Bun's NATIVE
217+
// default, which throws `TypeError: First argument must be an Error object` for any
218+
// target that is not a real ErrorInstance — so once the wrapper leaks, every later
219+
// legacy-constructor error in the process (libsql's `SqliteError` is one:
220+
// an ES5-style constructor calling `Error.captureStackTrace`) throws that TypeError
221+
// *while being constructed*, replacing its real message. Snapshot and restore so
222+
// loading a config can never mutate the process's error hooks.
223+
const prepareStackTraceBefore = Error.prepareStackTrace;
224+
const stackTraceLimitBefore = Error.stackTraceLimit;
212225
try {
213226
const raw = (await jiti.import(loadPath)) as MetaobjectsGenConfig | { default: MetaobjectsGenConfig };
214227
// jiti's interopDefault doesn't always unwrap the default export when accessed
@@ -221,6 +234,14 @@ export async function loadMetaobjectsConfig(projectRoot: string): Promise<Metaob
221234
}
222235
return cfg;
223236
} finally {
237+
// Restoring is safe: Babel's installer self-neuters after the first call, so
238+
// dropping its wrapper only costs cosmetic frame-hiding in later Babel diagnostics.
239+
if (Error.prepareStackTrace !== prepareStackTraceBefore) {
240+
Error.prepareStackTrace = prepareStackTraceBefore;
241+
}
242+
if (Error.stackTraceLimit !== stackTraceLimitBefore) {
243+
Error.stackTraceLimit = stackTraceLimitBefore;
244+
}
224245
if (tempCreated) {
225246
try { unlinkSync(loadPath); } catch { /* best-effort cleanup */ }
226247
}
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// Global-state hygiene at the config-loader seam.
2+
//
3+
// Loading a `metaobjects.config.ts` must not mutate `Error.prepareStackTrace`.
4+
// Under Bun, when native import of the config fails, jiti falls back to its bundled
5+
// Babel transformer, whose rewrite-stack-trace permanently installs a
6+
// `prepareStackTrace` wrapper delegating to the value it captured. On Node that value
7+
// is `undefined` (harmless); on Bun it is Bun's NATIVE default, which throws
8+
// `TypeError: First argument must be an Error object` for anything that is not a real
9+
// ErrorInstance. Once leaked, every later legacy-constructor error in the process
10+
// throws that TypeError *while being constructed* — libsql's `SqliteError` is exactly
11+
// that shape, so a real "CHECK constraint failed" became the TypeError instead.
12+
//
13+
// The observable damage: in any workspace-wide `bun test`, four migrate-ts real-engine
14+
// gates failed on their error-MESSAGE assertions while the engine was working
15+
// correctly. Every CI lane runs `bun test` per package, so `cli` and `migrate-ts` never
16+
// shared a process and the leak was invisible.
17+
18+
import { describe, test, expect } from "bun:test";
19+
import { mkdtempSync, writeFileSync, rmSync } from "node:fs";
20+
import { tmpdir } from "node:os";
21+
import { join } from "node:path";
22+
import { loadMetaobjectsConfig } from "../../src/lib/load-metaobjects-config.js";
23+
24+
describe("loadMetaobjectsConfig — global hooks hygiene", () => {
25+
test("a throwing config surfaces its error AND leaves Error.prepareStackTrace untouched", async () => {
26+
const before = Error.prepareStackTrace;
27+
const dir = mkdtempSync(join(tmpdir(), "meta-config-hygiene-"));
28+
try {
29+
writeFileSync(join(dir, "metaobjects.config.ts"), "throw new Error('broken config boom');\n");
30+
// The failure must still be reported — the fix restores a hook, it does not swallow.
31+
await expect(loadMetaobjectsConfig(dir)).rejects.toThrow(/broken config boom/);
32+
expect(Error.prepareStackTrace).toBe(before);
33+
} finally {
34+
rmSync(dir, { recursive: true, force: true });
35+
}
36+
});
37+
38+
test("a legacy-constructor error still carries its own message after a failed config load", async () => {
39+
// The end-to-end consequence, independent of the hook identity above: this is the
40+
// exact shape of libsql's SqliteError (ES5 constructor + Error.captureStackTrace).
41+
const dir = mkdtempSync(join(tmpdir(), "meta-config-hygiene-legacy-"));
42+
try {
43+
writeFileSync(join(dir, "metaobjects.config.ts"), "throw new Error('broken config boom');\n");
44+
await expect(loadMetaobjectsConfig(dir)).rejects.toThrow(/broken config boom/);
45+
46+
function LegacyError(this: Record<string, unknown>, msg: string) {
47+
this["message"] = msg;
48+
Error.captureStackTrace(this, LegacyError);
49+
}
50+
LegacyError.prototype = Object.create(Error.prototype);
51+
52+
const constructed = new (LegacyError as unknown as new (m: string) => { message: string })(
53+
"CHECK constraint failed: things",
54+
);
55+
expect(constructed.message).toMatch(/CHECK constraint failed/);
56+
} finally {
57+
rmSync(dir, { recursive: true, force: true });
58+
}
59+
});
60+
});

server/typescript/packages/migrate-ts/test/integration/sqlite-uuid-pk-kitchen-sink.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ describe("uuid-PK kitchen sink — SQLite/libsql", () => {
221221
test("VALUE SEMANTICS: the enum CHECK actually rejects a value outside @values", async () => {
222222
await migrate(original());
223223
await insertTakingDefaults("ok", "IMAGE");
224-
expect(insertTakingDefaults("bad", "DOCUMENT")).rejects.toThrow();
224+
await expect(insertTakingDefaults("bad", "DOCUMENT")).rejects.toThrow(/CHECK/i);
225225
});
226226

227227
test("VALUE SEMANTICS: the partial UNIQUE index stays PARTIAL (its WHERE is not dropped)", async () => {
@@ -250,7 +250,7 @@ describe("uuid-PK kitchen sink — SQLite/libsql", () => {
250250
await migrate(original());
251251
await insertTakingDefaults("a", "IMAGE");
252252
// Before the change the CHECK must reject DOCUMENT.
253-
expect(insertTakingDefaults("pre", "DOCUMENT")).rejects.toThrow();
253+
await expect(insertTakingDefaults("pre", "DOCUMENT")).rejects.toThrow(/CHECK/i);
254254

255255
// Widen @values on MediaAsset.kind.
256256
const json = JSON.parse(original());
@@ -282,7 +282,7 @@ describe("uuid-PK kitchen sink — SQLite/libsql", () => {
282282
expect(Number(n["c"])).toBe(1);
283283

284284
// …a value still outside @values is STILL rejected (the CHECK was migrated, not dropped)…
285-
expect(insertTakingDefaults("nope", "SPREADSHEET")).rejects.toThrow();
285+
await expect(insertTakingDefaults("nope", "SPREADSHEET")).rejects.toThrow(/CHECK/i);
286286

287287
// …and the schema converged.
288288
await assertConverged(expected, allow);

0 commit comments

Comments
 (0)