From 233eef2081d093b08ec02e68445c5e9c26ebe58c Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Wed, 18 May 2022 14:07:12 +0100 Subject: [PATCH] fix: display the correct help information when a subcommand is invalid (#1052) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, when an invalid subcommand was used, such as `wrangler r2 foo`, the help that was displayed showed the top-level commands prefixed by the command in used. E.g. ``` wrangler r2 init [name] 📥 Create a wrangler.toml configuration file wrangler r2 dev [script] 👂 Start a local server for developing your worker wrangler r2 publish [script] 🆙 Publish your Worker to Cloudflare. ... ``` Now the correct command help is displayed: ``` $ wrangler r2 foo ✘ [ERROR] Unknown argument: foo wrangler r2 📦 Interact with an R2 store Commands: wrangler r2 bucket Manage R2 buckets Flags: -c, --config Path to .toml configuration file [string] -h, --help Show help [boolean] -v, --version Show version number [boolean] ``` Fixes #871 --- .changeset/heavy-rabbits-accept.md | 39 +++++ packages/wrangler/src/__tests__/dev.test.tsx | 13 +- packages/wrangler/src/__tests__/index.test.ts | 11 +- packages/wrangler/src/__tests__/kv.test.ts | 137 +++++++----------- packages/wrangler/src/__tests__/r2.test.ts | 71 ++++++--- packages/wrangler/src/index.tsx | 13 +- 6 files changed, 163 insertions(+), 121 deletions(-) create mode 100644 .changeset/heavy-rabbits-accept.md diff --git a/.changeset/heavy-rabbits-accept.md b/.changeset/heavy-rabbits-accept.md new file mode 100644 index 000000000000..167293cef1d0 --- /dev/null +++ b/.changeset/heavy-rabbits-accept.md @@ -0,0 +1,39 @@ +--- +"wrangler": patch +--- + +fix: display the correct help information when a subcommand is invalid + +Previously, when an invalid subcommand was used, such as `wrangler r2 foo`, +the help that was displayed showed the top-level commands prefixed by the command in used. +E.g. + +``` +wrangler r2 init [name] 📥 Create a wrangler.toml configuration file +wrangler r2 dev [script] 👂 Start a local server for developing your worker +wrangler r2 publish [script] 🆙 Publish your Worker to Cloudflare. +... +``` + +Now the correct command help is displayed: + +``` +$ wrangler r2 foo + +✘ [ERROR] Unknown argument: foo + + +wrangler r2 + +📦 Interact with an R2 store + +Commands: + wrangler r2 bucket Manage R2 buckets + +Flags: + -c, --config Path to .toml configuration file [string] + -h, --help Show help [boolean] + -v, --version Show version number [boolean] +``` + +Fixes #871 diff --git a/packages/wrangler/src/__tests__/dev.test.tsx b/packages/wrangler/src/__tests__/dev.test.tsx index 6f217fd7f0e2..52f4efe945a0 100644 --- a/packages/wrangler/src/__tests__/dev.test.tsx +++ b/packages/wrangler/src/__tests__/dev.test.tsx @@ -774,7 +774,11 @@ describe("wrangler dev", () => { expect(std).toMatchInlineSnapshot(` Object { "debug": "", - "err": "wrangler dev [script] + "err": "X [ERROR] Not enough arguments following: site + + ", + "out": " + wrangler dev [script] 👂 Start a local server for developing your worker @@ -811,12 +815,7 @@ describe("wrangler dev", () => { --minify Minify the script [boolean] --node-compat Enable node.js compatibility [boolean] --experimental-enable-local-persistence Enable persistence for this session (only for local mode) [boolean] - --inspect Enable dev tools [deprecated] [boolean] - X [ERROR] Not enough arguments following: site - - ", - "out": " - ", + --inspect Enable dev tools [deprecated] [boolean]", "warn": "", } `); diff --git a/packages/wrangler/src/__tests__/index.test.ts b/packages/wrangler/src/__tests__/index.test.ts index 3a867b2fa178..7c527a641de1 100644 --- a/packages/wrangler/src/__tests__/index.test.ts +++ b/packages/wrangler/src/__tests__/index.test.ts @@ -63,10 +63,7 @@ describe("wrangler", () => { expect(std.out).toMatchInlineSnapshot(` " - " - `); - expect(std.err).toMatchInlineSnapshot(` - "wrangler + wrangler Commands: wrangler init [name] 📥 Create a wrangler.toml configuration file @@ -86,8 +83,10 @@ describe("wrangler", () => { Flags: -c, --config Path to .toml configuration file [string] -h, --help Show help [boolean] - -v, --version Show version number [boolean] - X [ERROR] Unknown argument: invalid-command + -v, --version Show version number [boolean]" + `); + expect(std.err).toMatchInlineSnapshot(` + "X [ERROR] Unknown argument: invalid-command " `); diff --git a/packages/wrangler/src/__tests__/kv.test.ts b/packages/wrangler/src/__tests__/kv.test.ts index b95a18c74d7a..46cc5e2f646b 100644 --- a/packages/wrangler/src/__tests__/kv.test.ts +++ b/packages/wrangler/src/__tests__/kv.test.ts @@ -48,10 +48,7 @@ describe("wrangler", () => { ); expect(std.out).toMatchInlineSnapshot(` " - " - `); - expect(std.err).toMatchInlineSnapshot(` - "wrangler kv:namespace create + wrangler kv:namespace create Create a new namespace @@ -65,8 +62,10 @@ describe("wrangler", () => { Options: -e, --env Perform on a specific environment [string] - --preview Interact with a preview namespace [boolean] - X [ERROR] Not enough non-option arguments: got 0, need at least 1 + --preview Interact with a preview namespace [boolean]" + `); + expect(std.err).toMatchInlineSnapshot(` + "X [ERROR] Not enough non-option arguments: got 0, need at least 1 " `); @@ -80,10 +79,7 @@ describe("wrangler", () => { ); expect(std.out).toMatchInlineSnapshot(` " - " - `); - expect(std.err).toMatchInlineSnapshot(` - "wrangler kv:namespace create + wrangler kv:namespace create Create a new namespace @@ -97,8 +93,10 @@ describe("wrangler", () => { Options: -e, --env Perform on a specific environment [string] - --preview Interact with a preview namespace [boolean] - X [ERROR] Unknown arguments: def, ghi + --preview Interact with a preview namespace [boolean]" + `); + expect(std.err).toMatchInlineSnapshot(` + "X [ERROR] Unknown arguments: def, ghi " `); @@ -113,10 +111,7 @@ describe("wrangler", () => { expect(std.out).toMatchInlineSnapshot(` " - " - `); - expect(std.err).toMatchInlineSnapshot(` - "wrangler kv:namespace create + wrangler kv:namespace create Create a new namespace @@ -130,8 +125,10 @@ describe("wrangler", () => { Options: -e, --env Perform on a specific environment [string] - --preview Interact with a preview namespace [boolean] - X [ERROR] The namespace binding name \\"abc-def\\" is invalid. It can only have alphanumeric and _ characters, and cannot begin with a number. + --preview Interact with a preview namespace [boolean]" + `); + expect(std.err).toMatchInlineSnapshot(` + "X [ERROR] The namespace binding name \\"abc-def\\" is invalid. It can only have alphanumeric and _ characters, and cannot begin with a number. " `); @@ -283,21 +280,7 @@ describe("wrangler", () => { A namespace with binding name \\"otherBinding\\" was not found in the configured \\"kv_namespaces\\"." `); expect(std.err).toMatchInlineSnapshot(` - "wrangler kv:namespace delete - - Deletes a given namespace. - - Flags: - -c, --config Path to .toml configuration file [string] - -h, --help Show help [boolean] - -v, --version Show version number [boolean] - - Options: - --binding The name of the namespace to delete [string] - --namespace-id The id of the namespace to delete [string] - -e, --env Perform on a specific environment [string] - --preview Interact with a preview namespace [boolean] - X [ERROR] Not able to delete namespace. + "X [ERROR] Not able to delete namespace. A namespace with binding name \\"otherBinding\\" was not found in the configured \\"kv_namespaces\\". @@ -489,10 +472,7 @@ describe("wrangler", () => { expect(std.out).toMatchInlineSnapshot(` " - " - `); - expect(std.err).toMatchInlineSnapshot(` - "wrangler kv:key put [value] + wrangler kv:key put [value] Writes a single key/value pair to the given namespace. @@ -512,8 +492,10 @@ describe("wrangler", () => { --preview Interact with a preview namespace [boolean] --ttl Time for which the entries should be visible [number] --expiration Time since the UNIX epoch after which the entry expires [number] - --path Read value from the file at a given path [string] - X [ERROR] Not enough non-option arguments: got 0, need at least 1 + --path Read value from the file at a given path [string]" + `); + expect(std.err).toMatchInlineSnapshot(` + "X [ERROR] Not enough non-option arguments: got 0, need at least 1 " `); @@ -528,10 +510,7 @@ describe("wrangler", () => { expect(std.out).toMatchInlineSnapshot(` " - " - `); - expect(std.err).toMatchInlineSnapshot(` - "wrangler kv:key put [value] + wrangler kv:key put [value] Writes a single key/value pair to the given namespace. @@ -551,8 +530,10 @@ describe("wrangler", () => { --preview Interact with a preview namespace [boolean] --ttl Time for which the entries should be visible [number] --expiration Time since the UNIX epoch after which the entry expires [number] - --path Read value from the file at a given path [string] - X [ERROR] Exactly one of the arguments binding and namespace-id is required + --path Read value from the file at a given path [string]" + `); + expect(std.err).toMatchInlineSnapshot(` + "X [ERROR] Exactly one of the arguments binding and namespace-id is required " `); @@ -567,10 +548,7 @@ describe("wrangler", () => { expect(std.out).toMatchInlineSnapshot(` " - " - `); - expect(std.err).toMatchInlineSnapshot(` - "wrangler kv:key put [value] + wrangler kv:key put [value] Writes a single key/value pair to the given namespace. @@ -590,8 +568,10 @@ describe("wrangler", () => { --preview Interact with a preview namespace [boolean] --ttl Time for which the entries should be visible [number] --expiration Time since the UNIX epoch after which the entry expires [number] - --path Read value from the file at a given path [string] - X [ERROR] Arguments binding and namespace-id are mutually exclusive + --path Read value from the file at a given path [string]" + `); + expect(std.err).toMatchInlineSnapshot(` + "X [ERROR] Arguments binding and namespace-id are mutually exclusive " `); @@ -606,10 +586,7 @@ describe("wrangler", () => { expect(std.out).toMatchInlineSnapshot(` " - " - `); - expect(std.err).toMatchInlineSnapshot(` - "wrangler kv:key put [value] + wrangler kv:key put [value] Writes a single key/value pair to the given namespace. @@ -629,8 +606,10 @@ describe("wrangler", () => { --preview Interact with a preview namespace [boolean] --ttl Time for which the entries should be visible [number] --expiration Time since the UNIX epoch after which the entry expires [number] - --path Read value from the file at a given path [string] - X [ERROR] Exactly one of the arguments value and path is required + --path Read value from the file at a given path [string]" + `); + expect(std.err).toMatchInlineSnapshot(` + "X [ERROR] Exactly one of the arguments value and path is required " `); @@ -645,10 +624,7 @@ describe("wrangler", () => { expect(std.out).toMatchInlineSnapshot(` " - " - `); - expect(std.err).toMatchInlineSnapshot(` - "wrangler kv:key put [value] + wrangler kv:key put [value] Writes a single key/value pair to the given namespace. @@ -668,8 +644,10 @@ describe("wrangler", () => { --preview Interact with a preview namespace [boolean] --ttl Time for which the entries should be visible [number] --expiration Time since the UNIX epoch after which the entry expires [number] - --path Read value from the file at a given path [string] - X [ERROR] Arguments value and path are mutually exclusive + --path Read value from the file at a given path [string]" + `); + expect(std.err).toMatchInlineSnapshot(` + "X [ERROR] Arguments value and path are mutually exclusive " `); @@ -965,10 +943,7 @@ describe("wrangler", () => { ); expect(std.out).toMatchInlineSnapshot(` " - " - `); - expect(std.err).toMatchInlineSnapshot(` - "wrangler kv:key get + wrangler kv:key get Reads a single value by key from the given namespace. @@ -984,8 +959,10 @@ describe("wrangler", () => { --binding The name of the namespace to get from [string] --namespace-id The id of the namespace to get from [string] -e, --env Perform on a specific environment [string] - --preview Interact with a preview namespace [boolean] [default: false] - X [ERROR] Not enough non-option arguments: got 0, need at least 1 + --preview Interact with a preview namespace [boolean] [default: false]" + `); + expect(std.err).toMatchInlineSnapshot(` + "X [ERROR] Not enough non-option arguments: got 0, need at least 1 " `); @@ -999,10 +976,7 @@ describe("wrangler", () => { ); expect(std.out).toMatchInlineSnapshot(` " - " - `); - expect(std.err).toMatchInlineSnapshot(` - "wrangler kv:key get + wrangler kv:key get Reads a single value by key from the given namespace. @@ -1018,8 +992,10 @@ describe("wrangler", () => { --binding The name of the namespace to get from [string] --namespace-id The id of the namespace to get from [string] -e, --env Perform on a specific environment [string] - --preview Interact with a preview namespace [boolean] [default: false] - X [ERROR] Exactly one of the arguments binding and namespace-id is required + --preview Interact with a preview namespace [boolean] [default: false]" + `); + expect(std.err).toMatchInlineSnapshot(` + "X [ERROR] Exactly one of the arguments binding and namespace-id is required " `); @@ -1034,10 +1010,7 @@ describe("wrangler", () => { expect(std.out).toMatchInlineSnapshot(` " - " - `); - expect(std.err).toMatchInlineSnapshot(` - "wrangler kv:key get + wrangler kv:key get Reads a single value by key from the given namespace. @@ -1053,8 +1026,10 @@ describe("wrangler", () => { --binding The name of the namespace to get from [string] --namespace-id The id of the namespace to get from [string] -e, --env Perform on a specific environment [string] - --preview Interact with a preview namespace [boolean] [default: false] - X [ERROR] Arguments binding and namespace-id are mutually exclusive + --preview Interact with a preview namespace [boolean] [default: false]" + `); + expect(std.err).toMatchInlineSnapshot(` + "X [ERROR] Arguments binding and namespace-id are mutually exclusive " `); diff --git a/packages/wrangler/src/__tests__/r2.test.ts b/packages/wrangler/src/__tests__/r2.test.ts index 780d84d8c411..c5281ab9d637 100644 --- a/packages/wrangler/src/__tests__/r2.test.ts +++ b/packages/wrangler/src/__tests__/r2.test.ts @@ -17,6 +17,33 @@ describe("wrangler", () => { describe("r2", () => { describe("bucket", () => { + it("should show the correct help when an invalid command is passed", async () => { + await expect(() => + runWrangler("r2 bucket foo") + ).rejects.toThrowErrorMatchingInlineSnapshot(`"Unknown argument: foo"`); + expect(std.err).toMatchInlineSnapshot(` + "X [ERROR] Unknown argument: foo + + " + `); + expect(std.out).toMatchInlineSnapshot(` + " + wrangler r2 bucket + + Manage R2 buckets + + Commands: + wrangler r2 bucket create Create a new R2 bucket + wrangler r2 bucket list List R2 buckets + wrangler r2 bucket delete Delete an R2 bucket + + Flags: + -c, --config Path to .toml configuration file [string] + -h, --help Show help [boolean] + -v, --version Show version number [boolean]" + `); + }); + describe("list", () => { function mockListRequest(buckets: R2BucketInfo[]) { const requests = { count: 0 }; @@ -69,10 +96,7 @@ describe("wrangler", () => { ); expect(std.out).toMatchInlineSnapshot(` " - " - `); - expect(std.err).toMatchInlineSnapshot(` - "wrangler r2 bucket create + wrangler r2 bucket create Create a new R2 bucket @@ -82,8 +106,10 @@ describe("wrangler", () => { Flags: -c, --config Path to .toml configuration file [string] -h, --help Show help [boolean] - -v, --version Show version number [boolean] - X [ERROR] Not enough non-option arguments: got 0, need at least 1 + -v, --version Show version number [boolean]" + `); + expect(std.err).toMatchInlineSnapshot(` + "X [ERROR] Not enough non-option arguments: got 0, need at least 1 " `); @@ -97,10 +123,7 @@ describe("wrangler", () => { ); expect(std.out).toMatchInlineSnapshot(` " - " - `); - expect(std.err).toMatchInlineSnapshot(` - "wrangler r2 bucket create + wrangler r2 bucket create Create a new R2 bucket @@ -110,8 +133,10 @@ describe("wrangler", () => { Flags: -c, --config Path to .toml configuration file [string] -h, --help Show help [boolean] - -v, --version Show version number [boolean] - X [ERROR] Unknown arguments: def, ghi + -v, --version Show version number [boolean]" + `); + expect(std.err).toMatchInlineSnapshot(` + "X [ERROR] Unknown arguments: def, ghi " `); @@ -151,10 +176,7 @@ describe("wrangler", () => { ); expect(std.out).toMatchInlineSnapshot(` " - " - `); - expect(std.err).toMatchInlineSnapshot(` - "wrangler r2 bucket delete + wrangler r2 bucket delete Delete an R2 bucket @@ -164,8 +186,10 @@ describe("wrangler", () => { Flags: -c, --config Path to .toml configuration file [string] -h, --help Show help [boolean] - -v, --version Show version number [boolean] - X [ERROR] Not enough non-option arguments: got 0, need at least 1 + -v, --version Show version number [boolean]" + `); + expect(std.err).toMatchInlineSnapshot(` + "X [ERROR] Not enough non-option arguments: got 0, need at least 1 " `); @@ -179,10 +203,7 @@ describe("wrangler", () => { ); expect(std.out).toMatchInlineSnapshot(` " - " - `); - expect(std.err).toMatchInlineSnapshot(` - "wrangler r2 bucket delete + wrangler r2 bucket delete Delete an R2 bucket @@ -192,8 +213,10 @@ describe("wrangler", () => { Flags: -c, --config Path to .toml configuration file [string] -h, --help Show help [boolean] - -v, --version Show version number [boolean] - X [ERROR] Unknown arguments: def, ghi + -v, --version Show version number [boolean]" + `); + expect(std.err).toMatchInlineSnapshot(` + "X [ERROR] Unknown arguments: def, ghi " `); diff --git a/packages/wrangler/src/index.tsx b/packages/wrangler/src/index.tsx index e56a42112e8a..9cc29d816ac2 100644 --- a/packages/wrangler/src/index.tsx +++ b/packages/wrangler/src/index.tsx @@ -208,7 +208,7 @@ function demandOneOfOption(...options: string[]) { class CommandLineArgsError extends Error {} -export async function main(argv: string[]): Promise { +function createCLIParser(argv: string[]) { const wrangler = makeCLI(argv) .strict() // We handle errors ourselves in a try-catch around `yargs.parse`. @@ -2655,14 +2655,21 @@ export async function main(argv: string[]): Promise { wrangler.version(wranglerVersion).alias("v", "version"); wrangler.exitProcess(false); + return wrangler; +} + +export async function main(argv: string[]): Promise { + const wrangler = createCLIParser(argv); try { await wrangler.parse(); } catch (e) { logger.log(""); // Just adds a bit of space if (e instanceof CommandLineArgsError) { - wrangler.showHelp("error"); - logger.log(""); // Add a bit of space. logger.error(e.message); + // We are not able to ask the `wrangler` CLI parser to show help for a subcommand programmatically. + // The workaround is to re-run the parsing with an additional `--help` flag, which will result in the correct help message being displayed. + // The `wrangler` object is "frozen"; we cannot reuse that with different args, so we must create a new CLI parser to generate the help message. + await createCLIParser([...argv, "--help"]).parse(); } else if (e instanceof ParseError) { e.notes.push({ text: "\nIf you think this is a bug, please open an issue at: https://github.com/cloudflare/wrangler2/issues/new",