Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions src/install/PackageManager/CommandLineArguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1054,8 +1054,8 @@ Full documentation is available at <magenta>https://bun.com/docs/cli/pm#scan<r>.
Some(l) => l,
None => {
Output::err_generic(
"Expected --linker to be one of 'isolated' or 'hoisted'",
(),
"Invalid value for --linker: {}. Must be 'isolated' or 'hoisted'.",
(bun_core::fmt::quote(linker),),
);
Global::exit(1);
}
Expand Down Expand Up @@ -1382,12 +1382,27 @@ Full documentation is available at <magenta>https://bun.com/docs/cli/pm#scan<r>.
}

if subcommand == Subcommand::Patch && cli.positionals.len() < 2 {
Output::err_generic("Missing pkg to patch\n", ());
if let PatchOpts::Commit { .. } = cli.patch {
Output::err_generic(
"Missing path to the package directory containing your changes.\n <d>Usage:<r> bun patch --commit <cyan>node_modules/\\<package\\><r>",
(),
);
} else {
Output::err_generic(
"Missing package name to patch.\n <d>Usage:<r> bun patch <cyan>\\<package\\><r><d>[@\\<version\\>]<r>",
(),
);
}
bun_core::note!("Run 'bun patch --help' for more information");
Global::crash();
}

if subcommand == Subcommand::PatchCommit && cli.positionals.len() < 2 {
Output::err_generic("Missing pkg folder to patch\n", ());
Output::err_generic(
"Missing path to the package directory containing your changes.\n <d>Usage:<r> bun patch-commit <cyan>node_modules/\\<package\\><r>",
(),
);
bun_core::note!("Run 'bun patch-commit --help' for more information");
Global::crash();
}

Expand Down
5 changes: 4 additions & 1 deletion src/runtime/cli/Arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2282,7 +2282,10 @@ fn parse_build_command_options(

if let Some(format_str) = args.option(b"--format") {
let Some(format) = options::Format::from_string(format_str) else {
Output::err_generic("Invalid format - must be esm, cjs, or iife", ());
Output::err_generic(
"Invalid value for --format: {}. Must be 'esm', 'cjs', or 'iife'.",
(bun_core::fmt::quote(format_str),),
);
Global::crash();
};

Expand Down
22 changes: 13 additions & 9 deletions src/runtime/cli/colon_list_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,18 @@ impl<T: ColonListValue> ColonListType<T> {
.unwrap_or(u32::MAX)
.min(strings::index_of_char(str, b'=').unwrap_or(u32::MAX));
if midpoint == u32::MAX {
return Err(err!("InvalidSeparator"));
if T::IS_LOADER {
pretty_errorln!(
"<r><red>error<r><d>:<r> <b>--loader {}<r> is missing a \":\" separator. Expected <cyan>--loader .ext:loader<r>, for example <cyan>--loader .md:text<r>",
bun_fmt::quote(str),
);
} else {
pretty_errorln!(
"<r><red>error<r><d>:<r> <b>--define {}<r> is missing a \":\" or \"=\" separator. Expected <cyan>--define key=value<r>, for example <cyan>--define process.env.NODE_ENV='\"production\"'<r>",
bun_fmt::quote(str),
);
}
Global::exit(1);
}
let midpoint = midpoint as usize;

Expand Down Expand Up @@ -72,14 +83,7 @@ impl<T: ColonListValue> ColonListType<T> {

pub(crate) fn resolve(input: &[&'static [u8]]) -> Result<Self, Error> {
let mut list = Self::init(input.len());
match list.load(input) {
Ok(()) => {}
Err(e) if e == err!("InvalidSeparator") => {
pretty_errorln!("<r><red>error<r><d>:<r> expected \":\" separator");
Global::exit(1);
}
Err(e) => return Err(e),
}
list.load(input)?;
Ok(list)
}
}
5 changes: 4 additions & 1 deletion src/runtime/cli/filter_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -765,7 +765,10 @@ pub(crate) fn run_scripts_with_filter(
None,
IncludeScripts::IncludeScripts,
) else {
bun_core::warn!("Failed to read package.json\n");
bun_core::warn!(
"Failed to read {}, skipping this workspace package\n",
bun_core::fmt::quote(&*package_json_path),
);
continue;
};
let Some(pkgscripts) = &pkgjson.scripts else {
Expand Down
52 changes: 52 additions & 0 deletions test/bundler/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -466,3 +466,55 @@ test("multi-entry build writes each entry point into the output directory", asyn
expect(a).toContain('"A"');
expect(b).toContain('"B"');
});

describe("CLI argument error messages", () => {
test("--format with an unrecognized value echoes the value back", async () => {
using dir = tempDir("build-format-err", { "in.js": "console.log(1)" });
await using proc = Bun.spawn({
cmd: [bunExe(), "build", "--format=commonjs", "in.js"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect({ stdout, stderr }).toEqual({
stdout: "",
stderr: expect.stringContaining('--format: "commonjs"'),
});
expect(stderr).toContain("'esm', 'cjs', or 'iife'");
expect(exitCode).toBe(1);
});

test("--loader without a ':' separator names the flag and the bad token", async () => {
using dir = tempDir("build-loader-err", { "in.js": "console.log(1)" });
await using proc = Bun.spawn({
cmd: [bunExe(), "build", "--loader", "text", "in.js"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toContain("--loader");
expect(stderr).toContain('"text"');
expect(stderr).toContain(".ext:loader");
expect(exitCode).toBe(1);
});

test("--define without a separator names the flag and shows an example", async () => {
using dir = tempDir("build-define-err", { "in.js": "console.log(FOO)" });
await using proc = Bun.spawn({
cmd: [bunExe(), "build", "--define", "FOO", "in.js"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toContain("--define");
expect(stderr).toContain('"FOO"');
expect(stderr).toContain("key=value");
expect(exitCode).toBe(1);
});
});
56 changes: 56 additions & 0 deletions test/cli/install/bun-patch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,62 @@ const platformPath = (path: string) => path;

setDefaultTimeout(1000 * 60 * 5);

describe("error messages", () => {
test("'bun patch' with no package name shows a usage example", async () => {
const dir = tempDirWithFiles("bun-patch-noarg", {
"package.json": JSON.stringify({ name: "t" }),
});
await using proc = Bun.spawn({
cmd: [bunExe(), "patch"],
env: bunEnv,
cwd: dir,
stdout: "pipe",
stderr: "pipe",
});
const [, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toContain("Missing package name to patch");
expect(stderr).toContain("bun patch <package>");
expect(stderr).toContain("bun patch --help");
expect(exitCode).toBe(1);
});

test("'bun patch --commit' with no directory shows a usage example", async () => {
const dir = tempDirWithFiles("bun-patch-commit-noarg", {
"package.json": JSON.stringify({ name: "t" }),
});
await using proc = Bun.spawn({
cmd: [bunExe(), "patch", "--commit"],
env: bunEnv,
cwd: dir,
stdout: "pipe",
stderr: "pipe",
});
const [, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toContain("Missing path to the package directory");
expect(stderr).toContain("bun patch --commit node_modules/<package>");
expect(stderr).toContain("bun patch --help");
expect(exitCode).toBe(1);
});

test("'bun patch-commit' with no directory shows a usage example", async () => {
const dir = tempDirWithFiles("bun-patchcommit-noarg", {
"package.json": JSON.stringify({ name: "t" }),
});
await using proc = Bun.spawn({
cmd: [bunExe(), "patch-commit"],
env: bunEnv,
cwd: dir,
stdout: "pipe",
stderr: "pipe",
});
const [, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toContain("Missing path to the package directory");
expect(stderr).toContain("bun patch-commit node_modules/<package>");
expect(stderr).toContain("bun patch-commit --help");
expect(exitCode).toBe(1);
});
});

describe("bun patch <pkg>", async () => {
describe("workspace interactions", async () => {
/**
Expand Down
17 changes: 17 additions & 0 deletions test/cli/install/isolated-install.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2026,3 +2026,20 @@ test("rejects dependency aliases that traverse outside node_modules", async () =
expect(() => lstatSync(join(packageDir, "pwned-by-alias"))).toThrow();
expect(exitCode).not.toBe(0);
});

test("invalid --linker value is echoed back in the error", async () => {
using dir = tempDir("install-linker-err", {
"package.json": JSON.stringify({ name: "t" }),
});
await using proc = spawn({
cmd: [bunExe(), "install", "--linker=isoalted"],
cwd: String(dir),
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toContain('--linker: "isoalted"');
expect(stderr).toContain("'isolated' or 'hoisted'");
expect(exitCode).toBe(1);
});
32 changes: 31 additions & 1 deletion test/cli/run/filter-workspace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ describe("bun", () => {
runInCwdFailure(cwd_root, "*", "notpresent", /No packages matched/);
});
test("should warn about malformed package.json", () => {
runInCwdFailure(cwd_root, "*", "x", /Failed to read package.json/);
runInCwdFailure(cwd_root, "*", "x", /Failed to read .*malformed2.*package\.json/);
});
test("nonzero exit code on failure", () => {
const dir = tempDirWithFiles("testworkspace", {
Expand Down Expand Up @@ -590,4 +590,34 @@ describe("bun", () => {
expect(stdoutval).toMatch(/(?:log_line[\s\S]*?){20}/);
expect(exitCode).toBe(0);
});

test("warning names which package.json failed to parse", async () => {
const dir = tempDirWithFiles("filter-bad-pkgjson", {
packages: {
good: {
"package.json": JSON.stringify({ name: "good", scripts: { go: "echo ok" } }),
},
broken: {
"package.json": "this is { not valid json",
},
},
"package.json": JSON.stringify({ name: "ws", workspaces: ["packages/*"] }),
});

await using proc = Bun.spawn({
cmd: [bunExe(), "run", "--filter", "*", "go"],
cwd: dir,
env: { ...bunEnv, NO_COLOR: "1" },
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
// The good package still runs; the broken one is skipped with a warning
// that names its path.
expect(stdout).toContain("ok");
const sep = process.platform === "win32" ? "\\" : "/";
expect(stderr).toContain(`broken${sep}package.json`);
expect(stderr).toContain("skipping this workspace package");
Comment thread
robobun marked this conversation as resolved.
expect(exitCode).toBe(0);
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});
Loading