Skip to content
Merged
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
13 changes: 13 additions & 0 deletions .changeset/improve-error-messages-cli-types-auth-dev-compat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
"wrangler": patch
---

Improve error messages for CLI flags, type generation, auth scopes, dev server tunnels, and compatibility flags

Error messages across several areas now name the exact flags or values involved and suggest how to fix the problem:

- KV commands (`kv key put`, `kv key get`, `kv key delete`): error messages now include `--` prefixes and clear "Missing required option" / "Conflicting options" phrasing instead of the vague "Exactly one of the arguments ... is required".
- `wrangler types --include-env=false --include-runtime=false`: the error now names both flags and explains what each does.
- `wrangler login --scopes`: invalid scopes are individually identified instead of dumping the entire array.
- `wrangler dev --tunnel --remote`: the error now explains why tunnels require local mode and suggests two concrete fixes.
- Conflicting compatibility flags (`nodejs_compat_populate_process_env` / `nodejs_compat_do_not_populate_process_env`, `global_navigator` / `no_global_navigator`): errors now name the specific conflicting flags.
7 changes: 7 additions & 0 deletions .changeset/improve-hyperdrive-errors.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"wrangler": patch
---

Improve Hyperdrive error messages for missing required options

Error messages thrown when creating or updating a Hyperdrive config with missing individual parameters (e.g. `--origin-host`, `--origin-port`, `--database`, `--origin-user`, `--origin-password`, `--origin-scheme`, `--access-client-id`/`--access-client-secret`) now clearly state which option is missing, provide a usage example, and suggest `--connection-string` as an alternative where applicable.
2 changes: 1 addition & 1 deletion packages/wrangler/src/__tests__/dev.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3077,7 +3077,7 @@ describe.sequential("wrangler dev", () => {
await expect(
runWrangler("dev --tunnel --remote")
).rejects.toThrowErrorMatchingInlineSnapshot(
`[Error: --tunnel is only supported in local mode.]`
`[Error: --tunnel cannot be used with --remote. Tunnels expose your local dev server to the internet, which is only applicable in local mode. Remove --remote to use --tunnel, or remove --tunnel to use --remote.]`
);
});
});
Expand Down
35 changes: 28 additions & 7 deletions packages/wrangler/src/__tests__/hyperdrive.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ describe("hyperdrive commands", () => {
)
).rejects.toThrow();
expect(std.err).toMatchInlineSnapshot(`
"X [ERROR] You must provide an origin hostname for the database
"X [ERROR] Missing required option --origin-host. Specify the hostname of the origin database, e.g. --origin-host=database.example.com.

"
`);
Expand Down Expand Up @@ -861,7 +861,7 @@ describe("hyperdrive commands", () => {
)
).rejects.toThrow();
expect(std.err).toMatchInlineSnapshot(`
"X [ERROR] You must provide both an Access Client ID and Access Client Secret when configuring Hyperdrive-over-Access
"X [ERROR] Missing required option --access-client-id or --access-client-secret. Both --access-client-id and --access-client-secret must be provided together when configuring Hyperdrive-over-Access.

"
`);
Expand Down Expand Up @@ -1437,7 +1437,28 @@ describe("hyperdrive commands", () => {
)
).rejects.toThrow();
expect(std.err).toMatchInlineSnapshot(`
"X [ERROR] You must provide a password for the origin database
"X [ERROR] Missing required option --origin-password. Specify the password for the origin database, e.g. --origin-password=mypassword. Alternatively, use --connection-string to provide all origin details at once.

"
`);
expect(std.out).toMatchInlineSnapshot(`
"
⛅️ wrangler x.x.x
──────────────────
"
`);
});

it("should throw an exception when creating a hyperdrive config without network origin options", async ({
expect,
}) => {
await expect(() =>
runWrangler(
"hyperdrive create test123 --database=mydb --origin-user=newuser --origin-password=mypassword"
)
).rejects.toThrow();
expect(std.err).toMatchInlineSnapshot(`
"X [ERROR] Missing required network origin options. Provide the origin host and port via --origin-host and --origin-port, a Workers VPC Service ID via --service-id, or use --connection-string to provide all origin details at once.

"
`);
Expand All @@ -1459,7 +1480,7 @@ describe("hyperdrive commands", () => {
)
).rejects.toThrow();
expect(std.err).toMatchInlineSnapshot(`
"X [ERROR] You must provide an origin hostname for the database
"X [ERROR] Missing required option --origin-host. Specify the hostname of the origin database, e.g. --origin-host=database.example.com.

"
`);
Expand Down Expand Up @@ -1735,7 +1756,7 @@ describe("hyperdrive commands", () => {
)
).rejects.toThrow();
expect(std.err).toMatchInlineSnapshot(`
"X [ERROR] You must provide a nonzero origin port for the database
"X [ERROR] Missing required option --origin-port. Specify the port of the origin database, e.g. --origin-port=5432.

"
`);
Expand All @@ -1757,7 +1778,7 @@ describe("hyperdrive commands", () => {
)
).rejects.toThrow();
expect(std.err).toMatchInlineSnapshot(`
"X [ERROR] You must provide an origin hostname for the database
"X [ERROR] Missing required option --origin-host. Specify the hostname of the origin database, e.g. --origin-host=database.example.com.

"
`);
Expand Down Expand Up @@ -1795,7 +1816,7 @@ describe("hyperdrive commands", () => {
)
).rejects.toThrow();
expect(std.err).toMatchInlineSnapshot(`
"X [ERROR] You must provide both an Access Client ID and Access Client Secret when configuring Hyperdrive-over-Access
"X [ERROR] Missing required option --access-client-id or --access-client-secret. Both --access-client-id and --access-client-secret must be provided together when configuring Hyperdrive-over-Access.

"
`);
Expand Down
24 changes: 12 additions & 12 deletions packages/wrangler/src/__tests__/kv/key.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ describe("kv", () => {
await expect(
runWrangler("kv key put --remote foo bar")
).rejects.toThrowErrorMatchingInlineSnapshot(
`[Error: Exactly one of the arguments binding and namespace-id is required]`
`[Error: Missing required option: exactly one of --binding and --namespace-id must be provided]`
);

expect(std.out).toMatchInlineSnapshot(`
Expand Down Expand Up @@ -479,7 +479,7 @@ describe("kv", () => {
--persist-to Directory for local persistence [string]"
`);
expect(std.err).toMatchInlineSnapshot(`
"X [ERROR] Exactly one of the arguments binding and namespace-id is required
"X [ERROR] Missing required option: exactly one of --binding and --namespace-id must be provided

"
`);
Expand All @@ -493,7 +493,7 @@ describe("kv", () => {
"kv key put --remote foo bar --binding x --namespace-id y"
)
).rejects.toThrowErrorMatchingInlineSnapshot(
`[Error: Arguments binding and namespace-id are mutually exclusive]`
`[Error: Conflicting options: --binding and --namespace-id cannot be used together. Please provide only one.]`
);

expect(std.out).toMatchInlineSnapshot(`
Expand Down Expand Up @@ -531,7 +531,7 @@ describe("kv", () => {
--persist-to Directory for local persistence [string]"
`);
expect(std.err).toMatchInlineSnapshot(`
"X [ERROR] Arguments binding and namespace-id are mutually exclusive
"X [ERROR] Conflicting options: --binding and --namespace-id cannot be used together. Please provide only one.

"
`);
Expand All @@ -543,7 +543,7 @@ describe("kv", () => {
await expect(
runWrangler("kv key put --remote key --namespace-id 12345")
).rejects.toThrowErrorMatchingInlineSnapshot(
`[Error: Exactly one of the arguments value and path is required]`
`[Error: Missing required option: exactly one of <value> and --path must be provided]`
);

expect(std.out).toMatchInlineSnapshot(`
Expand Down Expand Up @@ -581,7 +581,7 @@ describe("kv", () => {
--persist-to Directory for local persistence [string]"
`);
expect(std.err).toMatchInlineSnapshot(`
"X [ERROR] Exactly one of the arguments value and path is required
"X [ERROR] Missing required option: exactly one of <value> and --path must be provided

"
`);
Expand Down Expand Up @@ -642,7 +642,7 @@ describe("kv", () => {
"kv key put --remote key value --path xyz --namespace-id 12345"
)
).rejects.toThrowErrorMatchingInlineSnapshot(
`[Error: Arguments value and path are mutually exclusive]`
`[Error: Conflicting options: <value> and --path cannot be used together. Please provide only one.]`
);

expect(std.out).toMatchInlineSnapshot(`
Expand Down Expand Up @@ -680,7 +680,7 @@ describe("kv", () => {
--persist-to Directory for local persistence [string]"
`);
expect(std.err).toMatchInlineSnapshot(`
"X [ERROR] Arguments value and path are mutually exclusive
"X [ERROR] Conflicting options: <value> and --path cannot be used together. Please provide only one.

"
`);
Expand Down Expand Up @@ -1133,7 +1133,7 @@ describe("kv", () => {
await expect(
runWrangler("kv key get --remote foo")
).rejects.toThrowErrorMatchingInlineSnapshot(
`[Error: Exactly one of the arguments binding and namespace-id is required]`
`[Error: Missing required option: exactly one of --binding and --namespace-id must be provided]`
);
expect(std.out).toMatchInlineSnapshot(`
"
Expand Down Expand Up @@ -1163,7 +1163,7 @@ describe("kv", () => {
--persist-to Directory for local persistence [string]"
`);
expect(std.err).toMatchInlineSnapshot(`
"X [ERROR] Exactly one of the arguments binding and namespace-id is required
"X [ERROR] Missing required option: exactly one of --binding and --namespace-id must be provided

"
`);
Expand All @@ -1175,7 +1175,7 @@ describe("kv", () => {
await expect(
runWrangler("kv key get --remote foo --binding x --namespace-id y")
).rejects.toThrowErrorMatchingInlineSnapshot(
`[Error: Arguments binding and namespace-id are mutually exclusive]`
`[Error: Conflicting options: --binding and --namespace-id cannot be used together. Please provide only one.]`
);

expect(std.out).toMatchInlineSnapshot(`
Expand Down Expand Up @@ -1206,7 +1206,7 @@ describe("kv", () => {
--persist-to Directory for local persistence [string]"
`);
expect(std.err).toMatchInlineSnapshot(`
"X [ERROR] Arguments binding and namespace-id are mutually exclusive
"X [ERROR] Conflicting options: --binding and --namespace-id cannot be used together. Please provide only one.

"
`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ describe("isNavigatorDefined", () => {
assert(false, "Unreachable");
} catch (e) {
expect(e).toMatchInlineSnapshot(
`[Error: Can't both enable and disable a flag]`
`[Error: Conflicting compatibility flags: "global_navigator" and "no_global_navigator" cannot both be set. Remove one of these flags from your configuration.]`
);
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ describe("isProcessEnvPopulated", () => {
assert(false, "Unreachable");
} catch (e) {
expect(e).toMatchInlineSnapshot(
`[Error: Can't both enable and disable a flag]`
`[Error: Conflicting compatibility flags: "nodejs_compat_populate_process_env" and "nodejs_compat_do_not_populate_process_env" cannot both be set. Remove one of these flags from your configuration.]`
);
}
});
Expand Down
20 changes: 19 additions & 1 deletion packages/wrangler/src/__tests__/type-generation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3413,6 +3413,24 @@ describe("generate types - CLI", () => {
);
}
});

it("should error if both --include-env and --include-runtime are false", async ({
expect,
}) => {
fs.writeFileSync(
"./wrangler.jsonc",
JSON.stringify({
vars: bindingsConfigMock.vars,
}),
"utf-8"
);

await expect(
runWrangler("types --include-env=false --include-runtime=false")
).rejects.toThrowError(
"At least one of --include-env or --include-runtime must be enabled."
);
});
});

it("should allow multiple customizations to be applied together", async ({
Expand Down Expand Up @@ -3897,7 +3915,7 @@ describe("generate types - API", () => {
includeRuntime: false,
})
).rejects.toThrow(
"You cannot run this command without including either Env or Runtime types"
"At least one of includeEnv or includeRuntime must be enabled."
);

await expect(
Expand Down
20 changes: 20 additions & 0 deletions packages/wrangler/src/__tests__/user.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,26 @@ describe("User", () => {
);
});
});

it("should error if --scopes contains an invalid scope", async ({
expect,
}) => {
await expect(
runWrangler("login --scopes account:read bogus_scope")
).rejects.toThrowErrorMatchingInlineSnapshot(
`[Error: Invalid authentication scope: "bogus_scope". Run "wrangler login --scopes-list" to see all valid scopes.]`
);
});

it("should error if --scopes contains multiple invalid scopes", async ({
expect,
}) => {
await expect(
runWrangler("login --scopes bad_one bad_two")
).rejects.toThrowErrorMatchingInlineSnapshot(
`[Error: Invalid authentication scopes: "bad_one", "bad_two". Run "wrangler login --scopes-list" to see all valid scopes.]`
);
});
});

it("should handle errors for failed token refresh in a non-interactive environment", async ({
Expand Down
48 changes: 40 additions & 8 deletions packages/wrangler/src/core/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,59 @@ import type {
NamespaceDefinition,
} from "./types";

/**
* Formats an argument name for display in error messages.
* Positional arguments are shown as `<name>`, flags as `--name`.
*
* @param name - The argument name
* @param positionalArgs - Optional set of argument names that are positional (not flags)
* @returns The formatted argument name
*/
function formatArgName(
name: string,
positionalArgs?: ReadonlySet<string>
): string {
return positionalArgs?.has(name) ? `<${name}>` : `--${name}`;
}

/** Formats a list of items as a human-readable conjunction (e.g. "a, b, and c"). */
const listFormatter = new Intl.ListFormat("en-US");

/**
* A helper to demand one of a set of options
* via https://github.com/yargs/yargs/issues/1093#issuecomment-491299261
*
* @param options - The option names to demand exactly one of
* @param def - Optional command definition used to distinguish positional arguments
* from flags in error messages (positional args are shown as `<name>` instead of `--name`).
* @returns A validation function that checks the argv object
*/
export function demandOneOfOption(...options: string[]) {
export function demandOneOfOption(
options: string[],
def?: { positionalArgs?: ReadonlyArray<string> }
) {
const positionalArgs = def?.positionalArgs
? new Set(def.positionalArgs)
: undefined;

return function (argv: { [key: string]: unknown }) {
const count = options.filter((option) => argv[option]).length;
const lastOption = options.pop();
const flagList = listFormatter.format(
options.map((o) => formatArgName(o, positionalArgs))
);

if (count === 0) {
throw new CommandLineArgsError(
`Exactly one of the arguments ${options.join(
", "
)} and ${lastOption} is required`,
`Missing required option: exactly one of ${flagList} must be provided`,
{ telemetryMessage: "core arguments missing exclusive option" }
);
} else if (count > 1) {
const provided = options
.filter((option) => argv[option])
.map((o) => formatArgName(o, positionalArgs));
const providedList = listFormatter.format(provided);
throw new CommandLineArgsError(
`Arguments ${options.join(
", "
)} and ${lastOption} are mutually exclusive`,
`Conflicting options: ${providedList} cannot be used together. Please provide only one.`,
{ telemetryMessage: "core arguments mutually exclusive options" }
);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/wrangler/src/core/register-yargs-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ function createHandler(def: InternalCommandDefinition, argv: string[]) {
}
}

await def.validateArgs?.(args);
await def.validateArgs?.(args, def);

const shouldPrintResourceLocation =
typeof def.behaviour?.printResourceLocation === "function"
Expand Down
8 changes: 7 additions & 1 deletion packages/wrangler/src/core/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,8 +267,14 @@ export type CommandDefinition<
* A hook to implement custom validation of the args before the handler is called.
* Throw `CommandLineArgsError` with actionable error message if args are invalid.
* The return value is ignored.
*
* @param args - The parsed CLI arguments
* @param def - The command definition, useful for passing to helpers like `demandOneOfOption`
*/
validateArgs?: (args: HandlerArgs<NamedArgDefs>) => void | Promise<void>;
validateArgs?: (
args: HandlerArgs<NamedArgDefs>,
def: CommandDefinition<NamedArgDefs>
) => void | Promise<void>;

/**
* The implementation of the command which is given camelCase'd args
Expand Down
Loading
Loading