Add missing module polling in watch mode #23618
Conversation
When a script tries to import a file that doesn't exist in watch mode, Bun now polls for the missing module instead of exiting with an error. This allows developers to create the file and have Bun automatically reload once it appears. ## Implementation - Created `MissingModulePollTimer.zig` that polls for missing files using exponential backoff (2ms to 100ms intervals) - Added timer integration to EventLoopTimer enum and fire handler - Integrated polling detection in `VirtualMachine.uncaughtException()` and `handlePendingInternalPromiseRejection()` - Detects MODULE_NOT_FOUND and ERR_MODULE_NOT_FOUND error codes - Triggers hot reload when the missing file appears ## Testing Added tests that verify the module polling works correctly when a missing module is created after the process starts. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
WalkthroughAdds watch-mode handling for missing modules by introducing a polling timer that checks for file appearance and triggers hot reload. Integrates the timer into the VM’s error paths and the event loop. Extends Timer API to expose and manage the new timer. Adds CLI watch tests validating the behavior. Changes
Suggested reviewers
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
src/bun.js/VirtualMachine.zig(3 hunks)src/bun.js/api/Timer.zig(2 hunks)src/bun.js/api/Timer/EventLoopTimer.zig(3 hunks)src/bun.js/api/Timer/MissingModulePollTimer.zig(1 hunks)test/cli/watch/watch-missing-external.test.ts(1 hunks)test/cli/watch/watch-missing-module-simple.test.ts(1 hunks)test/cli/watch/watch-missing-module.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.tstest/cli/watch/watch-missing-external.test.ts
test/cli/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/cli/**/*.{js,ts}: Place CLI command tests (e.g., bun install, bun init) under test/cli/
When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes
Files:
test/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.tstest/cli/watch/watch-missing-external.test.ts
test/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable
Files:
test/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.tstest/cli/watch/watch-missing-external.test.ts
test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.{ts,tsx}: Test files must live under test/ and end with .test.ts or .test.tsx
In tests, always use port: 0; do not hardcode ports or roll your own random port
Prefer normalizeBunSnapshot for snapshotting test output instead of asserting raw strings
Do not write tests that assert absence of crashes (e.g., 'no panic' or 'no uncaught exception')
Use Bun’s Jest-compatible runner (import { test, expect } from "bun:test") for tests
Avoid shell commands like find or grep in tests; use Bun’s Glob and built-in tools instead
Prefer running tests via bun bd test and use provided harness utilities (bunEnv, bunExe, tempDir)
Use Bun.spawn with proper stdio handling and await proc.exited in process-spawning tests
Files:
test/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.tstest/cli/watch/watch-missing-external.test.ts
test/cli/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place CLI command tests under test/cli/
Files:
test/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.tstest/cli/watch/watch-missing-external.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun:testfor files ending with*.test.{ts,js,jsx,tsx,mjs,cjs}
Prefer concurrent tests (test.concurrent/describe.concurrent) over sequential when feasible
Organize tests withdescribeblocks to group related tests
Use utilities likedescribe.each,toMatchSnapshot, and lifecycle hooks (beforeAll,beforeEach,afterEach) and track resources for cleanup
Files:
test/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.tstest/cli/watch/watch-missing-external.test.ts
test/**/*.{ts,tsx,js,jsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
For large/repetitive strings, use
Buffer.alloc(count, fill).toString()instead of"A".repeat(count)
Files:
test/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.tstest/cli/watch/watch-missing-external.test.ts
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
Files:
src/bun.js/api/Timer/EventLoopTimer.zigsrc/bun.js/api/Timer.zigsrc/bun.js/api/Timer/MissingModulePollTimer.zigsrc/bun.js/VirtualMachine.zig
src/{**/js_*.zig,bun.js/api/**/*.zig}
📄 CodeRabbit inference engine (.cursor/rules/registering-bun-modules.mdc)
Use bun.JSError!JSValue for proper error propagation in JS-exposed Zig functions
Files:
src/bun.js/api/Timer/EventLoopTimer.zigsrc/bun.js/api/Timer.zigsrc/bun.js/api/Timer/MissingModulePollTimer.zig
src/bun.js/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)
src/bun.js/**/*.zig: In Zig binding structs, expose generated bindings via pub const js = JSC.Codegen.JS and re-export toJS/fromJS/fromJSDirect
Constructors and prototype methods should return bun.JSError!JSC.JSValue to integrate Zig error handling with JS exceptions
Use parameter name globalObject (not ctx) and accept (*JSC.JSGlobalObject, *JSC.CallFrame) in binding methods/constructors
Implement getters as get(this, globalObject) returning JSC.JSValue and matching the .classes.ts interface
Provide deinit() for resource cleanup and finalize() that calls deinit(); use bun.destroy(this) or appropriate destroy pattern
Access JS call data via CallFrame (argument(i), argumentCount(), thisValue()) and throw errors with globalObject.throw(...)
For properties marked cache: true, use the generated Zig accessors (NameSetCached/GetCached) to work with GC-owned values
In finalize() for objects holding JS references, release them using .deref() before destroy
Files:
src/bun.js/api/Timer/EventLoopTimer.zigsrc/bun.js/api/Timer.zigsrc/bun.js/api/Timer/MissingModulePollTimer.zigsrc/bun.js/VirtualMachine.zig
src/**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, manage memory carefully and use defer for cleanup of allocations/resources
src/**/*.zig: Use the # prefix to declare private fields in Zig structs (e.g., struct { #foo: u32 })
Prefer decl literals when initializing values in Zig (e.g., const decl: Decl = .{ .binding = 0, .value = 0 })
Place @import directives at the bottom of Zig files
Use @import("bun") instead of @import("root").bunWhen adding debug logs in Zig, create a scoped logger and log via Bun APIs:
const log = bun.Output.scoped(.${SCOPE}, .hidden);thenlog("...", .{})
Files:
src/bun.js/api/Timer/EventLoopTimer.zigsrc/bun.js/api/Timer.zigsrc/bun.js/api/Timer/MissingModulePollTimer.zigsrc/bun.js/VirtualMachine.zig
🧠 Learnings (12)
📚 Learning: 2025-10-04T09:51:30.294Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.294Z
Learning: Applies to test/**/*.test.{ts,tsx} : Use Bun.spawn with proper stdio handling and await proc.exited in process-spawning tests
Applied to files:
test/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.tstest/cli/watch/watch-missing-external.test.ts
📚 Learning: 2025-10-12T02:22:34.349Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.349Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : Do not set explicit test timeouts; Bun already has timeouts
Applied to files:
test/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.tstest/cli/watch/watch-missing-external.test.ts
📚 Learning: 2025-10-12T02:22:34.349Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.349Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`
Applied to files:
test/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.tstest/cli/watch/watch-missing-external.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/cli/**/*.{js,ts} : When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes
Applied to files:
test/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.tstest/cli/watch/watch-missing-external.test.ts
📚 Learning: 2025-10-08T13:48:02.430Z
Learnt from: Jarred-Sumner
PR: oven-sh/bun#23373
File: test/js/bun/tarball/extract.test.ts:107-111
Timestamp: 2025-10-08T13:48:02.430Z
Learning: In Bun's test runner, use `expect(async () => { await ... }).toThrow()` to assert async rejections. Unlike Jest/Vitest, Bun does not require `await expect(...).rejects.toThrow()` - the async function wrapper with `.toThrow()` is the correct pattern for async error assertions in Bun tests.
Applied to files:
test/cli/watch/watch-missing-module-simple.test.ts
📚 Learning: 2025-10-04T09:51:30.294Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.294Z
Learning: Applies to test/**/*.test.{ts,tsx} : Prefer running tests via bun bd test <file> and use provided harness utilities (bunEnv, bunExe, tempDir)
Applied to files:
test/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.tstest/cli/watch/watch-missing-external.test.ts
📚 Learning: 2025-10-12T02:22:34.349Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.349Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : Always check exit codes and error scenarios in tests (e.g., spawned processes should assert non-zero on failure)
Applied to files:
test/cli/watch/watch-missing-module-simple.test.ts
📚 Learning: 2025-10-12T02:22:34.349Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.349Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js,test/**/*-fixture.ts} : Use `using`/`await using` for resource cleanup with Bun APIs (e.g., `Bun.spawn`, `Bun.listen`, `Bun.serve`)
Applied to files:
test/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.ts
📚 Learning: 2025-10-12T02:22:34.349Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.349Z
Learning: Applies to test/js/{bun,node}/** : Organize unit tests by module under `/test/js/bun/` and `/test/js/node/`
Applied to files:
test/cli/watch/watch-missing-module.test.ts
📚 Learning: 2025-10-12T02:22:34.349Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.349Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` for files ending with `*.test.{ts,js,jsx,tsx,mjs,cjs}`
Applied to files:
test/cli/watch/watch-missing-module.test.tstest/cli/watch/watch-missing-external.test.ts
📚 Learning: 2025-10-04T09:51:30.294Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.294Z
Learning: Applies to test/**/*.test.{ts,tsx} : Use Bun’s Jest-compatible runner (import { test, expect } from "bun:test") for tests
Applied to files:
test/cli/watch/watch-missing-module.test.ts
📚 Learning: 2025-10-12T02:22:34.349Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.349Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : Import common utilities from `harness` (e.g., `bunExe`, `bunEnv`, `tempDirWithFiles`, platform helpers, GC helpers)
Applied to files:
test/cli/watch/watch-missing-module.test.ts
🧬 Code graph analysis (3)
test/cli/watch/watch-missing-module-simple.test.ts (1)
test/harness.ts (2)
tempDir(277-284)bunExe(102-105)
test/cli/watch/watch-missing-module.test.ts (1)
test/harness.ts (2)
tempDir(277-284)bunExe(102-105)
test/cli/watch/watch-missing-external.test.ts (1)
test/harness.ts (2)
tempDir(277-284)bunExe(102-105)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Format
🔇 Additional comments (11)
src/bun.js/api/Timer.zig (2)
55-56: LGTM: Clean integration of MissingModulePollTimer.The field is properly initialized via
MissingModulePollTimer.init()and the comment clearly describes its purpose.
608-608: LGTM: Public import added.src/bun.js/api/Timer/EventLoopTimer.zig (1)
72-72: LGTM: EventLoopTimer integration follows existing patterns.The MissingModulePollTimer is correctly integrated into the timer system:
- Added to the Tag enum
- Type mapping references the correct type
- fire() handler properly retrieves the parent pointer and invokes onTimeout
Also applies to: 98-98, 193-196
src/bun.js/VirtualMachine.zig (2)
647-690: LGTM: Watch-mode missing module handler is well-guarded.The implementation correctly:
- Guards execution to watch mode only (line 650)
- Validates error structure and code (lines 652-668)
- Extracts the specifier safely (lines 671-676)
- Handles allocation failures gracefully with
catch return- Provides clear user feedback (line 688)
The UTF-8 conversion at lines 679-680 is immediately freed and then
startPollingdupes it again at line 683. While slightly inefficient, this is acceptable for error paths and keeps the ownership model clear.
714-716: LGTM: Integration points are correct.Calling
tryHandleMissingModuleInWatchModebefore the standard exception handling in bothuncaughtExceptionandhandlePendingInternalPromiseRejectionensures that watch mode can intercept module-not-found errors before they terminate the process.Also applies to: 732-736
test/cli/watch/watch-missing-external.test.ts (2)
23-24: Timing assumption may be fragile on slower CI systems.The 500ms wait assumes the process has started and hit the error. On slower CI systems or under load, this might not be sufficient.
Consider adding a more robust synchronization mechanism:
- Parse stderr for the "Waiting for module to exist:" message before writing the file
- Or increase the wait time to 1000ms for more margin
// Wait for the error message in stderr await new Promise(resolve => setTimeout(resolve, 1000));
34-44: LGTM: Good use of stdout assertions.Unlike the other tests, this one validates stdout contains the expected message, which provides stronger verification of the behavior.
src/bun.js/api/Timer/MissingModulePollTimer.zig (4)
39-59: LGTM: startPolling correctly manages state and memory.The function properly:
- Stops existing polling before starting new (lines 41-43)
- Frees old path and dupes new one (lines 46-49)
- Resets interval to minimum (line 52)
- Schedules first poll (line 56)
106-114: Exponential backoff is correctly implemented.The interval doubling with max cap ensures responsive polling for quickly-created files while avoiding excessive CPU usage for slow operations.
127-138: LGTM: deinit properly cleans up resources.The function removes the active timer and frees the allocated path, ensuring no leaks.
98-103: No changes needed:initEmpty(undefined)is correct.
Thereloadmethod’s Task parameter is unused (named_), so supplying an empty Task viaundefinedis intentional.
| const timeout = setTimeout(() => { | ||
| proc.kill(); | ||
| }, 5000); | ||
|
|
||
| try { | ||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
|
|
||
| clearTimeout(timeout); | ||
|
|
||
| console.log("STDOUT:", stdout); | ||
| console.log("STDERR:", stderr); | ||
| console.log("Exit code:", exitCode); | ||
|
|
||
| // The test passes if the process exits with code 0 | ||
| expect(exitCode).toBe(0); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider validating the fallback timeout behavior.
The test captures stdout/stderr but only logs them without assertions. While the current assertion on exitCode is sufficient for the happy path, consider:
- Adding assertions on stdout/stderr to validate that the error was caught and the polling message appeared.
- Testing the fallback case where the file is never created (though this would require a separate test with a shorter timeout).
Example:
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
clearTimeout(timeout);
- console.log("STDOUT:", stdout);
- console.log("STDERR:", stderr);
- console.log("Exit code:", exitCode);
+ // Validate that watch mode detected the error and started polling
+ expect(stderr).toContain("Waiting for module to exist:");
// The test passes if the process exits with code 0
expect(exitCode).toBe(0);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const timeout = setTimeout(() => { | |
| proc.kill(); | |
| }, 5000); | |
| try { | |
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | |
| clearTimeout(timeout); | |
| console.log("STDOUT:", stdout); | |
| console.log("STDERR:", stderr); | |
| console.log("Exit code:", exitCode); | |
| // The test passes if the process exits with code 0 | |
| expect(exitCode).toBe(0); | |
| const timeout = setTimeout(() => { | |
| proc.kill(); | |
| }, 5000); | |
| try { | |
| const [stdout, stderr, exitCode] = await Promise.all([ | |
| proc.stdout.text(), | |
| proc.stderr.text(), | |
| proc.exited, | |
| ]); | |
| clearTimeout(timeout); | |
| // Validate that watch mode detected the error and started polling | |
| expect(stderr).toContain("Waiting for module to exist:"); | |
| // The test passes if the process exits with code 0 | |
| expect(exitCode).toBe(0); |
🤖 Prompt for AI Agents
In test/cli/watch/watch-missing-module-simple.test.ts around lines 36 to 50, the
test only logs stdout/stderr and asserts exitCode; add assertions that stdout
and/or stderr contain the expected polling/fallback messages (e.g., the error
caught and the "waiting" or "polling" text) to validate normal fallback
behavior, and add a separate short-timeout test case that simulates the file
never being created to assert the fallback timeout path (assert that the polling
message repeats or that a specific timeout message appears) — implement these
assertions and the additional short timeout test to ensure both happy and
fallback paths are validated.
| test("watch mode should poll for missing modules", async () => { | ||
| using dir = tempDir("watch-missing-module", { | ||
| "entry.js": ` | ||
| try { | ||
| require("./missing.js"); | ||
| } catch (err) { | ||
| // Write the missing file | ||
| const fs = require("fs"); | ||
| const path = require("path"); | ||
| fs.writeFileSync(path.join(__dirname, "missing.js"), "process.exit(0);"); | ||
| // Re-throw so watch mode sees the error and starts polling | ||
| throw err; | ||
| } | ||
| // If we get here without the file existing, something went wrong | ||
| setTimeout(() => { | ||
| process.exit(1); | ||
| }, 5000); | ||
| `, | ||
| }); | ||
|
|
||
| const proc = Bun.spawn({ | ||
| cmd: [bunExe(), "--watch", "entry.js"], | ||
| cwd: String(dir), | ||
| env: bunEnv, | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
|
|
||
| const timeout = setTimeout(() => { | ||
| proc.kill(); | ||
| }, 10000); | ||
|
|
||
| try { | ||
| const exitCode = await proc.exited; | ||
| clearTimeout(timeout); | ||
|
|
||
| // The test passes if the process exits with code 0 | ||
| // This means the missing file was created and executed successfully | ||
| expect(exitCode).toBe(0); | ||
| } catch (err) { | ||
| clearTimeout(timeout); | ||
| proc.kill(); | ||
| throw err; | ||
| } | ||
| }, 15000); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider consolidating test structure or adding more assertions.
These tests cover both CommonJS and ES module scenarios, which is valuable. However:
- Similar to
watch-missing-module-simple.test.ts, consider adding assertions on stdout/stderr to validate the polling messages. - The two tests have very similar structure - consider using
test.eachto reduce duplication.
Example using test.each:
test.each([
{ name: "require", ext: "js", code: `require("./missing.js")` },
{ name: "import", ext: "mjs", code: `await import("./missing.mjs")` }
])("watch mode should poll for missing modules with $name", async ({ ext, code }) => {
// shared test logic
});Based on learnings
Also applies to: 52-99
WalkthroughAdds a MissingModulePollTimer and integrates it into the event loop. VirtualMachine invokes watch-mode missing-module handling on uncaught exceptions and pending promise rejections. Timer API gains a new timer field and public import. New timer implementation polls for file appearance and triggers reload. Tests cover watch-mode behavior for missing modules. Changes
Possibly related PRs
Suggested reviewers
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/bun.js/VirtualMachine.zig (2)
766-786: Stop poller on reload to avoid stale timersBefore reloading, stop any active missing-module polling to prevent leftover timers from firing post-reload.
pub fn reload(this: *VirtualMachine, _: *HotReloader.Task) void { + // Ensure missing-module polling is stopped before reload + this.timer.missing_module_poll_timer.stopPolling(this);
1983-1996: Deinit MissingModulePollTimer to free memoryCall deinit() during VM teardown to release duplicated path memory and cancel timers.
pub fn deinit(this: *VirtualMachine) void { this.auto_killer.deinit(); + // Cleanup missing-module poller + this.timer.missing_module_poll_timer.deinit(this); if (source_code_printer) |print| {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
src/bun.js/VirtualMachine.zig(3 hunks)src/bun.js/api/Timer.zig(2 hunks)src/bun.js/api/Timer/EventLoopTimer.zig(3 hunks)src/bun.js/api/Timer/MissingModulePollTimer.zig(1 hunks)test/cli/watch/watch-missing-external.test.ts(1 hunks)test/cli/watch/watch-missing-module-simple.test.ts(1 hunks)test/cli/watch/watch-missing-module.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/cli/watch/watch-missing-external.test.tstest/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.ts
test/cli/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/cli/**/*.{js,ts}: Place CLI command tests (e.g., bun install, bun init) under test/cli/
When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes
Files:
test/cli/watch/watch-missing-external.test.tstest/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.ts
test/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable
Files:
test/cli/watch/watch-missing-external.test.tstest/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.ts
test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.{ts,tsx}: Test files must live under test/ and end with .test.ts or .test.tsx
In tests, always use port: 0; do not hardcode ports or roll your own random port
Prefer normalizeBunSnapshot for snapshotting test output instead of asserting raw strings
Do not write tests that assert absence of crashes (e.g., 'no panic' or 'no uncaught exception')
Use Bun’s Jest-compatible runner (import { test, expect } from "bun:test") for tests
Avoid shell commands like find or grep in tests; use Bun’s Glob and built-in tools instead
Prefer running tests via bun bd test and use provided harness utilities (bunEnv, bunExe, tempDir)
Use Bun.spawn with proper stdio handling and await proc.exited in process-spawning tests
Files:
test/cli/watch/watch-missing-external.test.tstest/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.ts
test/cli/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place CLI command tests under test/cli/
Files:
test/cli/watch/watch-missing-external.test.tstest/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun:testfor files ending with*.test.{ts,js,jsx,tsx,mjs,cjs}
Prefer concurrent tests (test.concurrent/describe.concurrent) over sequential when feasible
Organize tests withdescribeblocks to group related tests
Use utilities likedescribe.each,toMatchSnapshot, and lifecycle hooks (beforeAll,beforeEach,afterEach) and track resources for cleanup
Files:
test/cli/watch/watch-missing-external.test.tstest/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.ts
test/**/*.{ts,tsx,js,jsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
For large/repetitive strings, use
Buffer.alloc(count, fill).toString()instead of"A".repeat(count)
Files:
test/cli/watch/watch-missing-external.test.tstest/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.ts
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
Files:
src/bun.js/api/Timer.zigsrc/bun.js/api/Timer/MissingModulePollTimer.zigsrc/bun.js/VirtualMachine.zigsrc/bun.js/api/Timer/EventLoopTimer.zig
src/{**/js_*.zig,bun.js/api/**/*.zig}
📄 CodeRabbit inference engine (.cursor/rules/registering-bun-modules.mdc)
Use bun.JSError!JSValue for proper error propagation in JS-exposed Zig functions
Files:
src/bun.js/api/Timer.zigsrc/bun.js/api/Timer/MissingModulePollTimer.zigsrc/bun.js/api/Timer/EventLoopTimer.zig
src/bun.js/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)
src/bun.js/**/*.zig: In Zig binding structs, expose generated bindings via pub const js = JSC.Codegen.JS and re-export toJS/fromJS/fromJSDirect
Constructors and prototype methods should return bun.JSError!JSC.JSValue to integrate Zig error handling with JS exceptions
Use parameter name globalObject (not ctx) and accept (*JSC.JSGlobalObject, *JSC.CallFrame) in binding methods/constructors
Implement getters as get(this, globalObject) returning JSC.JSValue and matching the .classes.ts interface
Provide deinit() for resource cleanup and finalize() that calls deinit(); use bun.destroy(this) or appropriate destroy pattern
Access JS call data via CallFrame (argument(i), argumentCount(), thisValue()) and throw errors with globalObject.throw(...)
For properties marked cache: true, use the generated Zig accessors (NameSetCached/GetCached) to work with GC-owned values
In finalize() for objects holding JS references, release them using .deref() before destroy
Files:
src/bun.js/api/Timer.zigsrc/bun.js/api/Timer/MissingModulePollTimer.zigsrc/bun.js/VirtualMachine.zigsrc/bun.js/api/Timer/EventLoopTimer.zig
src/**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, manage memory carefully and use defer for cleanup of allocations/resources
src/**/*.zig: Use the # prefix to declare private fields in Zig structs (e.g., struct { #foo: u32 })
Prefer decl literals when initializing values in Zig (e.g., const decl: Decl = .{ .binding = 0, .value = 0 })
Place @import directives at the bottom of Zig files
Use @import("bun") instead of @import("root").bunWhen adding debug logs in Zig, create a scoped logger and log via Bun APIs:
const log = bun.Output.scoped(.${SCOPE}, .hidden);thenlog("...", .{})
Files:
src/bun.js/api/Timer.zigsrc/bun.js/api/Timer/MissingModulePollTimer.zigsrc/bun.js/VirtualMachine.zigsrc/bun.js/api/Timer/EventLoopTimer.zig
🧠 Learnings (14)
📚 Learning: 2025-10-04T09:51:30.294Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.294Z
Learning: Applies to test/**/*.test.{ts,tsx} : Use Bun.spawn with proper stdio handling and await proc.exited in process-spawning tests
Applied to files:
test/cli/watch/watch-missing-external.test.tstest/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/cli/**/*.{js,ts} : When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes
Applied to files:
test/cli/watch/watch-missing-external.test.tstest/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.ts
📚 Learning: 2025-10-12T02:22:34.349Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.349Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` for files ending with `*.test.{ts,js,jsx,tsx,mjs,cjs}`
Applied to files:
test/cli/watch/watch-missing-external.test.tstest/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.ts
📚 Learning: 2025-10-12T02:22:34.349Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.349Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`
Applied to files:
test/cli/watch/watch-missing-external.test.tstest/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)
Applied to files:
test/cli/watch/watch-missing-external.test.tstest/cli/watch/watch-missing-module-simple.test.ts
📚 Learning: 2025-10-04T09:51:30.294Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.294Z
Learning: Applies to test/**/*.test.{ts,tsx} : Prefer running tests via bun bd test <file> and use provided harness utilities (bunEnv, bunExe, tempDir)
Applied to files:
test/cli/watch/watch-missing-external.test.tstest/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.ts
📚 Learning: 2025-10-12T02:22:34.349Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.349Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : Do not set explicit test timeouts; Bun already has timeouts
Applied to files:
test/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/**/*.{js,ts} : Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Applied to files:
test/cli/watch/watch-missing-module-simple.test.ts
📚 Learning: 2025-10-04T09:51:30.294Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.294Z
Learning: Applies to test/**/*.test.{ts,tsx} : Use Bun’s Jest-compatible runner (import { test, expect } from "bun:test") for tests
Applied to files:
test/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.ts
📚 Learning: 2025-10-12T02:22:34.349Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.349Z
Learning: Applies to test/js/{bun,node}/** : Organize unit tests by module under `/test/js/bun/` and `/test/js/node/`
Applied to files:
test/cli/watch/watch-missing-module-simple.test.ts
📚 Learning: 2025-10-08T13:48:02.430Z
Learnt from: Jarred-Sumner
PR: oven-sh/bun#23373
File: test/js/bun/tarball/extract.test.ts:107-111
Timestamp: 2025-10-08T13:48:02.430Z
Learning: In Bun's test runner, use `expect(async () => { await ... }).toThrow()` to assert async rejections. Unlike Jest/Vitest, Bun does not require `await expect(...).rejects.toThrow()` - the async function wrapper with `.toThrow()` is the correct pattern for async error assertions in Bun tests.
Applied to files:
test/cli/watch/watch-missing-module-simple.test.ts
📚 Learning: 2025-10-12T02:22:34.349Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.349Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : Always check exit codes and error scenarios in tests (e.g., spawned processes should assert non-zero on failure)
Applied to files:
test/cli/watch/watch-missing-module-simple.test.ts
📚 Learning: 2025-10-12T02:22:34.349Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.349Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js,test/**/*-fixture.ts} : Use `using`/`await using` for resource cleanup with Bun APIs (e.g., `Bun.spawn`, `Bun.listen`, `Bun.serve`)
Applied to files:
test/cli/watch/watch-missing-module-simple.test.tstest/cli/watch/watch-missing-module.test.ts
📚 Learning: 2025-10-12T02:22:34.349Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.349Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : Import common utilities from `harness` (e.g., `bunExe`, `bunEnv`, `tempDirWithFiles`, platform helpers, GC helpers)
Applied to files:
test/cli/watch/watch-missing-module.test.ts
🧬 Code graph analysis (3)
test/cli/watch/watch-missing-external.test.ts (1)
test/harness.ts (2)
tempDir(277-284)bunExe(102-105)
test/cli/watch/watch-missing-module-simple.test.ts (1)
test/harness.ts (2)
tempDir(277-284)bunExe(102-105)
test/cli/watch/watch-missing-module.test.ts (1)
test/harness.ts (2)
tempDir(277-284)bunExe(102-105)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Format
🔇 Additional comments (4)
src/bun.js/api/Timer.zig (1)
608-609: Public import looks goodImport placement and naming are consistent.
src/bun.js/api/Timer/EventLoopTimer.zig (1)
72-99: Timer tag and dispatch integration LGTMEnum, Type mapping, and fire() path correctly wire MissingModulePollTimer.
Also applies to: 193-197
src/bun.js/api/Timer/MissingModulePollTimer.zig (1)
117-124: Path resolution notecheckFileExists() uses cwd-relative stat. Ensure the VM passes absolute or cwd-relative paths only; otherwise nested module imports may never be detected. The guard suggested in VirtualMachine.tryHandleMissingModuleInWatchMode helps. Please verify nested require/import cases.
src/bun.js/VirtualMachine.zig (1)
647-691: Harden specifier handling before pollingStrip any “?…” suffix and only poll file-like specifiers. For example:
const slice = specifier.toUTF8(default_allocator); defer slice.deinit(); var spec = slice.slice(); // Strip query if (std.mem.indexOf(u8, spec, '?')) |i| spec = spec[0..i]; // Only poll absolute or relative paths if (!std.fs.path.isAbsolute(spec) and !(spec.len > 0 and (spec[0] == '.' or spec[0] == '/'))) return; // Start polling this.timer.missing_module_poll_timer.startPolling(this, spec) catch {};Verify that
missing_module_poll_timeris properly stopped inVirtualMachine.deinit(or on hot-reload).
| /// Polls for missing modules in watch mode | ||
| missing_module_poll_timer: MissingModulePollTimer = MissingModulePollTimer.init(), | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Add lifecycle cleanup for MissingModulePollTimer
This timer duplicates strings; without calling its deinit() on VM teardown, memory can leak. Ensure VM.deinit invokes vm.timer.missing_module_poll_timer.deinit(vm).
🏁 Script executed:
#!/bin/bash
set -e
rg -n "MissingModulePollTimer" -n src/bun.js/api/Timer.zig
rg -n "fn deinit" -n src/bun.js/api/Timer.zigLength of output: 175
🏁 Script executed:
#!/bin/bash
set -e
echo "=== Timer.zig (first 200 lines) ==="
sed -n '1,200p' src/bun.js/api/Timer.zig
echo "=== MissingModulePollTimer.zig (first 200 lines) ==="
sed -n '1,200p' src/bun.js/api/Timer/MissingModulePollTimer.zig
echo "=== Search for deinit in MissingModulePollTimer.zig ==="
rg -n "deinit" -n src/bun.js/api/Timer/MissingModulePollTimer.zig || trueLength of output: 11786
🏁 Script executed:
#!/bin/bash
set -e
# Find VirtualMachine.zig
fd --extension zig VirtualMachine.zig
# Display first 200 lines of its file(s)
for f in $(fd --extension zig VirtualMachine.zig); do
echo "=== $f ==="
sed -n '1,200p' "$f"
done
# Search for deinit or finalize
rg -n "fn deinit" -n $(fd --extension zig VirtualMachine.zig) || true
rg -n "fn finalize" -n $(fd --extension zig VirtualMachine.zig) || trueLength of output: 7515
🏁 Script executed:
#!/bin/bash
set -e
echo "=== VirtualMachine.deinit (lines 1980-2100) ==="
sed -n '1980,2100p' src/bun.js/VirtualMachine.zig
echo "=== References to missing_module_poll_timer in VirtualMachine.zig ==="
rg -n "missing_module_poll_timer" src/bun.js/VirtualMachine.zig || echo "no references"Length of output: 3986
Invoke MissingModulePollTimer.deinit in VM teardown
Add a call to this.timer.missing_module_poll_timer.deinit(this); inside VirtualMachine.deinit to free the duplicated path string and stop the active poll timer.
🤖 Prompt for AI Agents
In src/bun.js/api/Timer.zig around lines 55-57, the MissingModulePollTimer
created at missing_module_poll_timer: MissingModulePollTimer =
MissingModulePollTimer.init() allocates a duplicated path string and starts an
active poll timer that isn't being cleaned up; add a call to
this.timer.missing_module_poll_timer.deinit(this); inside the
VirtualMachine.deinit method so the duplicated path is freed and the poll timer
is stopped as part of VM teardown.
| /// Stop polling for the missing module | ||
| pub fn stopPolling(this: *MissingModulePollTimer, vm: *VirtualMachine) void { | ||
| if (!this.is_polling) return; | ||
|
|
||
| if (this.event_loop_timer.state == .ACTIVE) { | ||
| vm.timer.remove(&this.event_loop_timer); | ||
| } | ||
|
|
||
| this.is_polling = false; | ||
| this.current_interval_ms = this.min_interval_ms; | ||
|
|
||
| log("Stopped polling for missing module: {s}", .{this.missing_path}); | ||
| } | ||
|
|
There was a problem hiding this comment.
Free duplicated path on stop to avoid leaks
missing_path is duped in startPolling but never freed on stopPolling(). Free it and reset to empty.
pub fn stopPolling(this: *MissingModulePollTimer, vm: *VirtualMachine) void {
if (!this.is_polling) return;
if (this.event_loop_timer.state == .ACTIVE) {
vm.timer.remove(&this.event_loop_timer);
}
this.is_polling = false;
this.current_interval_ms = this.min_interval_ms;
- log("Stopped polling for missing module: {s}", .{this.missing_path});
+ log("Stopped polling for missing module: {s}", .{this.missing_path});
+ if (this.missing_path.len > 0) {
+ bun.default_allocator.free(this.missing_path);
+ this.missing_path = &.{};
+ }
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/bun.js/api/Timer/MissingModulePollTimer.zig around lines 61-74,
stopPolling currently never frees the duplicated missing_path from startPolling;
call the same allocator that was used to dup missing_path to free that buffer
(guarding against empty/null), then reset missing_path to an empty value (e.g.,
null/empty string) to avoid a dangling pointer and prevent future double-free;
perform the free before clearing is_polling and log after cleanup.
| /// Timer callback that checks if the missing file exists | ||
| pub fn onTimeout(this: *MissingModulePollTimer, vm: *VirtualMachine) jsc.API.Timer.EventLoopTimer.Arm { | ||
| this.event_loop_timer.state = .FIRED; | ||
|
|
||
| if (!this.is_polling) { | ||
| return .disarm; | ||
| } | ||
|
|
||
| // Check if the file exists | ||
| const file_exists = this.checkFileExists(); | ||
|
|
||
| if (file_exists) { | ||
| log("Missing module found: {s}. Triggering reload.", .{this.missing_path}); | ||
|
|
||
| // Stop polling | ||
| this.is_polling = false; | ||
|
|
||
| // Trigger a hot reload by calling reload directly | ||
| const HotReloader = jsc.hot_reloader.HotReloader; | ||
| var task = HotReloader.Task.initEmpty(undefined); | ||
| vm.reload(&task); | ||
|
|
||
| return .disarm; | ||
| } | ||
|
|
||
| // File still doesn't exist, increase interval with exponential backoff | ||
| this.current_interval_ms = @min(this.current_interval_ms * 2, this.max_interval_ms); | ||
|
|
||
| log("Missing module not found yet: {s}. Next poll in {}ms", .{ this.missing_path, this.current_interval_ms }); | ||
|
|
||
| // Schedule next poll | ||
| this.scheduleNextPoll(vm); | ||
|
|
||
| return .disarm; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Free duplicated path when file is found
When the module appears, release missing_path to prevent accumulation across reload cycles.
if (file_exists) {
log("Missing module found: {s}. Triggering reload.", .{this.missing_path});
// Stop polling
this.is_polling = false;
+ if (this.missing_path.len > 0) {
+ bun.default_allocator.free(this.missing_path);
+ this.missing_path = &.{};
+ }
// Trigger a hot reload by calling reload directly📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Timer callback that checks if the missing file exists | |
| pub fn onTimeout(this: *MissingModulePollTimer, vm: *VirtualMachine) jsc.API.Timer.EventLoopTimer.Arm { | |
| this.event_loop_timer.state = .FIRED; | |
| if (!this.is_polling) { | |
| return .disarm; | |
| } | |
| // Check if the file exists | |
| const file_exists = this.checkFileExists(); | |
| if (file_exists) { | |
| log("Missing module found: {s}. Triggering reload.", .{this.missing_path}); | |
| // Stop polling | |
| this.is_polling = false; | |
| // Trigger a hot reload by calling reload directly | |
| const HotReloader = jsc.hot_reloader.HotReloader; | |
| var task = HotReloader.Task.initEmpty(undefined); | |
| vm.reload(&task); | |
| return .disarm; | |
| } | |
| // File still doesn't exist, increase interval with exponential backoff | |
| this.current_interval_ms = @min(this.current_interval_ms * 2, this.max_interval_ms); | |
| log("Missing module not found yet: {s}. Next poll in {}ms", .{ this.missing_path, this.current_interval_ms }); | |
| // Schedule next poll | |
| this.scheduleNextPoll(vm); | |
| return .disarm; | |
| } | |
| /// Timer callback that checks if the missing file exists | |
| pub fn onTimeout(this: *MissingModulePollTimer, vm: *VirtualMachine) jsc.API.Timer.EventLoopTimer.Arm { | |
| this.event_loop_timer.state = .FIRED; | |
| if (!this.is_polling) { | |
| return .disarm; | |
| } | |
| // Check if the file exists | |
| const file_exists = this.checkFileExists(); | |
| if (file_exists) { | |
| log("Missing module found: {s}. Triggering reload.", .{this.missing_path}); | |
| // Stop polling | |
| this.is_polling = false; | |
| if (this.missing_path.len > 0) { | |
| bun.default_allocator.free(this.missing_path); | |
| this.missing_path = &.{}; | |
| } | |
| // Trigger a hot reload by calling reload directly | |
| const HotReloader = jsc.hot_reloader.HotReloader; | |
| var task = HotReloader.Task.initEmpty(undefined); | |
| vm.reload(&task); | |
| return .disarm; | |
| } | |
| // File still doesn't exist, increase interval with exponential backoff | |
| this.current_interval_ms = @min(this.current_interval_ms * 2, this.max_interval_ms); | |
| log("Missing module not found yet: {s}. Next poll in {}ms", .{ this.missing_path, this.current_interval_ms }); | |
| // Schedule next poll | |
| this.scheduleNextPoll(vm); | |
| return .disarm; | |
| } |
🤖 Prompt for AI Agents
In src/bun.js/api/Timer/MissingModulePollTimer.zig around lines 81 to 115, when
the missing file is found the duplicated path stored in this.missing_path is
never released; free the allocated memory for this.missing_path using the
appropriate allocator/free routine (e.g., call the timer/VM allocator's free
function) before or immediately after setting this.is_polling = false, then set
this.missing_path to null (or an empty/zero value) to avoid double-free on
subsequent reload cycles.
| console.log("STDOUT:", stdout); | ||
| console.log("STDERR:", stderr); | ||
| console.log("Exit code:", exitCode); | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove debug logging in tests
Drop console logs to reduce CI noise; keep assertions only.
🤖 Prompt for AI Agents
In test/cli/watch/watch-missing-external.test.ts around lines 38 to 41, remove
the three console.log debug statements (console.log("STDOUT:", stdout);
console.log("STDERR:", stderr); console.log("Exit code:", exitCode);) so the
test does not emit noisy output in CI; leave the existing assertions intact and
only assert against stdout/stderr/exitCode as needed.
| proc.kill(); | ||
| throw err; | ||
| } | ||
| }, 10000); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Avoid explicit test timeout
Let the runner manage timeouts; remove the 10000 override.
-}, 10000);
+});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| }, 10000); | |
| }); |
🤖 Prompt for AI Agents
In test/cli/watch/watch-missing-external.test.ts around line 50, the test has an
explicit timeout override "}, 10000);" which should be removed so the test
runner manages timeouts; delete the trailing numeric timeout argument from the
test closure (remove ", 10000") leaving the test definition without an explicit
timeout.
| console.log("STDOUT:", stdout); | ||
| console.log("STDERR:", stderr); | ||
| console.log("Exit code:", exitCode); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove debug logging in tests
These prints are unnecessary for assertions and bloat output.
🤖 Prompt for AI Agents
In test/cli/watch/watch-missing-module-simple.test.ts around lines 45 to 47, the
three console.log debug statements (printing STDOUT, STDERR, and Exit code)
should be removed because they are unnecessary for assertions and clutter test
output; delete these console.log calls and rely on existing assertions or add
focused assertions if visibility into outputs is needed.
| proc.kill(); | ||
| throw err; | ||
| } | ||
| }, 10000); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Avoid explicit test timeout
Rely on default runner timeout.
-}, 10000);
+});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| }, 10000); | |
| }); |
🤖 Prompt for AI Agents
In test/cli/watch/watch-missing-module-simple.test.ts around line 56 there is an
explicit test timeout passed (", 10000"); remove the explicit timeout argument
from the test call so the test relies on the test runner's default timeout
(i.e., delete the ", 10000" token and keep the test function invocation
unchanged otherwise) and ensure no other hard-coded timeouts remain in this
file.
| proc.kill(); | ||
| throw err; | ||
| } | ||
| }, 15000); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Avoid explicit test timeouts
Remove the 15000 overrides; rely on the runner defaults.
-}, 15000);
+});Also applies to: 99-99
🤖 Prompt for AI Agents
In test/cli/watch/watch-missing-module.test.ts around lines 50 and 99, the
explicit Jest/Mocha test timeout overrides (e.g., ", 15000") should be removed
so tests rely on the runner's default timeout; delete the numeric timeout
arguments on those test() or it() calls and leave the function signature with
only the test callback so no explicit timeout is specified.
1 participant
Summary
Fixes an issue where Bun's watch mode would exit with an error when trying to import a file that doesn't exist yet. Now, watch mode polls for missing modules and automatically reloads when they appear.
Changes
MissingModulePollTimer.zigimplements exponential backoff polling (2ms → 100ms)MODULE_NOT_FOUNDandERR_MODULE_NOT_FOUNDerrors in watch modeIntegration Points
VirtualMachine.uncaughtException()- Starts polling when module-not-found error occursVirtualMachine.handlePendingInternalPromiseRejection()- Same for promise rejectionsTest Plan
Added test that verifies:
Test passes:
bun bd test test/cli/watch/watch-missing-external.test.ts🤖 Generated with Claude Code