Repository navigation
Generate the Rust ErrorCode table from ErrorCode.ts - #33947
Conversation
src/jsc/ErrorCode.rs previously hand-maintained a 1400-line mirror of src/jsc/bindings/ErrorCode.ts (consts, ERR_ aliases, CODE_STR). It had to stay index-aligned with the generated C++ errors[] table; any drift is an out-of-bounds read in Bun__createErrorWithCode. generate-node-errors.ts already produces ErrorCode+List.h and ErrorCode+Data.h from that TS file. It now also emits ErrorCode.generated.rs with the same discriminants, and ErrorCode.rs include!s it from BUN_CODEGEN_DIR. The generated output is a strict superset of the old file: every existing constant keeps its discriminant, CODE_STR is byte-identical, and the 58 ERR_ aliases the hand file was missing are now present.
WalkthroughChangesThe codegen pipeline now emits Error code generation and integration
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Updated 7:03 PM PT - Jul 10th, 2026
⏳ @robobun, your commit 6a8186e is still building in |
The old test parsed the hand-maintained table out of ErrorCode.rs and compared it to ErrorCode.ts. That table is now generated, so the test instead asserts the file stays generated (no hardcoded discriminants) and spot-checks the Rust to C++ Bun__createErrorWithCode round-trip at runtime across the index range.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Pass explicit credentials and an invalid method so presign throws ERR_S3_INVALID_METHOD without consulting the environment.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/js/bun/util/error-code-mirror.test.ts`:
- Around line 48-63: Guard the JSON parsing in the child-process result handling
so empty or invalid stdout does not throw before diagnostics are asserted.
Update the assertions around the Promise.all result to parse stdout defensively
and include the child’s exit signal or termination status alongside stderr,
while preserving validation of the expected output when parsing succeeds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c67b6f08-a2e5-47ab-82ea-3a74023ee3b3
📒 Files selected for processing (6)
scripts/build/codegen.tssrc/codegen/generate-node-errors.tssrc/jsc/ErrorCode.rssrc/jsc/bindings/ErrorCode.tssrc/jsc/build.rstest/js/bun/util/error-code-mirror.test.ts
💤 Files with no reviewable changes (1)
- src/jsc/bindings/ErrorCode.ts
Print one code per line and assert on raw stdout; a crash now shows the partial output and signal in the toEqual diff instead of throwing inside JSON.parse.
There was a problem hiding this comment.
Both of my earlier findings (the stale mirror test, and the non-hermetic S3 case) are addressed, and I found nothing further on this pass. Deferring to a human because this rewires the memory-safety-critical Rust↔C++ ErrorCode discriminant table into codegen and touches the ninja + cargo build graph — worth a maintainer sign-off on the approach even though it follows the established generated_resolved_source_tag.rs pattern.
Checked: the Rust emission shares the same i counter and loop body as the C++ enumHeader/listHeader, so index alignment is structural; codeWithoutPrefix handling matches the deleted constants (incl. the ERR_MODULE_NOT_FOUND special case, HPE_*/ABORT_ERR non-aliasing, and _RangeError-suffixed extras); codegen.ts correctly routes only the two .h outputs to cppHeaders while the .rs goes to rustInputs; build.rs mirrors the existing rerun-if-changed pattern.
Extended reasoning...
Overview
Replaces the ~1400-line hand-maintained src/jsc/ErrorCode.rs constant/alias/CODE_STR mirror with an include! of ErrorCode.generated.rs, emitted by generate-node-errors.ts in the same loop that already produces ErrorCode+List.h / ErrorCode+Data.h. Wires the new output into the ninja codegen graph (scripts/build/codegen.ts) and src/jsc/build.rs, drops the now-obsolete "append only" comment from ErrorCode.ts, and rewrites error-code-mirror.test.ts as (1) a structural check that the hand-mirror is gone and (2) a runtime round-trip of nine Rust-thrown codes through Bun__createErrorWithCode.
Security risks
None. Build-time codegen from an in-tree TypeScript table; no user input, network, or auth surface.
Level of scrutiny
High. The ErrorCode discriminant is passed across FFI and used as an unchecked index into the C++ errors[] array — a misaligned table is UB. That said, the change makes drift structurally impossible (single loop, single counter, three outputs), which is strictly safer than the hand-maintained mirror it replaces. The build-graph and build.rs additions copy the existing generated_resolved_source_tag.rs / cpp.rs pattern line-for-line.
Other factors
I reviewed this PR across three iterations; all findings (stale regex-parsing test, non-hermetic S3 env dependence, and CodeRabbit's JSON.parse-masks-crash nit) were addressed and the threads are resolved. The generator's codeWithoutPrefix / alias-emission / extra-constructor logic reproduces the deleted constants exactly (including ERR_MODULE_NOT_FOUND staying prefixed and no self-aliasing for HPE_*/ABORT_ERR/MODULE_NOT_FOUND). The PR description reports byte-identical CODE_STR, matching COUNT, and 0 discriminant mismatches vs. main. Deferring rather than approving because this is an infrastructure change to a memory-safety-relevant boundary that touches every Rust-thrown error path — a maintainer should confirm the codegen-via-include! approach and the level of test coverage are what they want.
|
CI on 6a8186e: the only hard failure is The Ready for review. |
2 participants
What
src/jsc/ErrorCode.rshand-maintained a ~1400-line mirror ofsrc/jsc/bindings/ErrorCode.ts: theErrorCode::<NAME>consts, theERR_-prefixed aliases,COUNT, and theCODE_STRstring table. These must stay index-aligned with the C++errors[]table inErrorCode+Data.hbecauseBun__createErrorWithCodeindexes that array with the Rust discriminant with no bounds check.generate-node-errors.tsalready emitsErrorCode+List.h/ErrorCode+Data.hfromErrorCode.ts. It now also emitsErrorCode.generated.rsinto${BUN_CODEGEN_DIR}, andErrorCode.rsinclude!s it (same pattern asgenerated_resolved_source_tag.rs). The hand-written file now holds only the type definition, FFI extern, helpers, and sentinels.Verification
Compared the generated output against
main:src/jsc/ErrorCode.rs:CODE_STRbyte-identical, 336 entriesCOUNT= 336 on both sidesERR_aliases are now emitted (the hand-maintained alias block was incomplete)bun bdbuilds clean;cargo check -p bun_jscpasses; spot-checkedERR_INVALID_ARG_TYPE/ERR_OUT_OF_RANGE/ERR_INVALID_URLat runtime.Diff
[review] gate passed · iteration 1 · 6 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 1 rejected · iteration 1
evidence per changed file