Repository navigation
Conversation
…und" The resolver records a package.json that fails to parse in the log and returns a DirInfo without it. The script lookup then reported "Script not found" and never printed that log. Print the log on the not-found exit. When no package.json loaded at all, the parse error is the message and "Script not found" is not printed.
|
Status: reproduced with stock bun 1.4.3 ( |
|
Warning Review limit reached
On-demand reviews are free for the next 14 days. After that, they cost $0.25 per reviewed file. Or wait 49 seconds for your next included review. View limit detailsLimit details: You’ve used all 10 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (2)
Comment |
|
Updated 1:24 PM PT - Sep 6th, 2026
❌ @robobun, your commit ab202f1 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 41700That installs a local version of the PR into your bun-41700 --bun |
| bstr::BStr::new(target_name), | ||
| ); | ||
| } else { | ||
| } else if !(has_log_errors && root_dir.enclosing_package_json.is_none()) { |
There was a problem hiding this comment.
🟡 (optional) The suppression key is the inherited enclosing_package_json, so the misleading "Script not found" line this PR removes still prints in the common monorepo/workspace case: cwd's own package.json fails to parse, an ancestor's is valid, enclosing_package_json is Some(ancestor), and the user sees the parse error followed by "Script not found" for a script that may well be defined in the file that did not load. Fix: gate suppression on the cwd file being the one that failed (e.g. root_dir.package_json.is_none() when a package.json exists in cwd), not on whether any ancestor loaded.
Extended reasoning...
DirInfo.enclosing_package_json is inherited from the parent chain (src/resolver/resolver.rs:6231-6233) and only overwritten with the directory's own file when that file parses (resolver.rs:6354-6361). In a workspace: /repo/package.json valid, /repo/pkgs/app/package.json malformed, cwd /repo/pkgs/app, bun run dev. The resolver logs the parse error and leaves root_dir.package_json = None, but root_dir.enclosing_package_json = Some(/repo/package.json). At line 2704 has_log_errors && root_dir.enclosing_package_json.is_none() is true && false, so the guard is false and Script not found "dev" prints after the parse error. Base branch printed only Script not found, so this path is strictly better than base — but it is exactly the "missing script in a file that did not load" case the PR description says should show only the parse error, and workspaces (where every package sits under a valid root package.json) are where a broken subdirectory package.json is most likely. The second new test only exercises the reverse layout (cwd valid, ancestor broken), so this…
Verification: nit — the mechanism is exactly as described, but merging leaves the base strictly better on this path too (base showed only "Script not found" with no parse error at all; this PR adds the parse error and merely fails to also suppress the trailing line in the monorepo-leaf case). Mechanism verified: - src/resolver/resolver.rs:6231-6233: `info.enclosing_package_json =… | nit — the mechanism is…
| let log = unsafe { ctx.log() }; | ||
| let has_log_errors = log.has_errors(); | ||
| if has_log_errors { | ||
| let _ = log.print(std::ptr::from_mut::<bun_core::io::Writer>( | ||
| Output::error_writer(), | ||
| )); | ||
| } |
There was a problem hiding this comment.
🟣 Pre-existing sibling left unfixed: the new ctx.log() dump is placed after the --if-present early return at line 2658, so bun run --if-present <script> with a broken package.json still exits 0 with no output — the parse error stays swallowed exactly as on the base branch. Fix: dump ctx.log() before the if_present check (or inside it) so every not-found exit surfaces the resolver's package.json error, whether or not --if-present suppresses the "Script not found" line and the nonzero exit.
Extended reasoning...
Run::exec reaches the failure section: line 2657 checks ctx.runtime_options.if_present and returns Ok(true) before line 2668 reads ctx.log(). With printf '{"scripts":{"s":"x"' > package.json && bun run --if-present s, the resolver logs the parse error into ctx.log and returns a DirInfo with enclosing_package_json = None; no script matches; if_present returns success and the process exits 0 with empty stderr. On base this path also swallowed the error, so this is a same-class exit the partial fix leaves behind — the PR's stated goal ("print the package.json parse error") does not reach --if-present users, who are the ones most likely to miss that their scripts silently stopped running. Hoisting the let log = …; if log.has_errors() { log.print(...) } block above line 2657 covers both exits with no behavior change for the tested paths.
Verification: pre-existing — same-class sibling exit left uncovered by the fix. At src/runtime/cli/run_command.rs:2657-2659: if ctx.runtime_options.if_present { return Ok(true); } returns before the newly added log dump at 2668-2674 (let log = unsafe { ctx.log() }; ... log.print(...)). The failure scenario is exactly as stated: with a malformed package.json, the resolver records the parse…
|
This also fixes #42992, which reports the same "Script not found" output for a package.json that does not parse. |
Problem
package.jsondoes not parse,bun run <script>prints onlyerror: Script not found "<script>"and exits 1. The parse error thatbun -e,bun build, andbun installprint for the same file never appears.npm runprintsEJSONPARSEand names the file.Run::exec(src/runtime/cli/run_command.rs:2661). The resolver records the parse error inctx.logand returns aDirInfowith nopackage_json. The script lookup sees no scripts and prints "Script not found" without that log.Fix
ctx.logto stderr when it holds errors. This is the same dumpconfigure_env_for_runandboot_failed_exitalready use in this file.enclosing_package_jsonisNone) and the log explains why, skip "Script not found". The parse error is the message. A missing script in a file that did not load is not a fact bun can report.test/cli/run/run_command.test.ts(two new tests, stock bun fails both). Also rantest/cli/run/if-present.test.ts.Background
Run::execcallsconfigure_env_for_run, which asks the resolver for the cwdDirInfo. The resolver walks up the directory tree and parses eachpackage.jsonit finds. A parse failure goes to the resolver's log (PackageJSON::parseinsrc/resolver/package_json.rs:461) and the directory is treated as having no package.json.ctx.logis the process-lifetimebun_ast::Logthat the resolver writes to. Other exits inrun_command.rsprint it before their own error line. The not-found exit did not.Notes
Reproduction with stock bun (1.4.3):
With this change,
bun run sprints the parse error with the file path and line:column, and exits 1.bun run ./missing.jswith the same broken package.json prints the parse error and thenModule not found "./missing.js". That message is about the file argument, so it stays.Not covered here: a
package.jsonthat is a directory. The resolver only parses regular files, so nothing is logged and the output staysScript not found. Also not covered:bun buildfailing on a broken ancestor package.json when the project's own file is valid. That is a resolver-level decision about ancestor manifests and is separate from this exit path.no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/cli/run/run_command.test.ts