bun object: don't report a propagating sql module failure as uncaught #37198
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| import { describe, expect, test } from "bun:test"; | ||
| import { bunEnv, bunExe } from "harness"; | ||
|
|
||
| // Clobbering a global that an internal module needs makes that module fail to | ||
| // evaluate. The Bun.sql and Bun.SQL lazy getters used to report the failure | ||
| // while the exception was still pending on the VM, which aborted debug builds | ||
| // inside the uncaught exception handler (and in the process.emit path when | ||
| // process._fatalException had not been reified yet). The access must instead | ||
| // throw the evaluation error to the caller and leave the process healthy. | ||
| describe.concurrent("Bun object lazy getters", () => { | ||
| for (const reifyFatalException of [false, true]) { | ||
| test(`sql getter module failure propagates cleanly (fatalException reified: ${reifyFatalException})`, async () => { | ||
| await using proc = Bun.spawn({ | ||
| cmd: [ | ||
| bunExe(), | ||
| "-e", | ||
| ` | ||
| ${reifyFatalException ? "process._fatalException;" : ""} | ||
| globalThis.Object = undefined; | ||
| let caught = ""; | ||
| try { | ||
| Bun.sql; | ||
| } catch (e) { | ||
| caught = e.constructor.name; | ||
| } | ||
| try { | ||
| Bun.SQL; | ||
| } catch (e) {} | ||
| console.log("caught " + caught); | ||
| `, | ||
| ], | ||
| env: bunEnv, | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
| const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]); | ||
| expect(stdout.trim()).toBe("caught TypeError"); | ||
|
Check warning on line 37 in test/js/bun/bun-object/lazy-getter-module-failure.test.ts
|
||
|
Comment on lines
+33
to
+37
Contributor
There was a problem hiding this comment. 🟡 The test sets Extended reasoning...What the bug isThe subprocess test configures stderr: "pipe",
});
const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]);
expect(stdout.trim()).toBe("caught TypeError");
expect(exitCode).toBe(0);REVIEW.md's subprocess-test rule is explicit: " Code path that triggers itThe test spawns That message, plus the ASan/backtrace dump, goes to stderr. Because stderr is piped but never read, that output is discarded when the process is reaped. Why existing code doesn't prevent itNothing in the test consumes Impact
Step-by-step proof
Fixconst [stdout, stderr, exitCode] = await Promise.all([
proc.stdout.text(),
proc.stderr.text(),
proc.exited,
]);
expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({
stdout: "caught TypeError",
stderr: "",
exitCode: 0,
});Alternatively, drop |
||
| expect(exitCode).toBe(0); | ||
|
Comment on lines
+34
to
+38
Contributor
There was a problem hiding this comment. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
file=$(git ls-files 'test/js/bun/bun-object/lazy-getter-module-failure.test.ts' | head -n 1)
printf '%s\n' "FILE=$file"
if [ -n "$file" ]; then
wc -l "$file"
cat -n "$file"
fi
printf '%s\n' '--- relevant subprocess patterns ---'
rg -n -C 4 'lazy-getter-module-failure|stderr:\s*"pipe"|Promise\.all\(\[.*proc\.stdout|proc\.stderr\.text|proc\.exited' test/js/bun/bun-object -g '*.test.ts' || trueRepository: oven-sh/bun Length of output: 4074 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- review guidance files ---'
git ls-files | rg '(^|/)REVIEW\.md$|situational|landing' || true
printf '%s\n' '--- subprocess assertions in neighboring tests ---'
rg -n -C 5 'stderr:\s*"pipe"|proc\.stderr\.text\(\)|proc\.exited' test/js/bun/bun-object -g '*.test.ts' | head -n 240
printf '%s\n' '--- bunEnv definition and usage ---'
rg -n -C 8 'export\s+(const|let)\s+bunEnv|bunEnv\s*=' test packages scripts -g '*.ts' -g '*.js' 2>/dev/null | head -n 160 || true
printf '%s\n' '--- structural verifier for the target subprocess ---'
python3 - <<'PY'
from pathlib import Path
import re
path = Path("test/js/bun/bun-object/lazy-getter-module-failure.test.ts").resolve()
text = path.read_text()
spawn = re.search(r'Bun\.spawn\(\{(?P<body>.*?)\n\s*\}\)', text, re.S)
assert spawn, "target Bun.spawn call not found"
body = spawn.group("body")
assert 'stdout: "pipe"' in body
assert 'stderr: "pipe"' in body
awaits = re.search(r'const \[(?P<vars>[^\]]+)\]\s*=\s*await Promise\.all\(\[(?P<items>[^\]]+)\]\)', text)
assert awaits, "subprocess Promise.all not found"
vars_ = [v.strip() for v in awaits.group("vars").split(",")]
items = [i.strip() for i in awaits.group("items").split(",")]
print("absolute_test_path:", path)
print("configured_streams:", [x for x in ("stdout", "stderr") if f'{x}: "pipe"' in body])
print("awaited_values:", vars_)
print("awaited_expressions:", items)
print("stderr_is_consumed:", any("proc.stderr" in item for item in items))
print("stderr_is_asserted:", bool(re.search(r'expect\([^)]*stderr|expect\(\{[^}]*stderr', text)))
PYRepository: oven-sh/bun Length of output: 10585 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- neighboring combined assertion ---'
sed -n '220,250p' test/js/bun/bun-object/deep-equals.test.ts
printf '%s\n' '--- bunEnv quiet settings ---'
sed -n '60,82p' test/harness.ts
printf '%s\n' '--- standalone unread-pipe probe ---'
python3 - <<'PY'
import errno
import fcntl
import os
read_fd, write_fd = os.pipe()
try:
flags = fcntl.fcntl(write_fd, fcntl.F_GETFL)
fcntl.fcntl(write_fd, fcntl.F_SETFL, flags | os.O_NONBLOCK)
chunk = b"x" * 4096
total = 0
while True:
try:
total += os.write(write_fd, chunk)
except BlockingIOError as exc:
assert exc.errno == errno.EAGAIN
break
print("unread_pipe_write_bytes_before_block:", total)
print("unread_pipe_can_block_writer:", total > 0)
finally:
os.close(write_fd)
os.close(read_fd)
PYRepository: oven-sh/bun Length of output: 2342 Drain and assert the piped Read 🤖 Prompt for AI AgentsSources: Coding guidelines, Learnings |
||
| }); | ||
| } | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: oven-sh/bun
Length of output: 18427
🏁 Script executed:
Repository: oven-sh/bun
Length of output: 48091
🏁 Script executed:
Repository: oven-sh/bun
Length of output: 24117
Assert the
Bun.SQLfailure.The empty catch does not prove that
Bun.SQLthrows. Capture its error type, include it in the output, and update the expected output. Otherwise, this test can pass whenBun.SQLreturns normally.🤖 Prompt for AI Agents
Source: Coding guidelines