Skip to content

fix: exit non-zero when CLI flag/env parsing fails - #1714

Open
stareezy-1 wants to merge 1 commit into
cloudflare:masterfrom
stareezy-1:fix/1606-grace-period-exit
Open

fix: exit non-zero when CLI flag/env parsing fails#1714
stareezy-1 wants to merge 1 commit into
cloudflare:masterfrom
stareezy-1:fix/1606-grace-period-exit

Conversation

@stareezy-1

Copy link
Copy Markdown

Summary

cloudflared exits with status 0 and no output when a flag or environment
variable fails to parse, e.g. TUNNEL_GRACE_PERIOD=10 (see #1606).

Problem

TUNNEL_GRACE_PERIOD=10 (missing the duration unit) fails
time.ParseDuration inside urfave/cli's flag setup. App.Run returns that
error without printing anything, and all four runApp implementations
discarded the returned error:

app.Run(os.Args)      // generic, windows (interactive)
_ = app.Run(os.Args)  // linux, macos

so the process exited 0 silently. The same discard made the CLI case
(cloudflared tunnel --grace-period 10 run) exit 0 after printing
"Incorrect Usage", which breaks scripts and service managers that rely on
the exit status. The issue also reports systemd reporting a confusing
"protocol" failure.

Fix

  • New runAppAndExit helper (cmd/cloudflared/run_app.go): runs the app,
    prints any returned error to stderr, and exits 1.
  • All four runApp implementations (generic/linux/macos/windows) now use
    it instead of discarding the error.

Behavior before → after:

Invocation Before After
TUNNEL_GRACE_PERIOD=10 cloudflared tunnel run exit 0, no output exit 1, could not parse "10" as duration value for flag grace-period: time: missing unit in duration "10"
cloudflared tunnel --grace-period 10 run exit 0 exit 1
cloudflared --version / --help exit 0 exit 0 (unchanged)

Tests

  • New regression test TestRunAppInvalidGracePeriodEnvExitsNonZero
    (cmd/cloudflared/main_test.go): re-executes the test binary with
    TUNNEL_GRACE_PERIOD=10 via the helper-process pattern and asserts a
    non-zero exit code plus the error message. Fails without the fix (exit 0),
    passes with it.
  • make test (go test -race ./...): 940 packages pass, 0 failures.
  • go vet ./cmd/cloudflared/ clean; golangci-lint reports no new issues
    in the touched files.

Flag and env-var parse errors (e.g. an invalid TUNNEL_GRACE_PERIOD) are
returned by urfave/cli's app.Run without being printed, and runApp
discarded the error, so cloudflared exited 0 with no output. Print the
error to stderr and exit 1 so scripts and service managers can detect
the failure.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant