FileSystemRouter: build JS error before freeing arena in error paths - #29971
Merged
Claude / Claude Code Review
completed
Apr 30, 2026 in 20m 31s
Code review found 1 potential issue
Found 1 candidates, confirmed 1. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 0 |
| 🟣 Pre-existing | 1 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟣 Pre-existing | src/bun.js/api/filesystem_router.zig:262-269 |
reload() silently swallows route-validation errors (no log.errors check) |
Annotations
Check notice on line 269 in src/bun.js/api/filesystem_router.zig
claude / Claude Code Review
reload() silently swallows route-validation errors (no log.errors check)
Pre-existing, not introduced here, but while you're normalizing these error paths: `reload()` is missing the `if (log.errors + log.warnings > 0)` check that the constructor has after `loadRoutes()`. Route-validation failures like `[foo.tsx` only do `log.addErrorFmt(...); return null` — `loadRoutes()` itself returns success — so adding such a file and calling `router.reload()` silently swaps in the new router and discards the logged error, while the constructor would throw. Adding the same `const
Loading