From f824274093b27921e20a65c37fc684c6b55263f8 Mon Sep 17 00:00:00 2001 From: Sonia Appasamy Date: Tue, 29 Aug 2023 10:55:38 -0400 Subject: [PATCH] cli/serve: shorten help text on error Our BETA serve help text is long and often hides the actual error in the user's usage. Instead of printing the full text, prompt users to use `serve --help` if they want the help info. Fixes #14274 Signed-off-by: Sonia Appasamy --- cmd/tailscale/cli/serve.go | 24 ++++++++++++++---------- cmd/tailscale/cli/serve_test.go | 12 ++++++------ 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/cmd/tailscale/cli/serve.go b/cmd/tailscale/cli/serve.go index c33c8e75c..39c0e106e 100644 --- a/cmd/tailscale/cli/serve.go +++ b/cmd/tailscale/cli/serve.go @@ -120,6 +120,10 @@ func newServeCommand(e *serveEnv) *ffcli.Command { } } +// errHelp is standard error text that prompts users to +// run `serve --help` for information on how to use serve. +var errHelp = errors.New("try `tailscale serve --help` for usage info") + func (e *serveEnv) newFlags(name string, setup func(fs *flag.FlagSet)) *flag.FlagSet { onError, out := flag.ExitOnError, Stderr if e.testFlagOut != nil { @@ -244,7 +248,7 @@ func (e *serveEnv) runServe(ctx context.Context, args []string) error { if len(args) < 2 || ((srcType == "https" || srcType == "http") && !turnOff && len(args) < 3) { fmt.Fprintf(os.Stderr, "error: invalid number of arguments\n\n") - return flag.ErrHelp + return errHelp } if srcType == "https" && !turnOff { @@ -286,7 +290,7 @@ func (e *serveEnv) runServe(ctx context.Context, args []string) error { default: fmt.Fprintf(os.Stderr, "error: invalid serve type %q\n", srcType) fmt.Fprint(os.Stderr, "must be one of: http:, https:, tcp: or tls-terminated-tcp:\n\n", srcType) - return flag.ErrHelp + return errHelp } } @@ -322,13 +326,13 @@ func (e *serveEnv) handleWebServe(ctx context.Context, srvPort uint16, useTLS bo } if !filepath.IsAbs(source) { fmt.Fprintf(os.Stderr, "error: path must be absolute\n\n") - return flag.ErrHelp + return errHelp } source = filepath.Clean(source) fi, err := os.Stat(source) if err != nil { fmt.Fprintf(os.Stderr, "error: invalid path: %v\n\n", err) - return flag.ErrHelp + return errHelp } if fi.IsDir() && !strings.HasSuffix(mount, "/") { // dir mount points must end in / @@ -354,7 +358,7 @@ func (e *serveEnv) handleWebServe(ctx context.Context, srvPort uint16, useTLS bo if sc.IsTCPForwardingOnPort(srvPort) { fmt.Fprintf(os.Stderr, "error: cannot serve web; already serving TCP\n") - return flag.ErrHelp + return errHelp } mak.Set(&sc.TCP, srvPort, &ipn.TCPPortHandler{HTTPS: useTLS, HTTP: !useTLS}) @@ -542,18 +546,18 @@ func (e *serveEnv) handleTCPServe(ctx context.Context, srcType string, srcPort u terminateTLS = true default: fmt.Fprintf(os.Stderr, "error: invalid TCP source %q\n\n", dest) - return flag.ErrHelp + return errHelp } dstURL, err := url.Parse(dest) if err != nil { fmt.Fprintf(os.Stderr, "error: invalid TCP source %q: %v\n\n", dest, err) - return flag.ErrHelp + return errHelp } host, dstPortStr, err := net.SplitHostPort(dstURL.Host) if err != nil { fmt.Fprintf(os.Stderr, "error: invalid TCP source %q: %v\n\n", dest, err) - return flag.ErrHelp + return errHelp } switch host { @@ -562,12 +566,12 @@ func (e *serveEnv) handleTCPServe(ctx context.Context, srcType string, srcPort u default: fmt.Fprintf(os.Stderr, "error: invalid TCP source %q\n", dest) fmt.Fprint(os.Stderr, "must be one of: localhost or 127.0.0.1\n\n", dest) - return flag.ErrHelp + return errHelp } if p, err := strconv.ParseUint(dstPortStr, 10, 16); p == 0 || err != nil { fmt.Fprintf(os.Stderr, "error: invalid port %q\n\n", dstPortStr) - return flag.ErrHelp + return errHelp } cursc, err := e.lc.GetServeConfig(ctx) diff --git a/cmd/tailscale/cli/serve_test.go b/cmd/tailscale/cli/serve_test.go index 18d223930..34049921f 100644 --- a/cmd/tailscale/cli/serve_test.go +++ b/cmd/tailscale/cli/serve_test.go @@ -339,19 +339,19 @@ type step struct { add(step{reset: true}) add(step{ // must include scheme for tcp command: cmd("tls-terminated-tcp:443 localhost:5432"), - wantErr: exactErr(flag.ErrHelp, "flag.ErrHelp"), + wantErr: exactErr(errHelp, "errHelp"), }) add(step{ // !somehost, must be localhost or 127.0.0.1 command: cmd("tls-terminated-tcp:443 tcp://somehost:5432"), - wantErr: exactErr(flag.ErrHelp, "flag.ErrHelp"), + wantErr: exactErr(errHelp, "errHelp"), }) add(step{ // bad target port, too low command: cmd("tls-terminated-tcp:443 tcp://somehost:0"), - wantErr: exactErr(flag.ErrHelp, "flag.ErrHelp"), + wantErr: exactErr(errHelp, "errHelp"), }) add(step{ // bad target port, too high command: cmd("tls-terminated-tcp:443 tcp://somehost:65536"), - wantErr: exactErr(flag.ErrHelp, "flag.ErrHelp"), + wantErr: exactErr(errHelp, "errHelp"), }) add(step{ command: cmd("tls-terminated-tcp:443 tcp://localhost:5432"), @@ -472,7 +472,7 @@ type step struct { }) add(step{ // bad path command: cmd("https:443 / bad/path"), - wantErr: exactErr(flag.ErrHelp, "flag.ErrHelp"), + wantErr: exactErr(errHelp, "errHelp"), }) add(step{reset: true}) add(step{ @@ -666,7 +666,7 @@ type step struct { }) add(step{ // try to start a web handler on the same port command: cmd("https:443 / localhost:3000"), - wantErr: exactErr(flag.ErrHelp, "flag.ErrHelp"), + wantErr: exactErr(errHelp, "errHelp"), }) add(step{reset: true}) add(step{ // start a web handler on port 443