Skip to content

test: pin #23139 + #22743 — repeated dynamic import of error-throwing module no longer hangs#30537

Merged
alii merged 2 commits into
mainfrom
claude/test-23139
May 12, 2026
Merged

test: pin #23139 + #22743 — repeated dynamic import of error-throwing module no longer hangs#30537
alii merged 2 commits into
mainfrom
claude/test-23139

Conversation

@alii

@alii alii commented May 12, 2026

Copy link
Copy Markdown
Member

A second await import() of a path whose first import threw used to hang forever instead of re-throwing. Two reported variants:

While verifying #30527 I checked whether oven-sh/WebKit#225 covered these — turns out both were already fixed before that bump (main + WebKit 88b2f7a2 doesn't hang either). The fix landed somewhere in the module-loader rewrite window (#29393#30262); the issues just never got re-tested.

build #23139 #22743
1.3.13 hangs hangs
main + WebKit 88b2f7a2 (pre-#30527)
main + WebKit 5488984d

Both tests fail on USE_SYSTEM_BUN=1 (stdout missing the post-hang lines, killed by spawn timeout) and pass on bun bd test.

Fixes #23139
Fixes #22743

…ger hangs

Reproduces on 1.3.13; fixed on main since the module-loader rewrite
window (#29393#30262, before #30527). Issue was never re-tested
after that landed.

Fixes #23139
@robobun

robobun commented May 12, 2026

Copy link
Copy Markdown
Collaborator
Updated 11:49 PM PT - May 11th, 2026

@alii, your commit 17a74b7 has 1 failures in Build #53606 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 30537

That installs a local version of the PR into your bun-30537 executable, so you can run:

bun-30537 --bun

@github-actions

Copy link
Copy Markdown
Contributor

Found 1 issue this PR may fix:

  1. Regression between Bun 1.2.20 and 1.2.21+ which causes Bun to hang instead of throwing error for certain dynamic import scenarios #22743 - Same root cause: re-importing a previously-errored dynamic import hangs instead of re-throwing, regressed in the same version range (1.2.21)

If this is helpful, copy the block below into the PR description to auto-close this issue on merge.

Fixes #22743

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: cc2ccd62-50b9-4e44-a140-357da8a979bd

📥 Commits

Reviewing files that changed from the base of the PR and between 93fe8a7 and 17a74b7.

📒 Files selected for processing (1)
  • test/regression/issue/22743.test.ts

Walkthrough

Adds two regression tests that spawn Bun subprocesses to exercise repeated dynamic imports of invalid JSON, asserting each failed import re-throws a SyntaxError, stdout shows the full error sequence plus done, stderr is empty, signalCode is null, and the subprocess exits with code 0.

Changes

Repeated dynamic-import regressions

Layer / File(s) Summary
Issue #23139 regression test
test/regression/issue/23139.test.ts
Spawns a Bun subprocess running an entry script that performs two consecutive await import("./bad.json") calls against an invalid JSON file; asserts both throw SyntaxError, stdout ends with done, and the subprocess exits with code 0 and no signal termination.
Issue #22743 regression test
test/regression/issue/22743.test.ts
Creates temp fixtures bad1.json and bad2.json and an entry.ts that imports them in X/Y/X order; spawns the script, asserts three SyntaxError re-throws and done in stdout, empty stderr, null signalCode, and zero exit code.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding regression tests for issues #23139 and #22743 where repeated dynamic imports of error-throwing modules no longer hang.
Description check ✅ Passed The description comprehensively explains what the PR does and includes verification through a matrix showing the fix exists on main. However, it lacks a dedicated 'How did you verify' section as specified in the template.
Linked Issues check ✅ Passed The PR adds regression tests for both #23139 and #22743 that verify the fix for the hang behavior, directly addressing the requirements in both linked issues.
Out of Scope Changes check ✅ Passed All changes are regression test files directly related to the linked issues. No unrelated or out-of-scope modifications are present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@alii alii changed the title test: pin #23139 — repeated dynamic import of unparseable file no longer hangs test: pin #23139 + #22743 — repeated dynamic import of error-throwing module no longer hangs May 12, 2026

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — test-only PR pinning two already-fixed module-loader regressions.

Extended reasoning...

Overview

This PR adds two new regression test files under test/regression/issue/ (#23139 and #22743). Both spawn a Bun subprocess that repeatedly await import()s unparseable JSON files and assert that each attempt re-throws SyntaxError rather than hanging. No production/runtime code is touched — the underlying fix already landed in the module-loader rewrite window; this PR just pins the behavior.

Security risks

None. The changes are confined to the test suite, write only to per-test temp directories via the standard tempDir harness helper, and execute self-contained fixture scripts with no network, no shell interpolation of external input, and no auth/crypto/permissions code.

Level of scrutiny

Low. Test-only additions following the repo's well-worn regression-test pattern (tempDir + Bun.spawn + bunExe()/bunEnv + stdout/exit-code assertions). Both tests bound the subprocess with a 10s spawn timeout and a 30s outer test timeout, so even if the regression resurfaces the suite fails fast with a clear diff rather than hanging CI.

Other factors

The robobun comment shows build-zig/build-cpp infrastructure failures on the first commit, but those are unrelated to these test-only files (no build scripts or native code touched). The bug-hunting system found no issues, CodeRabbit had no actionable comments, and there are no outstanding human review comments. The two tests are near-identical in structure and consistent with neighboring files in test/regression/issue/.

@alii alii merged commit 314ffe3 into main May 12, 2026
74 of 77 checks passed
@alii alii deleted the claude/test-23139 branch May 12, 2026 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants