Repository navigation
feat(node:inspector): implement Profiler API - #25939
Merged
Jarred-Sumner merged 9 commits intoJan 16, 2026
Merged
Conversation
Contributor
WalkthroughAdds a hardcoded module Changes
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (17)📚 Learning: 2025-11-24T18:37:47.899ZApplied to files:
📚 Learning: 2025-11-24T18:36:59.706ZApplied to files:
📚 Learning: 2025-11-24T18:36:59.706ZApplied to files:
📚 Learning: 2025-11-24T18:36:59.706ZApplied to files:
📚 Learning: 2025-12-23T06:50:41.142ZApplied to files:
📚 Learning: 2025-11-24T18:36:59.706ZApplied to files:
📚 Learning: 2025-11-24T18:36:59.706ZApplied to files:
📚 Learning: 2025-11-24T18:37:11.466ZApplied to files:
📚 Learning: 2025-11-24T18:36:59.706ZApplied to files:
📚 Learning: 2025-11-24T18:36:59.706ZApplied to files:
📚 Learning: 2025-10-18T20:59:45.579ZApplied to files:
📚 Learning: 2025-11-03T20:40:59.655ZApplied to files:
📚 Learning: 2025-10-01T21:59:54.571ZApplied to files:
📚 Learning: 2025-10-19T03:01:29.084ZApplied to files:
📚 Learning: 2025-09-19T19:55:22.427ZApplied to files:
📚 Learning: 2025-09-05T19:48:44.481ZApplied to files:
📚 Learning: 2025-11-24T18:36:59.706ZApplied to files:
🔇 Additional comments (3)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/js/node/inspector.ts (1)
1-2: Update stale comment - Profiler API is now implemented.The comment "This is a stub! None of this is actually implemented yet." is outdated. The Profiler API methods (
enable,disable,start,stop,setSamplingInterval) are now functional.📝 Suggested update
// Hardcoded module "node:inspector" and "node:inspector/promises" -// This is a stub! None of this is actually implemented yet. +// Partial implementation: Profiler API is supported, other APIs (open, close, waitForDebugger) remain stubs.
🤖 Fix all issues with AI agents
In @src/bun.js/bindings/BunCPUProfiler.cpp:
- Around line 28-40: The static globals s_samplingInterval and
s_isProfilerRunning are unsynchronized; change their declarations to use
std::atomic<int> and std::atomic<bool> (add #include <atomic>), update
setSamplingInterval to store the value via
s_samplingInterval.store(intervalMicroseconds) and update any readers (e.g.,
isCPUProfilerRunning) to return s_isProfilerRunning.load(), and ensure any other
direct reads/writes in this file use atomic load/store (optionally with
memory_order_relaxed) to make profiler state access thread-safe.
In @src/bun.js/bindings/JSInspectorProfiler.cpp:
- Around line 47-57: The code converts intervalArg.asNumber() (a double)
directly to int, which can truncate huge values and accept Infinity; update the
validation in the JSValue intervalArg handling to first use std::isfinite(value)
(add #include <cmath>), obtain double value = intervalArg.asNumber(), then check
that value is finite, > 0, and <= static_cast<double>(INT_MAX) (or another safe
max) before doing static_cast<int>; if any check fails, call throwRangeError
with the same message and return {} so Infinity/NaN/too-large values are
rejected rather than silently truncated.
In @test/js/node/inspector/inspector-profiler.test.ts:
- Around line 192-196: The test's expectation that profile.startTime and
profile.endTime are 0 for empty profiles is wrong; update the assertions to
reflect that empty profiles record the real profiling start timestamp—assert
that profile.startTime equals profile.endTime and that profile.startTime is
greater than 0 (i.e., replace the two .toBe(0) checks with assertions that
profile.startTime === profile.endTime and profile.startTime > 0).
📜 Review details
Configuration used: Path: .coderabbit.yaml
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.
📥 Commits
Reviewing files that changed from the base of the PR and between 7704dca and fca8484f5eae636ef6cdc9268bd66d2642774847.
📒 Files selected for processing (10)
src/bun.js/HardcodedModule.zigsrc/bun.js/bindings/BunCPUProfiler.cppsrc/bun.js/bindings/BunCPUProfiler.hsrc/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/bindings/JSInspectorProfiler.hsrc/bun.js/modules/BunJSCModule.hsrc/js/node/inspector.promises.tssrc/js/node/inspector.tstest/js/bun/jsc/bun-jsc.test.tstest/js/node/inspector/inspector-profiler.test.ts
🧰 Additional context used
📓 Path-based instructions (8)
src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}
📄 CodeRabbit inference engine (src/js/CLAUDE.md)
src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}: Use.$call()and.$apply()instead of.call()and.apply()to prevent user tampering with function invocation
Use string literalrequire()statements only; dynamic requires are not permitted
Export modules usingexport default { ... }syntax; modules are NOT ES modules
Use JSC intrinsics (prefixed with$) such as$Array.from(),$isCallable(), and$newArrayWithSize()for performance-critical operations
Use private globals and methods with$prefix (e.g.,$Array,map.$set()) instead of public JavaScript globals
Use$debug()for debug logging and$assert()for assertions; both are stripped in release builds
Validate function arguments using validators frominternal/validatorsand throw$ERR_*error codes for invalid arguments
Useprocess.platformandprocess.archfor platform detection; these values are inlined and dead-code eliminated at build time
Files:
src/js/node/inspector.promises.tssrc/js/node/inspector.ts
src/js/{builtins,node,bun,thirdparty,internal}/**/*.ts
📄 CodeRabbit inference engine (src/js/CLAUDE.md)
Builtin functions must include
thisparameter typing in TypeScript to enable direct method binding in C++
Files:
src/js/node/inspector.promises.tssrc/js/node/inspector.ts
**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, be careful with allocators and use defer for cleanup
Files:
src/bun.js/HardcodedModule.zig
src/**/*.zig
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.zig: Use the#prefix for private fields in Zig structs, e.g.,struct { #foo: u32 };
Use Decl literals in Zig, e.g.,const decl: Decl = .{ .binding = 0, .value = 0 };
Place@importstatements at the bottom of the file in Zig (auto formatter will handle positioning)
Never use@import()inline inside functions in Zig; always place imports at the bottom of the file or containing struct
Files:
src/bun.js/HardcodedModule.zig
**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts?(x): Never usebun testdirectly - always usebun bd testto run tests with debug build changes
For single-file tests, prefer-eflag overtempDir
For multi-file tests, prefertempDirandBun.spawnover single-file tests
UsenormalizeBunSnapshotto normalize snapshot output of tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
UsetempDirfromharnessto create temporary directories - do not usetmpdirSyncorfs.mkdtempSync
When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Do not write flaky tests - do not usesetTimeoutin tests; instead await the condition to be met
Verify tests fail withUSE_SYSTEM_BUN=1 bun test <file>and pass withbun bd test <file>- tests are invalid if they pass with USE_SYSTEM_BUN=1
Test files must end with.test.tsor.test.tsx
Avoid shell commands likefindorgrepin tests - use Bun's Glob and built-in tools instead
Files:
test/js/bun/jsc/bun-jsc.test.tstest/js/node/inspector/inspector-profiler.test.ts
test/**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
Always use
port: 0in tests - do not hardcode ports or use custom random port number functions
Files:
test/js/bun/jsc/bun-jsc.test.tstest/js/node/inspector/inspector-profiler.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 bd test <...test file>to run tests with compiled code changes. Do not usebun testas it will not include your changes.
Usebun:testfor files ending in*.test.{ts,js,jsx,tsx,mjs,cjs}. For test files without .test extension in test/js/node/test/{parallel,sequential}/*.js, usebun bd <file>instead ofbun bd test <file>since they expect exit code 0.
Do not set a timeout on tests. Bun already has timeouts built-in.
Files:
test/js/bun/jsc/bun-jsc.test.tstest/js/node/inspector/inspector-profiler.test.ts
src/bun.js/bindings/**/*.cpp
📄 CodeRabbit inference engine (CLAUDE.md)
src/bun.js/bindings/**/*.cpp: Create classes in three parts in C++ when there is a public constructor: Foo (JSDestructibleObject), FooPrototype (JSNonFinalObject), and FooConstructor (InternalFunction)
Define properties using HashTableValue arrays in C++ JavaScript class bindings
Add iso subspaces for C++ classes with fields in JavaScript class bindings
Cache structures in ZigGlobalObject for JavaScript class bindings
Files:
src/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/bindings/BunCPUProfiler.cpp
🧠 Learnings (39)
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.dyn : Add symbol names with leading underscore and semicolons in braces to src/symbols.dyn for each new V8 API method
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.hsrc/bun.js/HardcodedModule.zigsrc/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/bindings/BunCPUProfiler.h
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : Add BUN_EXPORT visibility attribute to all public V8 API functions to ensure proper symbol export across platforms
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.hsrc/bun.js/HardcodedModule.zigsrc/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/modules/BunJSCModule.hsrc/bun.js/bindings/BunCPUProfiler.hsrc/bun.js/bindings/BunCPUProfiler.cpp
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Add iso subspaces for C++ classes with fields in JavaScript class bindings
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.hsrc/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/modules/BunJSCModule.hsrc/bun.js/bindings/BunCPUProfiler.h
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.txt : Add symbol names without leading underscore to src/symbols.txt for each new V8 API method
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.hsrc/bun.js/HardcodedModule.zigsrc/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/bindings/BunCPUProfiler.h
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : Create V8 class headers with .h extension following the pattern V8ClassName.h that include pragma once, v8.h, V8Local.h, V8Isolate.h, and declare classes extending from Data with BUN_EXPORT static methods
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.hsrc/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/modules/BunJSCModule.hsrc/bun.js/bindings/BunCPUProfiler.h
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Create classes in three parts in C++ when there is a public constructor: Foo (JSDestructibleObject), FooPrototype (JSNonFinalObject), and FooConstructor (InternalFunction)
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.hsrc/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/modules/BunJSCModule.h
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Create V8 class implementations with .cpp extension following the pattern V8ClassName.cpp that include the header, v8_compatibility_assertions.h, use ASSERT_V8_TYPE_LAYOUT_MATCHES macro, and implement methods using isolate->currentHandleScope()->createLocal<T>() for handle creation
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.hsrc/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-12-23T06:50:31.577Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 25429
File: src/bun.js/bindings/helpers.h:422-422
Timestamp: 2025-12-23T06:50:31.577Z
Learning: In Bun's C++ bindings, when returning an empty JSC::Identifier and a VM is accessible, prefer using vm.propertyNames->emptyIdentifier over constructing with JSC::Identifier(JSC::Identifier::EmptyIdentifierFlag::EmptyIdentifier). The cached identifier from the VM's property names table is more efficient and consistent with WebKit upgrade patterns. Apply this guidance to src/bun.js/bindings/helpers.h and similar header files in the same bindings directory (i.e., any file that constructs an EmptyIdentifier).
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.hsrc/bun.js/bindings/BunCPUProfiler.h
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Export modules using `export default { ... }` syntax; modules are NOT ES modules
Applied to files:
src/js/node/inspector.promises.tssrc/bun.js/HardcodedModule.zig
📚 Learning: 2025-10-19T04:55:33.099Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/http/node-telemetry.test.ts:27-203
Timestamp: 2025-10-19T04:55:33.099Z
Learning: In test/js/bun/http/node-telemetry.test.ts and the Bun.telemetry._node_binding API, after the architecture refactor, the _node_binding interface only contains two methods: handleIncomingRequest(req, res) and handleWriteHead(res, statusCode). The handleRequestFinish hook and other lifecycle hooks were removed during simplification. Both current methods are fully tested.
Applied to files:
src/js/node/inspector.tstest/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/napi/napi.zig : For each new V8 C++ method, add both GCC/Clang and MSVC mangled symbol names to the V8API struct in src/napi/napi.zig using extern fn declarations
Applied to files:
src/bun.js/HardcodedModule.zig
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Write JS builtins for Bun's Node.js compatibility and APIs, and run `bun bd` after changes
Applied to files:
src/bun.js/HardcodedModule.zigsrc/bun.js/modules/BunJSCModule.h
📚 Learning: 2025-11-24T18:37:47.899Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/AGENTS.md:0-0
Timestamp: 2025-11-24T18:37:47.899Z
Learning: Applies to src/bun.js/bindings/v8/**/<UNKNOWN> : <UNKNOWN>
Applied to files:
src/bun.js/HardcodedModule.zigsrc/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/modules/BunJSCModule.hsrc/bun.js/bindings/BunCPUProfiler.h
📚 Learning: 2025-08-30T09:09:18.384Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22231
File: src/bundler/bundle_v2.zig:48-48
Timestamp: 2025-08-30T09:09:18.384Z
Learning: In Zig, when a module exports a top-level struct, import("./Module.zig") directly returns that struct type and can be used as a type alias without needing to access a field within the module. This is a common pattern in the Bun codebase.
Applied to files:
src/bun.js/HardcodedModule.zig
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8-module/main.cpp : Register new V8 API test functions in the Init method using NODE_SET_METHOD with exports object
Applied to files:
src/bun.js/HardcodedModule.zigsrc/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output
Applied to files:
src/bun.js/HardcodedModule.zigtest/js/bun/jsc/bun-jsc.test.tstest/js/node/inspector/inspector-profiler.test.tssrc/bun.js/modules/BunJSCModule.h
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Organize code into `builtins/` for individual functions, `node/` for Node.js modules, `bun/` for Bun modules, `thirdparty/` for NPM replacements, and `internal/` for internal modules
Applied to files:
src/bun.js/HardcodedModule.zig
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.
Applied to files:
src/bun.js/HardcodedModule.zigtest/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2026-01-05T16:32:07.551Z
Learnt from: alii
Repo: oven-sh/bun PR: 25474
File: src/bun.js/event_loop/Sigusr1Handler.zig:0-0
Timestamp: 2026-01-05T16:32:07.551Z
Learning: In Zig codebases (e.g., Bun), treat std.posix.sigaction as returning void and do not perform runtime error handling for its failure. The Zig standard library views sigaction failures as programmer errors (unreachable) because they only occur with invalid signals like SIGKILL/SIGSTOP. Apply this pattern across Zig files that call sigaction (e.g., crash_handler.zig, main.zig, filter_run.zig, process.zig) and ensure failures are not handled as recoverable errors; prefer reaching an explicit unreachable/compile-time assumption when such failures are detected.
Applied to files:
src/bun.js/HardcodedModule.zig
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : For multi-file tests, prefer `tempDir` and `Bun.spawn` over single-file tests
Applied to files:
test/js/bun/jsc/bun-jsc.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun bd test <...test file>` to run tests with compiled code changes. Do not use `bun test` as it will not include your changes.
Applied to files:
test/js/bun/jsc/bun-jsc.test.tssrc/bun.js/modules/BunJSCModule.h
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Never use `bun test` directly - always use `bun bd test` to run tests with debug build changes
Applied to files:
test/js/bun/jsc/bun-jsc.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8-module/main.cpp : Create test functions in test/v8/v8-module/main.cpp that take FunctionCallbackInfo<Value> parameter, use the test V8 API, print results for comparison with Node.js, and return Undefined
Applied to files:
test/js/bun/jsc/bun-jsc.test.tssrc/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Prefer async/await over callbacks in tests. When callbacks must be used for a single callback, use `Promise.withResolvers` to create a promise that can be resolved or rejected from a callback.
Applied to files:
test/js/bun/jsc/bun-jsc.test.tstest/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Do not write flaky tests - do not use `setTimeout` in tests; instead await the condition to be met
Applied to files:
test/js/bun/jsc/bun-jsc.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Use `describe.each()` for parameterized tests, `toMatchSnapshot()` for snapshot testing, and lifecycle hooks like `beforeAll()`, `afterEach()`, `beforeEach()` for setup/teardown.
Applied to files:
test/js/bun/jsc/bun-jsc.test.ts
📚 Learning: 2025-09-03T01:30:58.001Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: test/js/valkey/valkey.test.ts:264-271
Timestamp: 2025-09-03T01:30:58.001Z
Learning: For test/js/valkey/valkey.test.ts PUB/SUB tests, avoid arbitrary sleeps and async-forEach. Instead, resolve a Promise from the subscriber callback when the expected number of messages is observed and await it with a bounded timeout (e.g., withTimeout + Promise.withResolvers) to account for Redis server→subscriber propagation.
Applied to files:
test/js/bun/jsc/bun-jsc.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Ensure V8 API tests compare identical C++ code output between Node.js and Bun through the test suite validation process
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use `$debug()` for debug logging and `$assert()` for assertions; both are stripped in release builds
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-10-08T13:48:02.430Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 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/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-10-18T05:23:24.403Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/telemetry-server.test.ts:91-100
Timestamp: 2025-10-18T05:23:24.403Z
Learning: In the Bun codebase, telemetry tests (test/js/bun/telemetry-*.test.ts) should focus on telemetry API behavior: configure/disable/isEnabled, callback signatures and invocation, request ID correlation, and error handling. HTTP protocol behaviors like status code normalization (e.g., 200 with empty body → 204) should be tested in HTTP server tests (test/js/bun/http/), not in telemetry tests. Keep separation of concerns: telemetry tests verify the telemetry API contract; HTTP tests verify HTTP semantics.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-09-20T03:39:41.770Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 22534
File: test/regression/issue/21830.fixture.ts:14-63
Timestamp: 2025-09-20T03:39:41.770Z
Learning: Bun's test runner supports async describe callbacks, unlike Jest/Vitest where describe callbacks must be synchronous. The syntax `describe("name", async () => { ... })` is valid in Bun.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-11-06T00:58:23.965Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24417
File: test/js/bun/spawn/spawn.test.ts:903-918
Timestamp: 2025-11-06T00:58:23.965Z
Learning: In Bun test files, `await using` with spawn() is appropriate for long-running processes that need guaranteed cleanup on scope exit or when explicitly testing disposal behavior. For short-lived processes that exit naturally (e.g., console.log scripts), the pattern `const proc = spawn(...); await proc.exited;` is standard and more common, as evidenced by 24 instances vs 4 `await using` instances in test/js/bun/spawn/spawn.test.ts.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-10-25T17:20:19.041Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 24063
File: test/js/bun/telemetry/server-header-injection.test.ts:5-20
Timestamp: 2025-10-25T17:20:19.041Z
Learning: In the Bun telemetry codebase, tests are organized into two distinct layers: (1) Internal API tests in test/js/bun/telemetry/ use numeric InstrumentKind enum values to test Zig↔JS injection points and low-level integration; (2) Public API tests in packages/bun-otel/test/ use string InstrumentKind values ("http", "fetch", etc.) to test the public-facing BunSDK and instrumentation APIs. This separation allows internal tests to use efficient numeric enums for refactoring flexibility while the public API maintains a developer-friendly string-based interface.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-12-23T06:50:41.142Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 25429
File: src/bun.js/bindings/helpers.h:422-422
Timestamp: 2025-12-23T06:50:41.142Z
Learning: In Bun's C++ bindings (src/bun.js/bindings/helpers.h and similar files), when returning an empty JSC::Identifier and a VM is accessible, prefer using `vm.propertyNames->emptyIdentifier` over constructing with `JSC::Identifier(JSC::Identifier::EmptyIdentifierFlag::EmptyIdentifier)`. The cached identifier from the VM's property names table is more efficient and consistent with WebKit upgrade patterns.
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/modules/BunJSCModule.hsrc/bun.js/bindings/BunCPUProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Use V8_UNIMPLEMENTED() macro for functions not yet implemented in V8 compatibility classes
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Use localToJSValue() to convert V8 handles to JSC values and perform JSC operations within V8 method implementations
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Cache structures in ZigGlobalObject for JavaScript class bindings
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Use JSC::WriteBarrier for heap-allocated references in V8 objects and implement visitChildren() for custom heap objects to support garbage collection
Applied to files:
src/bun.js/modules/BunJSCModule.h
🧬 Code graph analysis (5)
src/js/node/inspector.promises.ts (1)
src/js/node/inspector.ts (1)
method(92-135)
src/js/node/inspector.ts (1)
src/bun.js/bindings/BunCPUProfiler.cpp (4)
startCPUProfiler(42-57)startCPUProfiler(42-42)isCPUProfilerRunning(37-40)isCPUProfilerRunning(37-37)
test/js/bun/jsc/bun-jsc.test.ts (2)
test/harness.ts (1)
isBuildKite(38-38)scripts/utils.mjs (1)
profile(1680-1680)
src/bun.js/bindings/JSInspectorProfiler.cpp (1)
src/bun.js/bindings/BunCPUProfiler.h (1)
JSC(6-9)
src/bun.js/bindings/BunCPUProfiler.h (1)
src/bun.js/bindings/BunCPUProfiler.cpp (6)
setSamplingInterval(32-35)setSamplingInterval(32-32)isCPUProfilerRunning(37-40)isCPUProfilerRunning(37-37)startCPUProfiler(42-57)startCPUProfiler(42-42)
🪛 Cppcheck (2.19.0)
src/bun.js/bindings/JSInspectorProfiler.cpp
[information] 42-42: Include file
(missingInclude)
[information] 52-52: Include file
(missingIncludeSystem)
[information] 59-59: Include file
(missingIncludeSystem)
[error] 66-66: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
🔇 Additional comments (19)
test/js/bun/jsc/bun-jsc.test.ts (1)
187-215: LGTM! Well-structured regression test.The test effectively validates that the profiler can be reused multiple times after the shutdown-to-pause fix. The Fibonacci workload generates sufficient CPU activity to produce meaningful stack traces, and the assertions verify the fix works correctly across multiple sequential invocations.
src/bun.js/modules/BunJSCModule.h (2)
673-680: LGTM! Correct fix for profiler reusability.The change from
shutdown()topause()+clearData()with explicit locking is the right approach. This ensures the profiler can be restarted in the same process while maintaining thread safety.
692-696: LGTM! Consistent error handling.The failure path correctly mirrors the success path by using
pause()with proper locking, ensuring the profiler remains reusable even after errors.src/bun.js/bindings/JSInspectorProfiler.h (1)
1-9: LGTM! Clean header declarations.The header follows JSC conventions correctly with appropriate includes and clear function declarations for the CPU profiler API surface.
src/bun.js/bindings/BunCPUProfiler.h (2)
13-14: LGTM! Useful runtime configuration APIs.The new functions provide necessary runtime control for sampling interval configuration and profiler state queries, supporting the inspector API requirements.
20-20: LGTM! Improved documentation clarity.The updated comment more precisely describes the return value semantics, distinguishing between "never started" and other failure modes.
src/js/node/inspector.promises.ts (1)
1-28: LGTM! Clean promise-based wrapper.The implementation correctly wraps the callback-based inspector Session with a promise-based API. The error handling and export structure follow the expected patterns for Node.js compatibility.
src/bun.js/HardcodedModule.zig (1)
68-68: LGTM! Module registration follows established patterns.The
node:inspector/promisesmodule is correctly registered following the same pattern as other/promisessubpath modules (e.g.,dns/promises,fs/promises,timers/promises):
- Enum member added
- Map entry added
- Common alias via
nodeEntry()for both prefixed and unprefixed resolution- Node-specific alias for the unprefixed
inspector/promisespathAlso applies to: 125-125, 235-235, 397-398
src/bun.js/bindings/BunCPUProfiler.cpp (2)
84-94: Good fix for profiler restart capability.Using
pause()instead ofshutdown()while holding the profiler lock correctly enables the profiler to be restarted within the same process. TheclearData()call afterreleaseStackTraces()ensures clean state for subsequent profiling sessions.
351-360: LGTM!The extern C wrapper functions follow the established pattern and correctly delegate to the internal Bun namespace functions.
src/js/node/inspector.ts (4)
6-10: LGTM!Native profiler bindings are correctly imported with matching function names and argument counts from JSInspectorProfiler.cpp.
30-50: LGTM!Session lifecycle management is correct:
connect()prevents double-connectiondisconnect()is idempotent and properly stops any running profiler- Private state fields use the
#prefix as expected
52-90: Implementation looks correct, verify Node.js compatibility.The
post()method correctly handles the overloaded signature(method, params?, callback?)and(method, callback?). Callback invocation viaqueueMicrotaskensures async delivery.Note: In Node.js, the callback-based
Session.post()is always asynchronous. The current implementation returns synchronously when no callback is provided, which differs from Node.js behavior wherepost()without callback returnsundefinedand results are only delivered via events. However, this synchronous return is actually more convenient for the promise wrapper and may be intentional. Consider documenting this deviation if intentional.
92-135: LGTM!The
#handleMethodimplementation correctly handles:
- Profiler lifecycle with proper state tracking
- Idempotent
Profiler.start(no-op if already running)- JSON parsing with error handling for
Profiler.stop- Validation for
setSamplingInterval(positive number, not while running)- Stub responses for coverage APIs (acceptable for initial implementation)
test/js/node/inspector/inspector-profiler.test.ts (3)
1-19: LGTM! Good test setup with proper isolation.The test file correctly imports from
bun:testand usesbeforeEach/afterEachfor session lifecycle management. ThePromise.withResolverspattern for callback testing follows best practices.
69-298: Comprehensive Profiler test coverage.Excellent test coverage including:
- Lifecycle transitions (enable → start → stop → disable)
- Error conditions (start without enable, stop without start)
- Profile structure validation (CDP format compliance)
- Restart capability (verifies the
pause()fix)- Cleanup on disconnect
setSamplingIntervalvalidation
376-429: LGTM! Promise API tests are well-structured.The
node:inspector/promisestests correctly verify:
- Promise-based
post()returns a Promise- Rejections on error conditions
- Complete profiling workflow with async/await
- Shared exports between callback and promise modules
src/bun.js/bindings/JSInspectorProfiler.cpp (2)
21-34: LGTM! Correct BunString memory management.The
stopCPUProfilerfunction properly:
- Handles empty/dead BunString by returning empty JS string
- Converts to WTF::String before dereference (ensuring the string data is copied)
- Correctly calls
deref()to release the BunString
14-19: LGTM!Simple, clean host function implementations that correctly wrap the native profiler functions.
Also applies to: 63-67
Implement the Chrome DevTools Protocol Profiler domain for node:inspector: - Profiler.enable/disable - Enable or disable the profiler - Profiler.start/stop - Start and stop CPU profiling - Profiler.setSamplingInterval - Configure sampling interval in microseconds Returns CPU profiles in Chrome DevTools JSON format, compatible with VS Code, Chrome DevTools, and speedscope. Also fixes bun:jsc profile() to use pause() instead of shutdown(), allowing the profiler to be restarted within the same process.
vadim-anthropic
force-pushed
the
claude/node-inspector-profiler
branch
from
January 10, 2026 00:50
1e50ff0 to
aba7424
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
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.
📥 Commits
Reviewing files that changed from the base of the PR and between fca8484f5eae636ef6cdc9268bd66d2642774847 and 1e50ff08cd20c52358a0ef21765a0470bb385d01.
📒 Files selected for processing (1)
src/bun.js/bindings/JSInspectorProfiler.cpp
🧰 Additional context used
📓 Path-based instructions (1)
src/bun.js/bindings/**/*.cpp
📄 CodeRabbit inference engine (CLAUDE.md)
src/bun.js/bindings/**/*.cpp: Create classes in three parts in C++ when there is a public constructor: Foo (JSDestructibleObject), FooPrototype (JSNonFinalObject), and FooConstructor (InternalFunction)
Define properties using HashTableValue arrays in C++ JavaScript class bindings
Add iso subspaces for C++ classes with fields in JavaScript class bindings
Cache structures in ZigGlobalObject for JavaScript class bindings
Files:
src/bun.js/bindings/JSInspectorProfiler.cpp
🧠 Learnings (18)
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : Add BUN_EXPORT visibility attribute to all public V8 API functions to ensure proper symbol export across platforms
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Add iso subspaces for C++ classes with fields in JavaScript class bindings
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.dyn : Add symbol names with leading underscore and semicolons in braces to src/symbols.dyn for each new V8 API method
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Create V8 class implementations with .cpp extension following the pattern V8ClassName.cpp that include the header, v8_compatibility_assertions.h, use ASSERT_V8_TYPE_LAYOUT_MATCHES macro, and implement methods using isolate->currentHandleScope()->createLocal<T>() for handle creation
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Create classes in three parts in C++ when there is a public constructor: Foo (JSDestructibleObject), FooPrototype (JSNonFinalObject), and FooConstructor (InternalFunction)
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.txt : Add symbol names without leading underscore to src/symbols.txt for each new V8 API method
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:37:47.899Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/AGENTS.md:0-0
Timestamp: 2025-11-24T18:37:47.899Z
Learning: Applies to src/bun.js/bindings/v8/**/<UNKNOWN> : <UNKNOWN>
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-12-23T06:50:41.142Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 25429
File: src/bun.js/bindings/helpers.h:422-422
Timestamp: 2025-12-23T06:50:41.142Z
Learning: In Bun's C++ bindings (src/bun.js/bindings/helpers.h and similar files), when returning an empty JSC::Identifier and a VM is accessible, prefer using `vm.propertyNames->emptyIdentifier` over constructing with `JSC::Identifier(JSC::Identifier::EmptyIdentifierFlag::EmptyIdentifier)`. The cached identifier from the VM's property names table is more efficient and consistent with WebKit upgrade patterns.
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8-module/main.cpp : Create test functions in test/v8/v8-module/main.cpp that take FunctionCallbackInfo<Value> parameter, use the test V8 API, print results for comparison with Node.js, and return Undefined
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/napi/napi.zig : For each new V8 C++ method, add both GCC/Clang and MSVC mangled symbol names to the V8API struct in src/napi/napi.zig using extern fn declarations
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Cache structures in ZigGlobalObject for JavaScript class bindings
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-09-20T05:35:57.318Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 22534
File: src/bun.js/bindings/headers.h:729-731
Timestamp: 2025-09-20T05:35:57.318Z
Learning: symbols.txt in the Bun codebase is specifically for V8 API mangled symbols (without leading underscore), not for general Bun host functions declared with BUN_DECLARE_HOST_FUNCTION. Host functions are handled through different build mechanisms.
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-10-01T21:59:54.571Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h:47-74
Timestamp: 2025-10-01T21:59:54.571Z
Learning: In the new bindings generator (bindgenv2) for `src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h`, the context-aware enumeration conversion overloads intentionally use stricter validation (requiring `value.isString()` without ToString coercion), diverging from Web IDL semantics. This is a design decision documented in comments.
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-10-18T20:59:45.579Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:458-475
Timestamp: 2025-10-18T20:59:45.579Z
Learning: In src/bun.js/telemetry.zig, the RequestId (u64) to JavaScript number (f64) conversion in jsRequestId() is intentionally allowed to lose precision beyond 2^53-1. This is acceptable because: (1) at 1M requests/sec it takes ~285 years to overflow, (2) the counter resets per-process, and (3) these are observability IDs, not critical distributed IDs. Precision loss is an acceptable trade-off for this use case.
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8-module/main.cpp : Register new V8 API test functions in the Init method using NODE_SET_METHOD with exports object
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Use V8_UNIMPLEMENTED() macro for functions not yet implemented in V8 compatibility classes
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : Create V8 class headers with .h extension following the pattern V8ClassName.h that include pragma once, v8.h, V8Local.h, V8Isolate.h, and declare classes extending from Data with BUN_EXPORT static methods
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Use localToJSValue() to convert V8 handles to JSC values and perform JSC operations within V8 method implementations
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
🧬 Code graph analysis (1)
src/bun.js/bindings/JSInspectorProfiler.cpp (1)
src/bun.js/bindings/BunCPUProfiler.cpp (12)
Bun__startCPUProfiler(19-19)Bun__startCPUProfiler(341-344)Bun__startCPUProfiler(341-341)Bun__stopCPUProfilerAndGetJSON(20-20)Bun__stopCPUProfilerAndGetJSON(346-350)Bun__stopCPUProfilerAndGetJSON(346-346)Bun__setCPUSamplingInterval(21-21)Bun__setCPUSamplingInterval(352-355)Bun__setCPUSamplingInterval(352-352)Bun__isCPUProfilerRunning(22-22)Bun__isCPUProfilerRunning(357-360)Bun__isCPUProfilerRunning(357-357)
🪛 Cppcheck (2.19.0)
src/bun.js/bindings/JSInspectorProfiler.cpp
[information] 42-42: Include file
(missingInclude)
[information] 52-52: Include file
(missingIncludeSystem)
[information] 59-59: Include file
(missingIncludeSystem)
[error] 66-66: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
🔇 Additional comments (4)
src/bun.js/bindings/JSInspectorProfiler.cpp (4)
9-12: LGTM: Extern declarations match the implementations.The extern "C" declarations correctly match the implementations in BunCPUProfiler.cpp as shown in the relevant code snippets.
14-19: LGTM: Simple and correct implementation.The function correctly wraps the native profiler start call and returns undefined, matching the expected inspector API behavior.
21-34: LGTM: Proper string handling and memory management.The function correctly handles the BunString result, including empty/dead string cases, and properly calls
result.deref()for cleanup.
63-67: LGTM: Simple and correct implementation.The function correctly wraps the native profiler status check and returns a boolean value.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/js/node/inspector.ts (1)
1-2: Outdated comment: file is no longer a stub.Line 2 states "This is a stub! None of this is actually implemented yet." but this PR adds a functional implementation. Consider updating or removing this comment.
Suggested fix
// Hardcoded module "node:inspector" and "node:inspector/promises" -// This is a stub! None of this is actually implemented yet. +// Partial implementation of node:inspector - supports Profiler API const { hideFromStack, throwNotImplemented } = require("internal/shared");
🤖 Fix all issues with AI agents
In @src/bun.js/bindings/BunCPUProfiler.cpp:
- Around line 28-30: s_samplingInterval and s_isProfilerRunning are plain
statics accessed concurrently from setSamplingInterval, startCPUProfiler,
stopCPUProfilerAndGetJSON, and isCPUProfilerRunning: convert them to atomic
types (e.g., std::atomic<int> s_samplingInterval and std::atomic<bool>
s_isProfilerRunning) and replace all direct reads/writes with .load()/.store()
(or appropriate atomic ops) in setSamplingInterval, startCPUProfiler,
stopCPUProfilerAndGetJSON, and isCPUProfilerRunning to eliminate the race
without changing logic.
- Around line 70-95: Move the global profiler state update into the locked
region inside stopCPUProfilerAndGetJSON: don’t set s_isProfilerRunning = false
before acquiring the profiler lock; instead, acquire the JSLockHolder and
profiler lock (profiler->getLock()/WTF::Locker profilerLocker) first, then set
s_isProfilerRunning = false immediately before calling profiler->pause() so
readers of isCPUProfilerRunning() won’t observe the profiler as stopped while
sampling may still be active.
In @src/js/node/inspector.promises.ts:
- Around line 7-18: Session.post is missing an explicit this-parameter type;
update the method signature to include the this parameter (e.g., post(this:
Session, method: string, params?: object): Promise<any>) so TypeScript knows the
correct receiver type when calling BaseSession.post and to satisfy builtin
typing rules; keep the existing body intact and only change the method
declaration to add the this: Session parameter, referencing Session.post and
BaseSession.post.
In @src/js/node/inspector.ts:
- Around line 126-130: The coverage-related inspector handlers
(Profiler.getBestEffortCoverage, Profiler.startPreciseCoverage,
Profiler.stopPreciseCoverage, Profiler.takePreciseCoverage) currently
unconditionally return {}; instead, check the profiler enabled state (the
#profilerEnabled flag) and either return an appropriate error/result when
profiler is not enabled (mimic Node’s behavior by responding with an error
indicating Profiler.enable must be called) or keep the no-op but explicitly
document that choice; update the handlers in the inspector dispatch so they
inspect #profilerEnabled before returning and produce the correct error or
documented no-op response.
- Around line 52-90: The Session instance methods (including post) lack explicit
this-parameter typing which prevents correct binding; update the method
signatures to include a first parameter like `this: Session` (e.g., change
`post(method: string, ...` to `post(this: Session, method: string, ...)`) and
apply the same change to other Session methods that are intended to be bound
(refer to Session methods such as post and any callers using direct binding or
passing method references), ensuring internal references like `this.#connected`
and `this.#handleMethod` remain valid after the signature change.
- Around line 110-116: The Profiler.stop handler can hit a TOCTOU where
isCPUProfilerRunning() was true but stopCPUProfiler() returns an empty string;
modify the "Profiler.stop" case to call stopCPUProfiler(), check the returned
value explicitly (e.g., if (!result) return new Error("Profiler is not started.
Call Profiler.start first.")), and only then call JSON.parse(result) inside the
try/catch so empty results are handled deterministically; reference the existing
isCPUProfilerRunning(), stopCPUProfiler(), and the "Profiler.stop" case when
applying the change.
📜 Review details
Configuration used: Path: .coderabbit.yaml
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.
📥 Commits
Reviewing files that changed from the base of the PR and between 1e50ff08cd20c52358a0ef21765a0470bb385d01 and aba7424.
📒 Files selected for processing (10)
src/bun.js/HardcodedModule.zigsrc/bun.js/bindings/BunCPUProfiler.cppsrc/bun.js/bindings/BunCPUProfiler.hsrc/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/bindings/JSInspectorProfiler.hsrc/bun.js/modules/BunJSCModule.hsrc/js/node/inspector.promises.tssrc/js/node/inspector.tstest/js/bun/jsc/bun-jsc.test.tstest/js/node/inspector/inspector-profiler.test.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts?(x): Never usebun testdirectly - always usebun bd testto run tests with debug build changes
For single-file tests, prefer-eflag overtempDir
For multi-file tests, prefertempDirandBun.spawnover single-file tests
UsenormalizeBunSnapshotto normalize snapshot output of tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
UsetempDirfromharnessto create temporary directories - do not usetmpdirSyncorfs.mkdtempSync
When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Do not write flaky tests - do not usesetTimeoutin tests; instead await the condition to be met
Verify tests fail withUSE_SYSTEM_BUN=1 bun test <file>and pass withbun bd test <file>- tests are invalid if they pass with USE_SYSTEM_BUN=1
Test files must end with.test.tsor.test.tsx
Avoid shell commands likefindorgrepin tests - use Bun's Glob and built-in tools instead
Files:
test/js/node/inspector/inspector-profiler.test.tstest/js/bun/jsc/bun-jsc.test.ts
test/**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
Always use
port: 0in tests - do not hardcode ports or use custom random port number functions
Files:
test/js/node/inspector/inspector-profiler.test.tstest/js/bun/jsc/bun-jsc.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 bd test <...test file>to run tests with compiled code changes. Do not usebun testas it will not include your changes.
Usebun:testfor files ending in*.test.{ts,js,jsx,tsx,mjs,cjs}. For test files without .test extension in test/js/node/test/{parallel,sequential}/*.js, usebun bd <file>instead ofbun bd test <file>since they expect exit code 0.
Do not set a timeout on tests. Bun already has timeouts built-in.
Files:
test/js/node/inspector/inspector-profiler.test.tstest/js/bun/jsc/bun-jsc.test.ts
src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}
📄 CodeRabbit inference engine (src/js/CLAUDE.md)
src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}: Use.$call()and.$apply()instead of.call()and.apply()to prevent user tampering with function invocation
Use string literalrequire()statements only; dynamic requires are not permitted
Export modules usingexport default { ... }syntax; modules are NOT ES modules
Use JSC intrinsics (prefixed with$) such as$Array.from(),$isCallable(), and$newArrayWithSize()for performance-critical operations
Use private globals and methods with$prefix (e.g.,$Array,map.$set()) instead of public JavaScript globals
Use$debug()for debug logging and$assert()for assertions; both are stripped in release builds
Validate function arguments using validators frominternal/validatorsand throw$ERR_*error codes for invalid arguments
Useprocess.platformandprocess.archfor platform detection; these values are inlined and dead-code eliminated at build time
Files:
src/js/node/inspector.tssrc/js/node/inspector.promises.ts
src/js/{builtins,node,bun,thirdparty,internal}/**/*.ts
📄 CodeRabbit inference engine (src/js/CLAUDE.md)
Builtin functions must include
thisparameter typing in TypeScript to enable direct method binding in C++
Files:
src/js/node/inspector.tssrc/js/node/inspector.promises.ts
src/bun.js/bindings/**/*.cpp
📄 CodeRabbit inference engine (CLAUDE.md)
src/bun.js/bindings/**/*.cpp: Create classes in three parts in C++ when there is a public constructor: Foo (JSDestructibleObject), FooPrototype (JSNonFinalObject), and FooConstructor (InternalFunction)
Define properties using HashTableValue arrays in C++ JavaScript class bindings
Add iso subspaces for C++ classes with fields in JavaScript class bindings
Cache structures in ZigGlobalObject for JavaScript class bindings
Files:
src/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/bindings/BunCPUProfiler.cpp
**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, be careful with allocators and use defer for cleanup
Files:
src/bun.js/HardcodedModule.zig
src/**/*.zig
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.zig: Use the#prefix for private fields in Zig structs, e.g.,struct { #foo: u32 };
Use Decl literals in Zig, e.g.,const decl: Decl = .{ .binding = 0, .value = 0 };
Place@importstatements at the bottom of the file in Zig (auto formatter will handle positioning)
Never use@import()inline inside functions in Zig; always place imports at the bottom of the file or containing struct
Files:
src/bun.js/HardcodedModule.zig
🧠 Learnings (58)
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output
Applied to files:
test/js/node/inspector/inspector-profiler.test.tstest/js/bun/jsc/bun-jsc.test.tssrc/bun.js/modules/BunJSCModule.hsrc/bun.js/HardcodedModule.zig
📚 Learning: 2025-10-19T04:55:33.099Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/http/node-telemetry.test.ts:27-203
Timestamp: 2025-10-19T04:55:33.099Z
Learning: In test/js/bun/http/node-telemetry.test.ts and the Bun.telemetry._node_binding API, after the architecture refactor, the _node_binding interface only contains two methods: handleIncomingRequest(req, res) and handleWriteHead(res, statusCode). The handleRequestFinish hook and other lifecycle hooks were removed during simplification. Both current methods are fully tested.
Applied to files:
test/js/node/inspector/inspector-profiler.test.tssrc/js/node/inspector.tstest/js/bun/jsc/bun-jsc.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Ensure V8 API tests compare identical C++ code output between Node.js and Bun through the test suite validation process
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-09-03T01:30:58.001Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: test/js/valkey/valkey.test.ts:264-271
Timestamp: 2025-09-03T01:30:58.001Z
Learning: For test/js/valkey/valkey.test.ts PUB/SUB tests, avoid arbitrary sleeps and async-forEach. Instead, resolve a Promise from the subscriber callback when the expected number of messages is observed and await it with a bounded timeout (e.g., withTimeout + Promise.withResolvers) to account for Redis server→subscriber propagation.
Applied to files:
test/js/node/inspector/inspector-profiler.test.tstest/js/bun/jsc/bun-jsc.test.ts
📚 Learning: 2025-10-18T05:23:24.403Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/telemetry-server.test.ts:91-100
Timestamp: 2025-10-18T05:23:24.403Z
Learning: In the Bun codebase, telemetry tests (test/js/bun/telemetry-*.test.ts) should focus on telemetry API behavior: configure/disable/isEnabled, callback signatures and invocation, request ID correlation, and error handling. HTTP protocol behaviors like status code normalization (e.g., 200 with empty body → 204) should be tested in HTTP server tests (test/js/bun/http/), not in telemetry tests. Keep separation of concerns: telemetry tests verify the telemetry API contract; HTTP tests verify HTTP semantics.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use `$debug()` for debug logging and `$assert()` for assertions; both are stripped in release builds
Applied to files:
test/js/node/inspector/inspector-profiler.test.tstest/js/bun/jsc/bun-jsc.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Do not set a timeout on tests. Bun already has timeouts built-in.
Applied to files:
test/js/node/inspector/inspector-profiler.test.tstest/js/bun/jsc/bun-jsc.test.ts
📚 Learning: 2025-10-08T13:48:02.430Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 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/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Prefer async/await over callbacks in tests. When callbacks must be used for a single callback, use `Promise.withResolvers` to create a promise that can be resolved or rejected from a callback.
Applied to files:
test/js/node/inspector/inspector-profiler.test.tstest/js/bun/jsc/bun-jsc.test.ts
📚 Learning: 2025-09-20T03:39:41.770Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 22534
File: test/regression/issue/21830.fixture.ts:14-63
Timestamp: 2025-09-20T03:39:41.770Z
Learning: Bun's test runner supports async describe callbacks, unlike Jest/Vitest where describe callbacks must be synchronous. The syntax `describe("name", async () => { ... })` is valid in Bun.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.
Applied to files:
test/js/node/inspector/inspector-profiler.test.tssrc/bun.js/HardcodedModule.zig
📚 Learning: 2025-11-06T00:58:23.965Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24417
File: test/js/bun/spawn/spawn.test.ts:903-918
Timestamp: 2025-11-06T00:58:23.965Z
Learning: In Bun test files, `await using` with spawn() is appropriate for long-running processes that need guaranteed cleanup on scope exit or when explicitly testing disposal behavior. For short-lived processes that exit naturally (e.g., console.log scripts), the pattern `const proc = spawn(...); await proc.exited;` is standard and more common, as evidenced by 24 instances vs 4 `await using` instances in test/js/bun/spawn/spawn.test.ts.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-10-25T17:20:19.041Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 24063
File: test/js/bun/telemetry/server-header-injection.test.ts:5-20
Timestamp: 2025-10-25T17:20:19.041Z
Learning: In the Bun telemetry codebase, tests are organized into two distinct layers: (1) Internal API tests in test/js/bun/telemetry/ use numeric InstrumentKind enum values to test Zig↔JS injection points and low-level integration; (2) Public API tests in packages/bun-otel/test/ use string InstrumentKind values ("http", "fetch", etc.) to test the public-facing BunSDK and instrumentation APIs. This separation allows internal tests to use efficient numeric enums for refactoring flexibility while the public API maintains a developer-friendly string-based interface.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : Add BUN_EXPORT visibility attribute to all public V8 API functions to ensure proper symbol export across platforms
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.hsrc/bun.js/bindings/BunCPUProfiler.hsrc/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/bindings/BunCPUProfiler.cppsrc/bun.js/modules/BunJSCModule.hsrc/bun.js/HardcodedModule.zig
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : Create V8 class headers with .h extension following the pattern V8ClassName.h that include pragma once, v8.h, V8Local.h, V8Isolate.h, and declare classes extending from Data with BUN_EXPORT static methods
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.hsrc/bun.js/bindings/BunCPUProfiler.hsrc/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/bindings/BunCPUProfiler.cppsrc/bun.js/modules/BunJSCModule.h
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Add iso subspaces for C++ classes with fields in JavaScript class bindings
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.hsrc/bun.js/bindings/BunCPUProfiler.hsrc/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/bindings/BunCPUProfiler.cppsrc/bun.js/modules/BunJSCModule.h
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.dyn : Add symbol names with leading underscore and semicolons in braces to src/symbols.dyn for each new V8 API method
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.hsrc/bun.js/bindings/BunCPUProfiler.hsrc/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/bindings/BunCPUProfiler.cppsrc/bun.js/HardcodedModule.zig
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Create V8 class implementations with .cpp extension following the pattern V8ClassName.cpp that include the header, v8_compatibility_assertions.h, use ASSERT_V8_TYPE_LAYOUT_MATCHES macro, and implement methods using isolate->currentHandleScope()->createLocal<T>() for handle creation
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.hsrc/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/bindings/BunCPUProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.txt : Add symbol names without leading underscore to src/symbols.txt for each new V8 API method
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.hsrc/bun.js/bindings/BunCPUProfiler.hsrc/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/bindings/BunCPUProfiler.cppsrc/bun.js/HardcodedModule.zig
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Create classes in three parts in C++ when there is a public constructor: Foo (JSDestructibleObject), FooPrototype (JSNonFinalObject), and FooConstructor (InternalFunction)
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.hsrc/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/modules/BunJSCModule.h
📚 Learning: 2025-12-23T06:50:31.577Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 25429
File: src/bun.js/bindings/helpers.h:422-422
Timestamp: 2025-12-23T06:50:31.577Z
Learning: In Bun's C++ bindings, when returning an empty JSC::Identifier and a VM is accessible, prefer using vm.propertyNames->emptyIdentifier over constructing with JSC::Identifier(JSC::Identifier::EmptyIdentifierFlag::EmptyIdentifier). The cached identifier from the VM's property names table is more efficient and consistent with WebKit upgrade patterns. Apply this guidance to src/bun.js/bindings/helpers.h and similar header files in the same bindings directory (i.e., any file that constructs an EmptyIdentifier).
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.hsrc/bun.js/bindings/BunCPUProfiler.h
📚 Learning: 2025-11-24T18:37:47.899Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/AGENTS.md:0-0
Timestamp: 2025-11-24T18:37:47.899Z
Learning: Applies to src/bun.js/bindings/v8/**/<UNKNOWN> : <UNKNOWN>
Applied to files:
src/bun.js/bindings/BunCPUProfiler.hsrc/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/bindings/BunCPUProfiler.cppsrc/bun.js/modules/BunJSCModule.hsrc/bun.js/HardcodedModule.zig
📚 Learning: 2025-12-23T06:50:41.142Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 25429
File: src/bun.js/bindings/helpers.h:422-422
Timestamp: 2025-12-23T06:50:41.142Z
Learning: In Bun's C++ bindings (src/bun.js/bindings/helpers.h and similar files), when returning an empty JSC::Identifier and a VM is accessible, prefer using `vm.propertyNames->emptyIdentifier` over constructing with `JSC::Identifier(JSC::Identifier::EmptyIdentifierFlag::EmptyIdentifier)`. The cached identifier from the VM's property names table is more efficient and consistent with WebKit upgrade patterns.
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/bindings/BunCPUProfiler.cppsrc/bun.js/modules/BunJSCModule.h
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8-module/main.cpp : Create test functions in test/v8/v8-module/main.cpp that take FunctionCallbackInfo<Value> parameter, use the test V8 API, print results for comparison with Node.js, and return Undefined
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8-module/main.cpp : Register new V8 API test functions in the Init method using NODE_SET_METHOD with exports object
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Cache structures in ZigGlobalObject for JavaScript class bindings
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/modules/BunJSCModule.h
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/napi/napi.zig : For each new V8 C++ method, add both GCC/Clang and MSVC mangled symbol names to the V8API struct in src/napi/napi.zig using extern fn declarations
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/bindings/BunCPUProfiler.cppsrc/bun.js/HardcodedModule.zig
📚 Learning: 2025-09-20T05:35:57.318Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 22534
File: src/bun.js/bindings/headers.h:729-731
Timestamp: 2025-09-20T05:35:57.318Z
Learning: symbols.txt in the Bun codebase is specifically for V8 API mangled symbols (without leading underscore), not for general Bun host functions declared with BUN_DECLARE_HOST_FUNCTION. Host functions are handled through different build mechanisms.
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-10-01T21:59:54.571Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h:47-74
Timestamp: 2025-10-01T21:59:54.571Z
Learning: In the new bindings generator (bindgenv2) for `src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h`, the context-aware enumeration conversion overloads intentionally use stricter validation (requiring `value.isString()` without ToString coercion), diverging from Web IDL semantics. This is a design decision documented in comments.
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-10-18T20:59:45.579Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:458-475
Timestamp: 2025-10-18T20:59:45.579Z
Learning: In src/bun.js/telemetry.zig, the RequestId (u64) to JavaScript number (f64) conversion in jsRequestId() is intentionally allowed to lose precision beyond 2^53-1. This is acceptable because: (1) at 1M requests/sec it takes ~285 years to overflow, (2) the counter resets per-process, and (3) these are observability IDs, not critical distributed IDs. Precision loss is an acceptable trade-off for this use case.
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-10T00:57:09.173Z
Learnt from: franciscop
Repo: oven-sh/bun PR: 24514
File: src/bun.js/api/crypto/PasswordObject.zig:86-101
Timestamp: 2025-11-10T00:57:09.173Z
Learning: In Bun's Zig codebase (PasswordObject.zig), when validating the parallelism parameter for Argon2, the upper limit is set to 65535 (2^16 - 1) rather than using `std.math.maxInt(u24)` because the latter triggers Zig's truncation limit checks. The value 65535 is a practical upper bound that avoids compiler issues while being sufficient for thread parallelism use cases.
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Use V8_UNIMPLEMENTED() macro for functions not yet implemented in V8 compatibility classes
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Use localToJSValue() to convert V8 handles to JSC values and perform JSC operations within V8 method implementations
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-10-19T03:01:29.084Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:286-289
Timestamp: 2025-10-19T03:01:29.084Z
Learning: In src/bun.js/telemetry.zig, the guard preventing double configuration (lines 213-216) is intentional. The telemetry API uses a single-shot configuration model where configure() is called once during application startup. Users must call Bun.telemetry.configure(null) to reset before reconfiguring. This design ensures: (1) predictable state—callbacks don't change mid-request, avoiding race conditions; (2) zero overhead when disabled—no checking for callback changes on every request; (3) clear ownership—one adapter (e.g., bun-otel) owns the telemetry config. Merge-style reconfiguration would require atomic updates during active requests, adding overhead to the hot path.
<!-- [/add_learning]
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cppsrc/bun.js/modules/BunJSCModule.h
📚 Learning: 2025-10-15T22:03:50.832Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/envvars.zig:135-144
Timestamp: 2025-10-15T22:03:50.832Z
Learning: In src/envvars.zig, the boolean feature flag cache uses a single atomic enum and should remain monotonic. Only the string cache (which uses two atomics: ptr and len) requires acquire/release ordering to prevent torn reads.
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cpp
📚 Learning: 2025-09-02T17:14:46.924Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22227
File: src/safety/alloc.zig:93-95
Timestamp: 2025-09-02T17:14:46.924Z
Learning: In bun's Zig codebase, they use a custom extension of Zig that supports private field syntax with the `#` prefix (e.g., `#allocator`, `#trace`). This is not standard Zig syntax but is valid in their custom implementation. Fields prefixed with `#` are private fields that cannot be accessed from outside the defining struct.
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cpp
📚 Learning: 2026-01-07T23:45:24.551Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2026-01-07T23:45:24.551Z
Learning: Applies to src/**/*.zig : Use the `#` prefix for private fields in Zig structs, e.g., `struct { #foo: u32 };`
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cpp
📚 Learning: 2025-09-02T18:25:27.976Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22227
File: src/allocators/allocation_scope.zig:284-314
Timestamp: 2025-09-02T18:25:27.976Z
Learning: In bun's custom Zig implementation, the `#` prefix for private fields is valid syntax and should not be flagged as invalid. The syntax `#fieldname` creates private fields that cannot be accessed from outside the defining struct, and usage like `self.#fieldname` is correct within the same struct. This applies to fields like `#parent`, `#state`, `#allocator`, `#trace`, etc. throughout the codebase.
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cpp
📚 Learning: 2025-09-02T18:27:23.279Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22227
File: src/collections/multi_array_list.zig:24-24
Timestamp: 2025-09-02T18:27:23.279Z
Learning: The `#allocator` syntax in bun's custom Zig implementation is valid and does not require quoting with @"#allocator". Private fields using the `#` prefix work correctly throughout the codebase without special quoting syntax.
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cpp
📚 Learning: 2026-01-07T23:45:24.551Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2026-01-07T23:45:24.551Z
Learning: Applies to src/**/*.zig : Never use `import()` inline inside functions in Zig; always place imports at the bottom of the file or containing struct
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cpp
📚 Learning: 2025-09-26T01:09:19.196Z
Learnt from: nektro
Repo: oven-sh/bun PR: 22781
File: src/sql/shared/SQLDataCell.zig:144-168
Timestamp: 2025-09-26T01:09:19.196Z
Learning: In Zig, packed structs with a specified backing integer type (e.g., `packed struct(u32)`) are ABI-identical to their underlying integer type and can be safely passed to C functions expecting the corresponding integer type without any casting or bitcasting.
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cpp
📚 Learning: 2025-10-11T15:19:30.301Z
Learnt from: mastermakrela
Repo: oven-sh/bun PR: 19167
File: src/bun.js/api.zig:31-31
Timestamp: 2025-10-11T15:19:30.301Z
Learning: The learnings about `pub const js = JSC.Codegen.JS<ClassName>` and re-exporting toJS/fromJS/fromJSDirect apply to class-based Zig bindings with constructors and prototype methods (e.g., those with `new` constructors). They do NOT apply to simple namespace-style API objects like TOMLObject, YAMLObject, and CSVObject which expose static functions via a `create()` method that returns a plain JS object.
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cpp
📚 Learning: 2025-10-01T21:48:38.278Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/Bindgen/IDLTypes.h:1-3
Timestamp: 2025-10-01T21:48:38.278Z
Learning: In the Bun codebase, for `BunIDL*` and `Bindgen*` headers (e.g., BunIDLTypes.h, Bindgen/IDLTypes.h), it's acceptable to rely on transitive includes for standard library headers like <type_traits> and <utility> rather than including them explicitly.
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cpp
📚 Learning: 2025-09-25T18:14:27.731Z
Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22842
File: src/bun.js/webcore/ResumableSink.zig:274-276
Timestamp: 2025-09-25T18:14:27.731Z
Learning: In Zig code, private fields are declared and accessed using the `#` prefix. When a field is declared as `#field_name`, it must be accessed as `this.#field_name`, not `this.field_name`. The `#` prefix is part of the private field access syntax and should not be removed.
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cpp
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun bd test <...test file>` to run tests with compiled code changes. Do not use `bun test` as it will not include your changes.
Applied to files:
test/js/bun/jsc/bun-jsc.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : For multi-file tests, prefer `tempDir` and `Bun.spawn` over single-file tests
Applied to files:
test/js/bun/jsc/bun-jsc.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Never use `bun test` directly - always use `bun bd test` to run tests with debug build changes
Applied to files:
test/js/bun/jsc/bun-jsc.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Do not write flaky tests - do not use `setTimeout` in tests; instead await the condition to be met
Applied to files:
test/js/bun/jsc/bun-jsc.test.ts
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.ts : Builtin functions must include `this` parameter typing in TypeScript to enable direct method binding in C++
Applied to files:
test/js/bun/jsc/bun-jsc.test.ts
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use JSC intrinsics (prefixed with `$`) such as `$Array.from()`, `$isCallable()`, and `$newArrayWithSize()` for performance-critical operations
Applied to files:
test/js/bun/jsc/bun-jsc.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Use `describe.each()` for parameterized tests, `toMatchSnapshot()` for snapshot testing, and lifecycle hooks like `beforeAll()`, `afterEach()`, `beforeEach()` for setup/teardown.
Applied to files:
test/js/bun/jsc/bun-jsc.test.ts
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Export modules using `export default { ... }` syntax; modules are NOT ES modules
Applied to files:
src/js/node/inspector.promises.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Use JSC::WriteBarrier for heap-allocated references in V8 objects and implement visitChildren() for custom heap objects to support garbage collection
Applied to files:
src/bun.js/modules/BunJSCModule.h
📚 Learning: 2025-08-30T09:09:18.384Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22231
File: src/bundler/bundle_v2.zig:48-48
Timestamp: 2025-08-30T09:09:18.384Z
Learning: In Zig, when a module exports a top-level struct, import("./Module.zig") directly returns that struct type and can be used as a type alias without needing to access a field within the module. This is a common pattern in the Bun codebase.
Applied to files:
src/bun.js/HardcodedModule.zig
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Write JS builtins for Bun's Node.js compatibility and APIs, and run `bun bd` after changes
Applied to files:
src/bun.js/HardcodedModule.zig
📚 Learning: 2025-11-20T19:51:32.288Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24880
File: packages/bun-vscode/package.json:382-385
Timestamp: 2025-11-20T19:51:32.288Z
Learning: In the Bun repository, dependencies may be explicitly added to package.json files (even when not directly imported in code) to force version upgrades on transitive dependencies, particularly as part of Aikido security scanner remediation to ensure vulnerable transitive dependencies resolve to patched versions.
Applied to files:
src/bun.js/HardcodedModule.zig
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Organize code into `builtins/` for individual functions, `node/` for Node.js modules, `bun/` for Bun modules, `thirdparty/` for NPM replacements, and `internal/` for internal modules
Applied to files:
src/bun.js/HardcodedModule.zig
📚 Learning: 2026-01-05T16:32:07.551Z
Learnt from: alii
Repo: oven-sh/bun PR: 25474
File: src/bun.js/event_loop/Sigusr1Handler.zig:0-0
Timestamp: 2026-01-05T16:32:07.551Z
Learning: In Zig codebases (e.g., Bun), treat std.posix.sigaction as returning void and do not perform runtime error handling for its failure. The Zig standard library views sigaction failures as programmer errors (unreachable) because they only occur with invalid signals like SIGKILL/SIGSTOP. Apply this pattern across Zig files that call sigaction (e.g., crash_handler.zig, main.zig, filter_run.zig, process.zig) and ensure failures are not handled as recoverable errors; prefer reaching an explicit unreachable/compile-time assumption when such failures are detected.
Applied to files:
src/bun.js/HardcodedModule.zig
🧬 Code graph analysis (6)
test/js/node/inspector/inspector-profiler.test.ts (2)
src/js/node/test.ts (2)
beforeEach(129-133)afterEach(135-139)scripts/utils.mjs (1)
profile(1680-1680)
src/js/node/inspector.ts (1)
src/bun.js/bindings/BunCPUProfiler.cpp (4)
startCPUProfiler(42-57)startCPUProfiler(42-42)isCPUProfilerRunning(37-40)isCPUProfilerRunning(37-37)
src/bun.js/bindings/BunCPUProfiler.h (1)
src/bun.js/bindings/BunCPUProfiler.cpp (6)
setSamplingInterval(32-35)setSamplingInterval(32-32)isCPUProfilerRunning(37-40)isCPUProfilerRunning(37-37)startCPUProfiler(42-57)startCPUProfiler(42-42)
src/bun.js/bindings/JSInspectorProfiler.cpp (2)
src/bun.js/bindings/BunCPUProfiler.h (1)
JSC(6-9)src/bun.js/bindings/BunCPUProfiler.cpp (12)
Bun__startCPUProfiler(19-19)Bun__startCPUProfiler(341-344)Bun__startCPUProfiler(341-341)Bun__stopCPUProfilerAndGetJSON(20-20)Bun__stopCPUProfilerAndGetJSON(346-350)Bun__stopCPUProfilerAndGetJSON(346-346)Bun__setCPUSamplingInterval(21-21)Bun__setCPUSamplingInterval(352-355)Bun__setCPUSamplingInterval(352-352)Bun__isCPUProfilerRunning(22-22)Bun__isCPUProfilerRunning(357-360)Bun__isCPUProfilerRunning(357-357)
test/js/bun/jsc/bun-jsc.test.ts (2)
test/harness.ts (1)
isBuildKite(38-38)scripts/utils.mjs (1)
profile(1680-1680)
src/js/node/inspector.promises.ts (1)
src/js/node/inspector.ts (1)
method(92-135)
🪛 Cppcheck (2.19.0)
src/bun.js/bindings/JSInspectorProfiler.cpp
[information] 42-42: Include file
(missingInclude)
[information] 52-52: Include file
(missingIncludeSystem)
[information] 59-59: Include file
(missingIncludeSystem)
[error] 66-66: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
🔇 Additional comments (23)
test/js/bun/jsc/bun-jsc.test.ts (1)
187-215: LGTM! Good regression test for profiler restart.This test properly verifies the bug fix for
Bun.profile()returning empty traces on subsequent calls. The test:
- Uses deterministic CPU-bound workload (fibonacci)
- Verifies profiler can be reused multiple times
- Makes meaningful assertions on result structure
- Correctly uses
it.todoIffor platform-specific skippingsrc/bun.js/modules/BunJSCModule.h (2)
673-680: LGTM! Correct fix for profiler restartability.The change from
shutdown()topause()+clearData()under lock correctly addresses the bug where the profiler couldn't be restarted. The lock acquisition pattern usingWTF::Lockerwith RAII is thread-safe, and the comment clearly explains the rationale.
690-696: Consistent locking in failure path.Good to see the same lock-guarded pause/clear pattern applied in the error path for consistency.
src/bun.js/bindings/JSInspectorProfiler.h (1)
1-9: LGTM! Clean header structure.The header follows project conventions with proper include guards and JSC macro usage. The function declarations match the native bindings exposed to JavaScript.
src/bun.js/bindings/BunCPUProfiler.h (1)
13-21: LGTM! API additions are well-documented.The new
setSamplingIntervalandisCPUProfilerRunningdeclarations align with their implementations. The updated comment forstopCPUProfilerAndGetJSONaccurately describes the empty-string return case.src/js/node/inspector.promises.ts (1)
21-28: LGTM! Export structure follows guidelines.The
export default { ... }syntax correctly follows the module export pattern required for hardcoded modules.test/js/node/inspector/inspector-profiler.test.ts (5)
1-19: LGTM! Well-structured test setup.The test file follows best practices:
- Proper imports from
bun:testbeforeEach/afterEachhooks for session lifecycle management- Try-catch in cleanup to handle already-disconnected sessions gracefully
58-66: Good use of Promise.withResolvers for callback testing.This correctly follows the guideline to use
Promise.withResolverswhen testing callback-based APIs, avoiding flakysetTimeoutpatterns.
111-145: Comprehensive profile structure validation.Excellent coverage of the CDP profile format including nodes, timestamps, samples, timeDeltas, and root call frame validation. This ensures compatibility with Chrome DevTools and other tools.
263-281: Critical regression test for profiler restart.This test directly validates the bug fix mentioned in the PR - ensuring the profiler can be restarted after stop. This is essential coverage for the
shutdown() → pause()change.
376-429: Good promises API parity tests.The promise-based API tests properly verify:
post()returns a Promise- Errors reject the Promise
- Complete workflow functions correctly
- Other exports are shared with the callback-based module
src/bun.js/bindings/BunCPUProfiler.cpp (4)
21-22: LGTM: New public API declarations.The extern "C" declarations for
Bun__setCPUSamplingIntervalandBun__isCPUProfilerRunningare correctly declared and match their implementations later in the file.
32-40: LGTM: Helper functions correctly implemented.The helper functions
setSamplingIntervalandisCPUProfilerRunningprovide appropriate access to the internal state. Note that the thread safety concerns mentioned earlier for the static variables also apply to these functions.
42-57: LGTM: Configurable sampling interval correctly implemented.The profiler now uses the configurable
s_samplingInterval(line 53) and correctly tracks the running state (line 56). The sequencing ensures the profiler is configured and started before marking it as running.
352-360: LGTM: Extern C wrappers correctly implemented.The extern "C" implementations properly delegate to the internal helper functions and match their forward declarations.
src/bun.js/HardcodedModule.zig (3)
68-68: LGTM: Module enum entry correctly added.The
@"node:inspector/promises"enum value is properly placed and follows the naming convention for Node.js modules.
125-125: LGTM: Module mappings and aliases correctly added.The new
node:inspector/promisesmodule is properly registered in three places:
- Public map (line 125): maps string to enum
- Common aliases with prefix (line 235):
node:inspector/promises- Common aliases without prefix (line 291):
inspector/promisesThis follows the same pattern as other Node.js sub-path modules like
dns/promisesandfs/promises.Also applies to: 235-235, 291-291
397-397: LGTM: Simplified node_aliases construction.The
node_aliasesnow derives solely fromcommon_alias_kvs, which is a clean simplification. This ensures consistency for thenode:inspector/promisesmodule across both Node.js and Bun targets.src/bun.js/bindings/JSInspectorProfiler.cpp (5)
1-12: LGTM: Includes and extern declarations.The includes are appropriate, and the extern "C" declarations correctly match the API provided by
BunCPUProfiler.cpp.
14-19: LGTM: Start profiler binding is straightforward.The function correctly calls
Bun__startCPUProfilerand returns undefined. Note that there's no error handling, which assumes the underlying C++ function cannot fail or will handle errors internally.
21-34: LGTM: Stop profiler binding with proper resource management.The function correctly:
- Retrieves the CPU profile JSON via
Bun__stopCPUProfilerAndGetJSON- Handles empty/dead results by returning an empty JS string
- Properly manages the
BunStringlifecycle withderef()(line 31)- Converts to JSC string value
36-61: LGTM: Sampling interval binding with thorough validation.The function provides comprehensive input validation:
- Argument count check (lines 42-45)
- Type check for number (lines 47-51)
- Range check for positive value (lines 54-57)
The
static_cast<int>(line 53) from double is acceptable for microseconds, though very large values might lose precision. Practically, sampling intervals are small numbers.
Consider validating profiler state when setting interval.
The function allows changing the sampling interval at any time without checking if the profiler is running. If the interval should only be changed when the profiler is stopped, consider adding a check:
if (Bun__isCPUProfilerRunning()) { throwTypeError(globalObject, scope, "Cannot change sampling interval while profiler is running"_s); return {}; }However, this may be intentional to allow dynamic adjustment. Verify the expected behavior based on the Chrome DevTools Protocol specification.
63-67: LGTM: Profiler running state query.The function correctly wraps
Bun__isCPUProfilerRunning()and returns a JavaScript boolean value. Simple and correct.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @src/bun.js/bindings/JSInspectorProfiler.cpp:
- Around line 9-13: Remove the local extern "C" redeclarations for
Bun__startCPUProfiler, Bun__stopCPUProfilerAndGetJSON,
Bun__setCPUSamplingInterval, and Bun__isCPUProfilerRunning in
JSInspectorProfiler.cpp and instead include the header that owns those
declarations (e.g. BunCPUProfiler.h) at the top of the file so the compiler uses
the single authoritative declarations; verify the header exposes the listed
symbols and delete the redundant extern "C" block from JSInspectorProfiler.cpp.
- Around line 21-34: The host function jsFunction_stopCPUProfiler does not use a
ThrowScope or check for exceptions around jsString(vm, wtfString) and may skip
returning {} on exceptions; wrap the jsString call in a JSC::ThrowScope (or
equivalent) and check ThrowScope::exception() after creating the JSString so
that you return JSValue::encode(jsEmptyString(...)) on exception; also ensure
result.deref() is always called once ownership is determined (move
result.deref() into a finally-like path executed before any return, e.g., call
deref() just after converting to WTF::String and before entering the ThrowScope
or ensure it runs in both success and exception branches) to avoid leaks.
- Around line 36-61: In jsFunction_setCPUSamplingInterval, avoid UB and silent
truncation from static_cast<int>(intervalArg.asNumber()) by validating the raw
double first: retrieve double d = intervalArg.asNumber(), check std::isfinite(d)
and not std::isnan, ensure d is an integer (e.g., check floor(d) == d or use
std::modf) and within int bounds and > 0; if any check fails, call
throwTypeError or throwRangeError as appropriate, otherwise safely cast to int
(or use a checked clamp to INT_MAX if desired) and pass that value to
Bun__setCPUSamplingInterval; keep references to intervalArg,
jsFunction_setCPUSamplingInterval, and Bun__setCPUSamplingInterval to locate the
change.
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 (1)
src/bun.js/bindings/JSInspectorProfiler.cpp
🧰 Additional context used
📓 Path-based instructions (1)
src/bun.js/bindings/**/*.cpp
📄 CodeRabbit inference engine (CLAUDE.md)
src/bun.js/bindings/**/*.cpp: Create classes in three parts in C++ when there is a public constructor: Foo (JSDestructibleObject), FooPrototype (JSNonFinalObject), and FooConstructor (InternalFunction)
Define properties using HashTableValue arrays in C++ JavaScript class bindings
Add iso subspaces for C++ classes with fields in JavaScript class bindings
Cache structures in ZigGlobalObject for JavaScript class bindings
Files:
src/bun.js/bindings/JSInspectorProfiler.cpp
🧠 Learnings (19)
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : Add BUN_EXPORT visibility attribute to all public V8 API functions to ensure proper symbol export across platforms
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.dyn : Add symbol names with leading underscore and semicolons in braces to src/symbols.dyn for each new V8 API method
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:37:47.899Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/AGENTS.md:0-0
Timestamp: 2025-11-24T18:37:47.899Z
Learning: Applies to src/bun.js/bindings/v8/**/<UNKNOWN> : <UNKNOWN>
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Create classes in three parts in C++ when there is a public constructor: Foo (JSDestructibleObject), FooPrototype (JSNonFinalObject), and FooConstructor (InternalFunction)
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.txt : Add symbol names without leading underscore to src/symbols.txt for each new V8 API method
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Add iso subspaces for C++ classes with fields in JavaScript class bindings
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-12-23T06:50:41.142Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 25429
File: src/bun.js/bindings/helpers.h:422-422
Timestamp: 2025-12-23T06:50:41.142Z
Learning: In Bun's C++ bindings (src/bun.js/bindings/helpers.h and similar files), when returning an empty JSC::Identifier and a VM is accessible, prefer using `vm.propertyNames->emptyIdentifier` over constructing with `JSC::Identifier(JSC::Identifier::EmptyIdentifierFlag::EmptyIdentifier)`. The cached identifier from the VM's property names table is more efficient and consistent with WebKit upgrade patterns.
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8-module/main.cpp : Create test functions in test/v8/v8-module/main.cpp that take FunctionCallbackInfo<Value> parameter, use the test V8 API, print results for comparison with Node.js, and return Undefined
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Create V8 class implementations with .cpp extension following the pattern V8ClassName.cpp that include the header, v8_compatibility_assertions.h, use ASSERT_V8_TYPE_LAYOUT_MATCHES macro, and implement methods using isolate->currentHandleScope()->createLocal<T>() for handle creation
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8-module/main.cpp : Register new V8 API test functions in the Init method using NODE_SET_METHOD with exports object
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Cache structures in ZigGlobalObject for JavaScript class bindings
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/napi/napi.zig : For each new V8 C++ method, add both GCC/Clang and MSVC mangled symbol names to the V8API struct in src/napi/napi.zig using extern fn declarations
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-09-20T05:35:57.318Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 22534
File: src/bun.js/bindings/headers.h:729-731
Timestamp: 2025-09-20T05:35:57.318Z
Learning: symbols.txt in the Bun codebase is specifically for V8 API mangled symbols (without leading underscore), not for general Bun host functions declared with BUN_DECLARE_HOST_FUNCTION. Host functions are handled through different build mechanisms.
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-10-01T21:59:54.571Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h:47-74
Timestamp: 2025-10-01T21:59:54.571Z
Learning: In the new bindings generator (bindgenv2) for `src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h`, the context-aware enumeration conversion overloads intentionally use stricter validation (requiring `value.isString()` without ToString coercion), diverging from Web IDL semantics. This is a design decision documented in comments.
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-10-18T20:59:45.579Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:458-475
Timestamp: 2025-10-18T20:59:45.579Z
Learning: In src/bun.js/telemetry.zig, the RequestId (u64) to JavaScript number (f64) conversion in jsRequestId() is intentionally allowed to lose precision beyond 2^53-1. This is acceptable because: (1) at 1M requests/sec it takes ~285 years to overflow, (2) the counter resets per-process, and (3) these are observability IDs, not critical distributed IDs. Precision loss is an acceptable trade-off for this use case.
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-10T00:57:09.173Z
Learnt from: franciscop
Repo: oven-sh/bun PR: 24514
File: src/bun.js/api/crypto/PasswordObject.zig:86-101
Timestamp: 2025-11-10T00:57:09.173Z
Learning: In Bun's Zig codebase (PasswordObject.zig), when validating the parallelism parameter for Argon2, the upper limit is set to 65535 (2^16 - 1) rather than using `std.math.maxInt(u24)` because the latter triggers Zig's truncation limit checks. The value 65535 is a practical upper bound that avoids compiler issues while being sufficient for thread parallelism use cases.
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Use V8_UNIMPLEMENTED() macro for functions not yet implemented in V8 compatibility classes
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : Create V8 class headers with .h extension following the pattern V8ClassName.h that include pragma once, v8.h, V8Local.h, V8Isolate.h, and declare classes extending from Data with BUN_EXPORT static methods
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Use localToJSValue() to convert V8 handles to JSC values and perform JSC operations within V8 method implementations
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
🧬 Code graph analysis (1)
src/bun.js/bindings/JSInspectorProfiler.cpp (2)
src/bun.js/bindings/BunCPUProfiler.h (1)
JSC(6-9)src/bun.js/bindings/BunCPUProfiler.cpp (12)
Bun__startCPUProfiler(19-19)Bun__startCPUProfiler(341-344)Bun__startCPUProfiler(341-341)Bun__stopCPUProfilerAndGetJSON(20-20)Bun__stopCPUProfilerAndGetJSON(346-350)Bun__stopCPUProfilerAndGetJSON(346-346)Bun__setCPUSamplingInterval(21-21)Bun__setCPUSamplingInterval(352-355)Bun__setCPUSamplingInterval(352-352)Bun__isCPUProfilerRunning(22-22)Bun__isCPUProfilerRunning(357-360)Bun__isCPUProfilerRunning(357-357)
🪛 Cppcheck (2.19.0)
src/bun.js/bindings/JSInspectorProfiler.cpp
[information] 42-42: Include file
(missingInclude)
[information] 52-52: Include file
(missingIncludeSystem)
[information] 59-59: Include file
(missingIncludeSystem)
[error] 66-66: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
🔇 Additional comments (1)
src/bun.js/bindings/JSInspectorProfiler.cpp (1)
14-19: Host bindings are straightforward and match the intended API surface.
startCPUProfiler/isCPUProfilerRunningare minimal and consistent with the C entry points.Also applies to: 63-67
- Use Bun:: namespace functions instead of Bun__ extern "C" wrappers - Use createNotEnoughArgumentsError() for missing arguments - Use Bun::V::validateInteger() for proper integer validation - Remove unused extern "C" wrappers (setSamplingInterval, isCPUProfilerRunning) - Reset sampling interval to default in profile() when not specified - Delete flaky timestamp test (low value, wrong assumptions)
Contributor
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 (1)
src/bun.js/bindings/BunCPUProfiler.cpp (1)
40-55: Add an "already running" guard tostartCPUProfiler()or enforce via call site.On lines 40-55,
startCPUProfiler()unconditionally resets the profiling start time, creates a new stopwatch, and restarts the sampling profiler. If called twice (or if the call site inJSInspectorProfiler.cppforgets to checkisCPUProfilerRunning()), the profiling data from the first start is lost and the profiler state becomes unpredictable.Additionally,
s_isProfilerRunningis a global static flag, not per-VM, creating potential data corruption if multiple VMs are profiling simultaneously.Either add an early return in
startCPUProfiler()ifs_isProfilerRunningis already true, or enforce the guard consistently at all call sites and migrate state to be per-VM.
🤖 Fix all issues with AI agents
In @src/bun.js/bindings/BunCPUProfiler.cpp:
- Around line 70-93: The code correctly grabs JSLockHolder, locks the profiler
(profiler->getLock()/WTF::Locker) and calls profiler->pause(),
releaseStackTraces(), clearData(), but the computed time deltas use wall-clock
conversions of stackTrace.timestamp which can be corrupted by system clock
jumps; change the delta computation to use monotonic deltas (delta =
stackTrace.timestamp - lastTimestamp) for per-sample intervals while continuing
to compute startTime/endTime as epoch microseconds for reporting; update
references where timeDeltas or similar arrays are filled to use monotonic
subtraction of stackTrace.timestamp vs lastTimestamp and only fallback/clamp for
negative deltas if strictly necessary.
In @src/bun.js/bindings/JSInspectorProfiler.cpp:
- Around line 45-49: The binding jsFunction_isCPUProfilerRunning currently calls
the global static Bun::isCPUProfilerRunning(), causing cross-session/worker
bleed; change it to query the VM- or inspector-specific profiler instance
instead (e.g., obtain the profiler from the JSGlobalObject/Inspector tied to the
current JS VM and call its instance isRunning()/isActive() method). Update
JSInspectorProfiler.cpp's jsFunction_isCPUProfilerRunning to fetch the per-VM
profiler (similar to patterns in BunCPUProfiler.cpp), call the instance method,
and return that result rather than using Bun::isCPUProfilerRunning().
- Around line 26-43: The jsFunction_setCPUSamplingInterval currently calls
Bun::V::validateInteger with jsUndefined() max and writes into an int named
interval, risking overflow; update the call to supply a safe explicit upper
bound (e.g. jsNumber(INT_MAX) or a reasonable MAX_SAMPLING_INTERVAL constant) or
validate/clamp interval after extraction and return a VM error if out of range,
so that validateInteger cannot write a value that overflows the int used by
Bun::setSamplingInterval; reference jsFunction_setCPUSamplingInterval, the local
variable interval, Bun::V::validateInteger, and Bun::setSamplingInterval when
making this change.
In @src/bun.js/modules/BunJSCModule.h:
- Around line 655-662: Validate sampleValue before calling toUInt32(): ensure
sampleValue.isNumber() && !sampleValue.isNaN() &&
isFinite(sampleValue.toNumber(globalObject)) and that the numeric value > 0 (and
optionally <= a sensible max) before using toUInt32(); if validation fails
(negative, NaN, zero, or out-of-range) fall back to the default 1000
microseconds when calling
samplingProfiler.setTimingInterval(Seconds::fromMicroseconds(...)). Use
sampleValue.toNumber(globalObject) for checks to avoid toUInt32() wraparound and
only call toUInt32() after the value is known valid.
- Around line 675-684: The profile-reporting code currently calls
samplingProfiler.reportTopFunctions, samplingProfiler.reportTopBytecodes, and
samplingProfiler.stackTracesAsJSON() before acquiring
samplingProfiler.getLock(), causing races with the sampling thread; fix it by
acquiring the lock (auto& lock = samplingProfiler.getLock(); WTF::Locker
locker{lock};) before invoking reportTopFunctions, reportTopBytecodes, and
stackTracesAsJSON(), then perform samplingProfiler.pause() and
samplingProfiler.clearData() while still holding the lock so all reads and state
changes are protected.
In @test/js/node/inspector/inspector-profiler.test.ts:
- Around line 365-371: The test "post() rejects on error" uses `await
expect(...).rejects.toThrow(...)`; change it to Bun's async-throw pattern by
wrapping the promise call in an async function and asserting `.toThrow("not
enabled")` on that function instead — e.g., create the Session via `new
inspectorPromises.Session()`, call `session.connect()`, then use `expect(async
() => { await session.post("Profiler.start"); }).toThrow("not enabled")`, and
finally call `session.disconnect()`; keep the test name and setup/teardown
(session.connect/disconnect) as-is.
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 (4)
src/bun.js/bindings/BunCPUProfiler.cppsrc/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/modules/BunJSCModule.htest/js/node/inspector/inspector-profiler.test.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts?(x): Never usebun testdirectly - always usebun bd testto run tests with debug build changes
For single-file tests, prefer-eflag overtempDir
For multi-file tests, prefertempDirandBun.spawnover single-file tests
UsenormalizeBunSnapshotto normalize snapshot output of tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
UsetempDirfromharnessto create temporary directories - do not usetmpdirSyncorfs.mkdtempSync
When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Do not write flaky tests - do not usesetTimeoutin tests; instead await the condition to be met
Verify tests fail withUSE_SYSTEM_BUN=1 bun test <file>and pass withbun bd test <file>- tests are invalid if they pass with USE_SYSTEM_BUN=1
Test files must end with.test.tsor.test.tsx
Avoid shell commands likefindorgrepin tests - use Bun's Glob and built-in tools instead
Files:
test/js/node/inspector/inspector-profiler.test.ts
test/**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
Always use
port: 0in tests - do not hardcode ports or use custom random port number functions
Files:
test/js/node/inspector/inspector-profiler.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 bd test <...test file>to run tests with compiled code changes. Do not usebun testas it will not include your changes.
Usebun:testfor files ending in*.test.{ts,js,jsx,tsx,mjs,cjs}. For test files without .test extension in test/js/node/test/{parallel,sequential}/*.js, usebun bd <file>instead ofbun bd test <file>since they expect exit code 0.
Do not set a timeout on tests. Bun already has timeouts built-in.
Files:
test/js/node/inspector/inspector-profiler.test.ts
src/bun.js/bindings/**/*.cpp
📄 CodeRabbit inference engine (CLAUDE.md)
src/bun.js/bindings/**/*.cpp: Create classes in three parts in C++ when there is a public constructor: Foo (JSDestructibleObject), FooPrototype (JSNonFinalObject), and FooConstructor (InternalFunction)
Define properties using HashTableValue arrays in C++ JavaScript class bindings
Add iso subspaces for C++ classes with fields in JavaScript class bindings
Cache structures in ZigGlobalObject for JavaScript class bindings
Files:
src/bun.js/bindings/BunCPUProfiler.cppsrc/bun.js/bindings/JSInspectorProfiler.cpp
🧠 Learnings (45)
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output
Applied to files:
test/js/node/inspector/inspector-profiler.test.tssrc/bun.js/modules/BunJSCModule.h
📚 Learning: 2025-10-19T04:55:33.099Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/http/node-telemetry.test.ts:27-203
Timestamp: 2025-10-19T04:55:33.099Z
Learning: In test/js/bun/http/node-telemetry.test.ts and the Bun.telemetry._node_binding API, after the architecture refactor, the _node_binding interface only contains two methods: handleIncomingRequest(req, res) and handleWriteHead(res, statusCode). The handleRequestFinish hook and other lifecycle hooks were removed during simplification. Both current methods are fully tested.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use `$debug()` for debug logging and `$assert()` for assertions; both are stripped in release builds
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Ensure V8 API tests compare identical C++ code output between Node.js and Bun through the test suite validation process
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Do not set a timeout on tests. Bun already has timeouts built-in.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-10-08T13:48:02.430Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 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/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Prefer async/await over callbacks in tests. When callbacks must be used for a single callback, use `Promise.withResolvers` to create a promise that can be resolved or rejected from a callback.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-10-18T05:23:24.403Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/telemetry-server.test.ts:91-100
Timestamp: 2025-10-18T05:23:24.403Z
Learning: In the Bun codebase, telemetry tests (test/js/bun/telemetry-*.test.ts) should focus on telemetry API behavior: configure/disable/isEnabled, callback signatures and invocation, request ID correlation, and error handling. HTTP protocol behaviors like status code normalization (e.g., 200 with empty body → 204) should be tested in HTTP server tests (test/js/bun/http/), not in telemetry tests. Keep separation of concerns: telemetry tests verify the telemetry API contract; HTTP tests verify HTTP semantics.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-09-20T03:39:41.770Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 22534
File: test/regression/issue/21830.fixture.ts:14-63
Timestamp: 2025-09-20T03:39:41.770Z
Learning: Bun's test runner supports async describe callbacks, unlike Jest/Vitest where describe callbacks must be synchronous. The syntax `describe("name", async () => { ... })` is valid in Bun.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-11-06T00:58:23.965Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24417
File: test/js/bun/spawn/spawn.test.ts:903-918
Timestamp: 2025-11-06T00:58:23.965Z
Learning: In Bun test files, `await using` with spawn() is appropriate for long-running processes that need guaranteed cleanup on scope exit or when explicitly testing disposal behavior. For short-lived processes that exit naturally (e.g., console.log scripts), the pattern `const proc = spawn(...); await proc.exited;` is standard and more common, as evidenced by 24 instances vs 4 `await using` instances in test/js/bun/spawn/spawn.test.ts.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-10-25T17:20:19.041Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 24063
File: test/js/bun/telemetry/server-header-injection.test.ts:5-20
Timestamp: 2025-10-25T17:20:19.041Z
Learning: In the Bun telemetry codebase, tests are organized into two distinct layers: (1) Internal API tests in test/js/bun/telemetry/ use numeric InstrumentKind enum values to test Zig↔JS injection points and low-level integration; (2) Public API tests in packages/bun-otel/test/ use string InstrumentKind values ("http", "fetch", etc.) to test the public-facing BunSDK and instrumentation APIs. This separation allows internal tests to use efficient numeric enums for refactoring flexibility while the public API maintains a developer-friendly string-based interface.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-10-19T03:01:29.084Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:286-289
Timestamp: 2025-10-19T03:01:29.084Z
Learning: In src/bun.js/telemetry.zig, the guard preventing double configuration (lines 213-216) is intentional. The telemetry API uses a single-shot configuration model where configure() is called once during application startup. Users must call Bun.telemetry.configure(null) to reset before reconfiguring. This design ensures: (1) predictable state—callbacks don't change mid-request, avoiding race conditions; (2) zero overhead when disabled—no checking for callback changes on every request; (3) clear ownership—one adapter (e.g., bun-otel) owns the telemetry config. Merge-style reconfiguration would require atomic updates during active requests, adding overhead to the hot path.
<!-- [/add_learning]
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cpp
📚 Learning: 2025-10-15T22:03:50.832Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/envvars.zig:135-144
Timestamp: 2025-10-15T22:03:50.832Z
Learning: In src/envvars.zig, the boolean feature flag cache uses a single atomic enum and should remain monotonic. Only the string cache (which uses two atomics: ptr and len) requires acquire/release ordering to prevent torn reads.
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cpp
📚 Learning: 2025-12-23T06:50:41.142Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 25429
File: src/bun.js/bindings/helpers.h:422-422
Timestamp: 2025-12-23T06:50:41.142Z
Learning: In Bun's C++ bindings (src/bun.js/bindings/helpers.h and similar files), when returning an empty JSC::Identifier and a VM is accessible, prefer using `vm.propertyNames->emptyIdentifier` over constructing with `JSC::Identifier(JSC::Identifier::EmptyIdentifierFlag::EmptyIdentifier)`. The cached identifier from the VM's property names table is more efficient and consistent with WebKit upgrade patterns.
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cppsrc/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/modules/BunJSCModule.h
📚 Learning: 2025-11-24T18:37:47.899Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/AGENTS.md:0-0
Timestamp: 2025-11-24T18:37:47.899Z
Learning: Applies to src/bun.js/bindings/v8/**/<UNKNOWN> : <UNKNOWN>
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cppsrc/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/modules/BunJSCModule.h
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/napi/napi.zig : For each new V8 C++ method, add both GCC/Clang and MSVC mangled symbol names to the V8API struct in src/napi/napi.zig using extern fn declarations
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cppsrc/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-09-02T17:14:46.924Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22227
File: src/safety/alloc.zig:93-95
Timestamp: 2025-09-02T17:14:46.924Z
Learning: In bun's Zig codebase, they use a custom extension of Zig that supports private field syntax with the `#` prefix (e.g., `#allocator`, `#trace`). This is not standard Zig syntax but is valid in their custom implementation. Fields prefixed with `#` are private fields that cannot be accessed from outside the defining struct.
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cpp
📚 Learning: 2026-01-07T23:45:24.551Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2026-01-07T23:45:24.551Z
Learning: Applies to src/**/*.zig : Use the `#` prefix for private fields in Zig structs, e.g., `struct { #foo: u32 };`
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cpp
📚 Learning: 2025-09-02T18:25:27.976Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22227
File: src/allocators/allocation_scope.zig:284-314
Timestamp: 2025-09-02T18:25:27.976Z
Learning: In bun's custom Zig implementation, the `#` prefix for private fields is valid syntax and should not be flagged as invalid. The syntax `#fieldname` creates private fields that cannot be accessed from outside the defining struct, and usage like `self.#fieldname` is correct within the same struct. This applies to fields like `#parent`, `#state`, `#allocator`, `#trace`, etc. throughout the codebase.
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cpp
📚 Learning: 2025-09-02T18:27:23.279Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22227
File: src/collections/multi_array_list.zig:24-24
Timestamp: 2025-09-02T18:27:23.279Z
Learning: The `#allocator` syntax in bun's custom Zig implementation is valid and does not require quoting with @"#allocator". Private fields using the `#` prefix work correctly throughout the codebase without special quoting syntax.
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cpp
📚 Learning: 2026-01-07T23:45:24.551Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2026-01-07T23:45:24.551Z
Learning: Applies to src/**/*.zig : Never use `import()` inline inside functions in Zig; always place imports at the bottom of the file or containing struct
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cpp
📚 Learning: 2025-09-26T01:09:19.196Z
Learnt from: nektro
Repo: oven-sh/bun PR: 22781
File: src/sql/shared/SQLDataCell.zig:144-168
Timestamp: 2025-09-26T01:09:19.196Z
Learning: In Zig, packed structs with a specified backing integer type (e.g., `packed struct(u32)`) are ABI-identical to their underlying integer type and can be safely passed to C functions expecting the corresponding integer type without any casting or bitcasting.
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cpp
📚 Learning: 2025-10-11T15:19:30.301Z
Learnt from: mastermakrela
Repo: oven-sh/bun PR: 19167
File: src/bun.js/api.zig:31-31
Timestamp: 2025-10-11T15:19:30.301Z
Learning: The learnings about `pub const js = JSC.Codegen.JS<ClassName>` and re-exporting toJS/fromJS/fromJSDirect apply to class-based Zig bindings with constructors and prototype methods (e.g., those with `new` constructors). They do NOT apply to simple namespace-style API objects like TOMLObject, YAMLObject, and CSVObject which expose static functions via a `create()` method that returns a plain JS object.
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cpp
📚 Learning: 2025-10-01T21:48:38.278Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/Bindgen/IDLTypes.h:1-3
Timestamp: 2025-10-01T21:48:38.278Z
Learning: In the Bun codebase, for `BunIDL*` and `Bindgen*` headers (e.g., BunIDLTypes.h, Bindgen/IDLTypes.h), it's acceptable to rely on transitive includes for standard library headers like <type_traits> and <utility> rather than including them explicitly.
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cppsrc/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-09-25T18:14:27.731Z
Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22842
File: src/bun.js/webcore/ResumableSink.zig:274-276
Timestamp: 2025-09-25T18:14:27.731Z
Learning: In Zig code, private fields are declared and accessed using the `#` prefix. When a field is declared as `#field_name`, it must be accessed as `this.#field_name`, not `this.field_name`. The `#` prefix is part of the private field access syntax and should not be removed.
Applied to files:
src/bun.js/bindings/BunCPUProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : Add BUN_EXPORT visibility attribute to all public V8 API functions to ensure proper symbol export across platforms
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/modules/BunJSCModule.h
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Create V8 class implementations with .cpp extension following the pattern V8ClassName.cpp that include the header, v8_compatibility_assertions.h, use ASSERT_V8_TYPE_LAYOUT_MATCHES macro, and implement methods using isolate->currentHandleScope()->createLocal<T>() for handle creation
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/modules/BunJSCModule.h
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.dyn : Add symbol names with leading underscore and semicolons in braces to src/symbols.dyn for each new V8 API method
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Add iso subspaces for C++ classes with fields in JavaScript class bindings
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/modules/BunJSCModule.h
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8-module/main.cpp : Create test functions in test/v8/v8-module/main.cpp that take FunctionCallbackInfo<Value> parameter, use the test V8 API, print results for comparison with Node.js, and return Undefined
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/modules/BunJSCModule.h
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Create classes in three parts in C++ when there is a public constructor: Foo (JSDestructibleObject), FooPrototype (JSNonFinalObject), and FooConstructor (InternalFunction)
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/modules/BunJSCModule.h
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.txt : Add symbol names without leading underscore to src/symbols.txt for each new V8 API method
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Use localToJSValue() to convert V8 handles to JSC values and perform JSC operations within V8 method implementations
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Cache structures in ZigGlobalObject for JavaScript class bindings
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-09-20T05:35:57.318Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 22534
File: src/bun.js/bindings/headers.h:729-731
Timestamp: 2025-09-20T05:35:57.318Z
Learning: symbols.txt in the Bun codebase is specifically for V8 API mangled symbols (without leading underscore), not for general Bun host functions declared with BUN_DECLARE_HOST_FUNCTION. Host functions are handled through different build mechanisms.
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-10-01T21:59:54.571Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h:47-74
Timestamp: 2025-10-01T21:59:54.571Z
Learning: In the new bindings generator (bindgenv2) for `src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h`, the context-aware enumeration conversion overloads intentionally use stricter validation (requiring `value.isString()` without ToString coercion), diverging from Web IDL semantics. This is a design decision documented in comments.
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-10-18T20:59:45.579Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:458-475
Timestamp: 2025-10-18T20:59:45.579Z
Learning: In src/bun.js/telemetry.zig, the RequestId (u64) to JavaScript number (f64) conversion in jsRequestId() is intentionally allowed to lose precision beyond 2^53-1. This is acceptable because: (1) at 1M requests/sec it takes ~285 years to overflow, (2) the counter resets per-process, and (3) these are observability IDs, not critical distributed IDs. Precision loss is an acceptable trade-off for this use case.
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-10T00:57:09.173Z
Learnt from: franciscop
Repo: oven-sh/bun PR: 24514
File: src/bun.js/api/crypto/PasswordObject.zig:86-101
Timestamp: 2025-11-10T00:57:09.173Z
Learning: In Bun's Zig codebase (PasswordObject.zig), when validating the parallelism parameter for Argon2, the upper limit is set to 65535 (2^16 - 1) rather than using `std.math.maxInt(u24)` because the latter triggers Zig's truncation limit checks. The value 65535 is a practical upper bound that avoids compiler issues while being sufficient for thread parallelism use cases.
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-10-24T10:43:09.398Z
Learnt from: fmguerreiro
Repo: oven-sh/bun PR: 23774
File: src/install/PackageManager/updatePackageJSONAndInstall.zig:548-548
Timestamp: 2025-10-24T10:43:09.398Z
Learning: In Bun's Zig codebase, the `as(usize, intCast(...))` cast pattern triggers a Zig compiler bug that causes compilation to hang indefinitely when used in complex control flow contexts (loops + short-circuit operators + optional unwrapping). Avoid this pattern and use simpler alternatives like just `intCast(...)` if type casting is necessary.
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : Create V8 class headers with .h extension following the pattern V8ClassName.h that include pragma once, v8.h, V8Local.h, V8Isolate.h, and declare classes extending from Data with BUN_EXPORT static methods
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/modules/BunJSCModule.h
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Use JSC::WriteBarrier for heap-allocated references in V8 objects and implement visitChildren() for custom heap objects to support garbage collection
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cppsrc/bun.js/modules/BunJSCModule.h
📚 Learning: 2025-11-03T20:40:59.655Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/bindings/JSValue.zig:545-586
Timestamp: 2025-11-03T20:40:59.655Z
Learning: In Bun's Zig codebase, JSErrors (returned as `bun.JSError!T`) must always be properly handled. Using `catch continue` or `catch { break; }` to silently suppress JSErrors is a bug. Errors should either be explicitly handled or propagated with `try`. This applies to snapshot serializer error handling where Jest's behavior is to throw when serializers throw.
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Use isolate->currentHandleScope()->createLocal<T>() to create local V8 handles and ensure all V8 values are created within an active handle scope
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8-module/main.cpp : Register new V8 API test functions in the Init method using NODE_SET_METHOD with exports object
Applied to files:
src/bun.js/bindings/JSInspectorProfiler.cpp
🧬 Code graph analysis (2)
src/bun.js/bindings/JSInspectorProfiler.cpp (2)
src/bun.js/bindings/BunCPUProfiler.h (1)
JSC(6-9)src/bun.js/bindings/BunCPUProfiler.cpp (8)
startCPUProfiler(40-55)startCPUProfiler(40-40)stopCPUProfilerAndGetJSON(68-335)stopCPUProfilerAndGetJSON(68-68)setSamplingInterval(30-33)setSamplingInterval(30-30)isCPUProfilerRunning(35-38)isCPUProfilerRunning(35-35)
src/bun.js/modules/BunJSCModule.h (5)
src/bun.js/bindings/bindings.cpp (7)
lock(2790-2790)lock(3692-3692)lock(4585-4585)locker(2654-2654)locker(4625-4625)locker(4631-4631)locker(4652-4652)src/bun.js/bindings/webcore/JSDOMPromiseDeferred.cpp (1)
lock(252-252)src/bun.js/bindings/webcore/ReadableStream.cpp (5)
lock(115-115)lock(161-167)lock(161-161)lock(176-176)lock(197-197)src/vm/SigintWatcher.cpp (6)
lock(121-121)lock(131-131)lock(148-148)lock(154-154)lock(189-189)lock(195-195)src/bun.js/bindings/BunCPUProfiler.cpp (1)
locker(77-77)
🪛 Cppcheck (2.19.0)
src/bun.js/bindings/BunCPUProfiler.cpp
[information] 52-52: Include file
(missingIncludeSystem)
src/bun.js/bindings/JSInspectorProfiler.cpp
[information] 42-42: Include file
(missingInclude)
🔇 Additional comments (4)
src/bun.js/modules/BunJSCModule.h (1)
694-700: Good: failure path now pauses + clears under the profiler lock.Locking before
pause()/clearData()on Line 694-700 is the right direction for thread-safety and profiler reusability.src/bun.js/bindings/JSInspectorProfiler.cpp (2)
11-16:startCPUProfilerhost binding is straightforward.
18-24:stopCPUProfilerreturns a string; confirm empty-string behavior is acceptable for callers.If
Bun::stopCPUProfilerAndGetJSON(vm)can return empty when not started, ensure the inspector command layer converts that into the expected protocol error/shape (rather than silently returning"").test/js/node/inspector/inspector-profiler.test.ts (1)
349-402: Please verify this test is “debug-build only” as required (bun bd test) and fails underUSE_SYSTEM_BUN=1.This file exercises newly-compiled runtime behavior, so it should (1) fail under system bun and (2) pass under
bun bd test, per test guidelines.#!/bin/bash set -euo pipefail FILE="test/js/node/inspector/inspector-profiler.test.ts" # Should PASS (uses your compiled changes) bun bd test "$FILE" # Should FAIL (system bun won't include your compiled changes); if it passes, the test is invalid per guidelines USE_SYSTEM_BUN=1 bun test "$FILE"
Coverage APIs (getBestEffortCoverage, startPreciseCoverage, etc.) now return an error instead of silently returning empty objects. This is consistent with how other unsupported methods are handled.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @src/js/node/inspector.ts:
- Around line 45-50: Wrap the call to stopCPUProfiler() inside defensive error
handling in disconnect(): call isCPUProfilerRunning() as before, but invoke
stopCPUProfiler() within a try/catch so any thrown error is caught (log the
error with a clear message) and does not prevent the rest of disconnect from
running; ensure that this method always sets this.#profilerEnabled = false and
this.#connected = false even if stopCPUProfiler() fails (use finally or set
flags after the catch) so disconnect() reliably completes cleanup.
- Around line 63-82: The code calls the global queueMicrotask in inspector
methods (see the session-connected check and the callback branch around
this.#handleMethod), which can be tampered with; replace both uses of
queueMicrotask(...) with the protected $queueMicrotask(...) private global to
ensure a safe microtask enqueue. Update the two occurrences that wrap the
callback (the one passing the error and the one dispatching result or error) to
call $queueMicrotask with the same arrow callbacks so behavior and timing remain
identical.
In @test/js/node/inspector/inspector-profiler.test.ts:
- Around line 365-371: The test "post() rejects on error" uses Jest/Vitest async
rejection syntax; change the assertion to Bun's preferred pattern by wrapping
the async call in an async arrow passed to expect and using toThrow(), e.g.
replace await expect(session.post("Profiler.start")).rejects.toThrow("not
enabled") with expect(async () => { await session.post("Profiler.start");
}).toThrow("not enabled"); keep the same Session instantiation
(inspectorPromises.Session), session.connect() and session.disconnect() behavior
(ensure disconnect still runs).
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 (2)
src/js/node/inspector.tstest/js/node/inspector/inspector-profiler.test.ts
🧰 Additional context used
📓 Path-based instructions (5)
src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}
📄 CodeRabbit inference engine (src/js/CLAUDE.md)
src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}: Use.$call()and.$apply()instead of.call()and.apply()to prevent user tampering with function invocation
Use string literalrequire()statements only; dynamic requires are not permitted
Export modules usingexport default { ... }syntax; modules are NOT ES modules
Use JSC intrinsics (prefixed with$) such as$Array.from(),$isCallable(), and$newArrayWithSize()for performance-critical operations
Use private globals and methods with$prefix (e.g.,$Array,map.$set()) instead of public JavaScript globals
Use$debug()for debug logging and$assert()for assertions; both are stripped in release builds
Validate function arguments using validators frominternal/validatorsand throw$ERR_*error codes for invalid arguments
Useprocess.platformandprocess.archfor platform detection; these values are inlined and dead-code eliminated at build time
Files:
src/js/node/inspector.ts
src/js/{builtins,node,bun,thirdparty,internal}/**/*.ts
📄 CodeRabbit inference engine (src/js/CLAUDE.md)
Builtin functions must include
thisparameter typing in TypeScript to enable direct method binding in C++
Files:
src/js/node/inspector.ts
**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts?(x): Never usebun testdirectly - always usebun bd testto run tests with debug build changes
For single-file tests, prefer-eflag overtempDir
For multi-file tests, prefertempDirandBun.spawnover single-file tests
UsenormalizeBunSnapshotto normalize snapshot output of tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
UsetempDirfromharnessto create temporary directories - do not usetmpdirSyncorfs.mkdtempSync
When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Do not write flaky tests - do not usesetTimeoutin tests; instead await the condition to be met
Verify tests fail withUSE_SYSTEM_BUN=1 bun test <file>and pass withbun bd test <file>- tests are invalid if they pass with USE_SYSTEM_BUN=1
Test files must end with.test.tsor.test.tsx
Avoid shell commands likefindorgrepin tests - use Bun's Glob and built-in tools instead
Files:
test/js/node/inspector/inspector-profiler.test.ts
test/**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
Always use
port: 0in tests - do not hardcode ports or use custom random port number functions
Files:
test/js/node/inspector/inspector-profiler.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 bd test <...test file>to run tests with compiled code changes. Do not usebun testas it will not include your changes.
Usebun:testfor files ending in*.test.{ts,js,jsx,tsx,mjs,cjs}. For test files without .test extension in test/js/node/test/{parallel,sequential}/*.js, usebun bd <file>instead ofbun bd test <file>since they expect exit code 0.
Do not set a timeout on tests. Bun already has timeouts built-in.
Files:
test/js/node/inspector/inspector-profiler.test.ts
🧠 Learnings (22)
📚 Learning: 2025-10-19T04:55:33.099Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/http/node-telemetry.test.ts:27-203
Timestamp: 2025-10-19T04:55:33.099Z
Learning: In test/js/bun/http/node-telemetry.test.ts and the Bun.telemetry._node_binding API, after the architecture refactor, the _node_binding interface only contains two methods: handleIncomingRequest(req, res) and handleWriteHead(res, statusCode). The handleRequestFinish hook and other lifecycle hooks were removed during simplification. Both current methods are fully tested.
Applied to files:
src/js/node/inspector.tstest/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.ts : Builtin functions must include `this` parameter typing in TypeScript to enable direct method binding in C++
Applied to files:
src/js/node/inspector.ts
📚 Learning: 2025-11-03T20:40:59.655Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/bindings/JSValue.zig:545-586
Timestamp: 2025-11-03T20:40:59.655Z
Learning: In Bun's Zig codebase, JSErrors (returned as `bun.JSError!T`) must always be properly handled. Using `catch continue` or `catch { break; }` to silently suppress JSErrors is a bug. Errors should either be explicitly handled or propagated with `try`. This applies to snapshot serializer error handling where Jest's behavior is to throw when serializers throw.
Applied to files:
src/js/node/inspector.tstest/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-12-05T19:28:39.534Z
Learnt from: alii
Repo: oven-sh/bun PR: 25360
File: src/bake/hmr-runtime-client.ts:240-249
Timestamp: 2025-12-05T19:28:39.534Z
Learning: In Bun's HMR runtime client (src/bake/hmr-runtime-client.ts), when handling error events where `event.error` is falsy, do not wrap `event.message` in a new Error object. Doing so would create misleading stack frames that show the error originating from the HMR client code rather than the actual error location, which hinders debugging.
Applied to files:
src/js/node/inspector.ts
📚 Learning: 2025-09-25T22:07:13.851Z
Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22946
File: test/js/sql/sql.test.ts:195-202
Timestamp: 2025-09-25T22:07:13.851Z
Learning: PR oven-sh/bun#22946: JSON/JSONB result parsing updates (e.g., returning parsed arrays instead of legacy strings) are out of scope for this PR; tests keep current expectations with a TODO. Handle parsing fixes in a separate PR.
Applied to files:
src/js/node/inspector.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use `$debug()` for debug logging and `$assert()` for assertions; both are stripped in release builds
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Ensure V8 API tests compare identical C++ code output between Node.js and Bun through the test suite validation process
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Do not set a timeout on tests. Bun already has timeouts built-in.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-10-18T05:23:24.403Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/telemetry-server.test.ts:91-100
Timestamp: 2025-10-18T05:23:24.403Z
Learning: In the Bun codebase, telemetry tests (test/js/bun/telemetry-*.test.ts) should focus on telemetry API behavior: configure/disable/isEnabled, callback signatures and invocation, request ID correlation, and error handling. HTTP protocol behaviors like status code normalization (e.g., 200 with empty body → 204) should be tested in HTTP server tests (test/js/bun/http/), not in telemetry tests. Keep separation of concerns: telemetry tests verify the telemetry API contract; HTTP tests verify HTTP semantics.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-09-20T00:57:56.685Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 22568
File: test/js/valkey/valkey.test.ts:268-276
Timestamp: 2025-09-20T00:57:56.685Z
Learning: For test/js/valkey/valkey.test.ts, do not comment on synchronous throw assertions for async Redis methods like ctx.redis.set() - the maintainer has explicitly requested to stop looking at this error pattern.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-09-20T00:58:38.042Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 22568
File: test/js/valkey/valkey.test.ts:561-564
Timestamp: 2025-09-20T00:58:38.042Z
Learning: For test/js/valkey/valkey.test.ts, do not comment on synchronous throw assertions for async Redis methods (like ctx.redis.set(), ctx.redis.unsubscribe(), etc.) - Bun's Redis client implementation differs from Node.js and can throw synchronously even for async methods. The maintainer has explicitly requested to stop looking at this error pattern.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-10-08T13:48:02.430Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 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/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-11-08T04:06:33.198Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24491
File: test/js/bun/transpiler/declare-global.test.ts:17-17
Timestamp: 2025-11-08T04:06:33.198Z
Learning: In Bun test files, `await using` with Bun.spawn() is the preferred pattern for spawned processes regardless of whether they are short-lived or long-running. Do not suggest replacing `await using proc = Bun.spawn(...)` with `const proc = Bun.spawn(...); await proc.exited;`.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-11-06T00:58:23.965Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24417
File: test/js/bun/spawn/spawn.test.ts:903-918
Timestamp: 2025-11-06T00:58:23.965Z
Learning: In Bun test files, `await using` with spawn() is appropriate for long-running processes that need guaranteed cleanup on scope exit or when explicitly testing disposal behavior. For short-lived processes that exit naturally (e.g., console.log scripts), the pattern `const proc = spawn(...); await proc.exited;` is standard and more common, as evidenced by 24 instances vs 4 `await using` instances in test/js/bun/spawn/spawn.test.ts.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Prefer async/await over callbacks in tests. When callbacks must be used for a single callback, use `Promise.withResolvers` to create a promise that can be resolved or rejected from a callback.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Do not write flaky tests - do not use `setTimeout` in tests; instead await the condition to be met
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-09-20T03:39:41.770Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 22534
File: test/regression/issue/21830.fixture.ts:14-63
Timestamp: 2025-09-20T03:39:41.770Z
Learning: Bun's test runner supports async describe callbacks, unlike Jest/Vitest where describe callbacks must be synchronous. The syntax `describe("name", async () => { ... })` is valid in Bun.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
📚 Learning: 2025-10-25T17:20:19.041Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 24063
File: test/js/bun/telemetry/server-header-injection.test.ts:5-20
Timestamp: 2025-10-25T17:20:19.041Z
Learning: In the Bun telemetry codebase, tests are organized into two distinct layers: (1) Internal API tests in test/js/bun/telemetry/ use numeric InstrumentKind enum values to test Zig↔JS injection points and low-level integration; (2) Public API tests in packages/bun-otel/test/ use string InstrumentKind values ("http", "fetch", etc.) to test the public-facing BunSDK and instrumentation APIs. This separation allows internal tests to use efficient numeric enums for refactoring flexibility while the public API maintains a developer-friendly string-based interface.
Applied to files:
test/js/node/inspector/inspector-profiler.test.ts
🧬 Code graph analysis (1)
src/js/node/inspector.ts (1)
src/bun.js/bindings/BunCPUProfiler.cpp (4)
startCPUProfiler(40-55)startCPUProfiler(40-40)isCPUProfilerRunning(35-38)isCPUProfilerRunning(35-35)
The report lambda creates its own throwScope and can throw via JSONParse. Both the async (resolve callback) and sync return paths needed proper exception propagation to satisfy JSC's exception scope verification.
robobun
pushed a commit
that referenced
this pull request
Jan 16, 2026
6 tasks done
dylan-conway
pushed a commit
that referenced
this pull request
Jul 20, 2026
…always fires (#33998) Fixes `test/js/bun/jsc/bun-jsc.test.ts` going red on the debian 13 x64 lane in [build 72010](https://buildkite.com/bun/bun/builds/72010#019f546c-0875-422c-8f22-fa88eac52a1b): ``` 215 | expect(result2.stackTraces.traces.length).toBeGreaterThan(0); error: expect(received).toBeGreaterThan(expected) Expected: > 0 Received: 0 ✗ bun:jsc > profile can be called multiple times [4.71ms] ``` ### Cause The test profiles `fib(26)` three times at a 50µs sample interval and asserts each run collected at least one trace. #33072 shrank the workload from `fib(30)` to `fib(26)` so three runs fit inside the per-test timeout on slow debug builds. On fast release hardware a JIT-compiled `fib(26)` can finish inside one 50µs interval (the whole 3-call test completed in 4.71ms there), so the sampler thread simply never fires during the second call. This is the same race #28873 addressed by lowering the sample interval; lowering the instruction count in #33072 reopened it. ### Fix Loop `fib(18)` for a fixed 10ms of wall-clock per `profile()` call instead of relying on a single `fib(n)` being slow enough. 10ms is ~200 sample intervals, so the sampler thread is guaranteed time to fire regardless of JIT tier or how slowly it wakes after `pause()`/`start()`. On debug builds the loop still exits after ~12ms of JS work (one `fib(18)` of overshoot), so the test is no slower there than before. The test still catches the original regression from #25939 (using `shutdown()` instead of `pause()`): a shut-down sampler returns zero traces no matter how long the workload runs. ### Verification - `USE_SYSTEM_BUN=1 bun test test/js/bun/jsc/bun-jsc.test.ts -t 'profile can be called multiple times'`: 30/30 pass, ~80 traces per call - `bun bd test test/js/bun/jsc/bun-jsc.test.ts`: 36 pass / 0 fail, profile test ~1.6s (dominated by the profiler's JSON reporting, same as before) This is a timing flake that does not reproduce on the dev container (`fib(26)` takes ~800µs there after JIT warmup, well above 50µs), so there is no deterministic fail-before to show; the CI failure above is the reproduction. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 2 · Platform-specific test-only change; deferring to CI. <!-- robobun:evidence:end -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Implements the
node:inspectorProfiler API for CPU profiling via the Chrome DevTools Protocol.Supported methods:
Profiler.enable/Profiler.disableProfiler.start/Profiler.stopProfiler.setSamplingIntervalBoth callback (
node:inspector) and promise (node:inspector/promises) APIs are supported.Example:
Bug fix: Also fixes
Bun.profile()frombun:jscreturning empty traces on subsequent calls (profiler thread was killed instead of paused).How did you verify your code works?
profile()calls🤖 Generated with Claude Code