From bf2aded8e988d24194b361cfcff58824d0d94a41 Mon Sep 17 00:00:00 2001 From: shreyas-goenka <88374338+shreyas-goenka@users.noreply.github.com> Date: Thu, 27 Feb 2025 18:57:36 +0530 Subject: [PATCH] Recover from panic gracefully (#2353) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Changes This PR adds a recovery function for panics. This indicates to all users running into a panic that it's a bug and they should report it to Databricks. ## Tests Manually and acceptance test. Before: ``` .venv➜ cli git:(panic-r) ✗ ./cli selftest panic panic: the databricks selftest panic command always panics goroutine 1 [running]: github.com/databricks/cli/cmd/selftest.New.newPanic.func1(0x1400016f208?, {0x1016ca925?, 0x4?, 0x1016ca929?}) /Users/shreyas.goenka/cli2/cli/cmd/selftest/panic.go:9 +0x2c github.com/spf13/cobra.(*Command).execute(0x1400016f208, {0x10279bc40, 0x0, 0x0}) /Users/shreyas.goenka/cli2/cli/vendor/github.com/spf13/cobra/command.go:989 +0x81c github.com/spf13/cobra.(*Command).ExecuteC(0x14000428908) /Users/shreyas.goenka/cli2/cli/vendor/github.com/spf13/cobra/command.go:1117 +0x344 github.com/spf13/cobra.(*Command).ExecuteContextC(...) /Users/shreyas.goenka/cli2/cli/vendor/github.com/spf13/cobra/command.go:1050 github.com/databricks/cli/cmd/root.Execute({0x101d60440?, 0x10279bc40?}, 0x10266dd78?) /Users/shreyas.goenka/cli2/cli/cmd/root/root.go:101 +0x58 main.main() /Users/shreyas.goenka/cli2/cli/main.go:13 +0x44 ``` After: ``` .venv➜ cli git:(panic-r) ./cli selftest panic The Databricks CLI unexpectedly had a fatal error. Please report this issue to Databricks in the form of a GitHub issue at: https://github.com/databricks/cli CLI Version: 0.0.0-dev+aae7ced52d36 Panic Payload: the databricks selftest panic command always panics Stack Trace: goroutine 1 [running]: runtime/debug.Stack() /Users/shreyas.goenka/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.4.darwin-arm64/src/runtime/debug/stack.go:26 +0x64 github.com/databricks/cli/cmd/root.Execute.func1() /Users/shreyas.goenka/cli2/cli/cmd/root/root.go:110 +0xa4 panic({0x10368b5e0?, 0x1039d6d70?}) /Users/shreyas.goenka/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.4.darwin-arm64/src/runtime/panic.go:785 +0x124 github.com/databricks/cli/cmd/selftest.New.newPanic.func1(0x14000145208?, {0x103356be5?, 0x4?, 0x103356be9?}) /Users/shreyas.goenka/cli2/cli/cmd/selftest/panic.go:9 +0x2c github.com/spf13/cobra.(*Command).execute(0x14000145208, {0x104427c40, 0x0, 0x0}) /Users/shreyas.goenka/cli2/cli/vendor/github.com/spf13/cobra/command.go:989 +0x81c github.com/spf13/cobra.(*Command).ExecuteC(0x14000400c08) /Users/shreyas.goenka/cli2/cli/vendor/github.com/spf13/cobra/command.go:1117 +0x344 github.com/spf13/cobra.(*Command).ExecuteContextC(...) /Users/shreyas.goenka/cli2/cli/vendor/github.com/spf13/cobra/command.go:1050 github.com/databricks/cli/cmd/root.Execute({0x1039ec440?, 0x104427c40?}, 0x14000400c08) /Users/shreyas.goenka/cli2/cli/cmd/root/root.go:128 +0x94 main.main() /Users/shreyas.goenka/cli2/cli/main.go:13 +0x44 ``` --- acceptance/panic/output.txt | 15 +++++++++++++++ acceptance/panic/script | 5 +++++ acceptance/panic/test.toml | 0 cmd/cmd.go | 2 ++ cmd/root/root.go | 31 ++++++++++++++++++++++++++++--- cmd/selftest/panic.go | 12 ++++++++++++ cmd/selftest/selftest.go | 16 ++++++++++++++++ 7 files changed, 78 insertions(+), 3 deletions(-) create mode 100644 acceptance/panic/output.txt create mode 100644 acceptance/panic/script create mode 100644 acceptance/panic/test.toml create mode 100644 cmd/selftest/panic.go create mode 100644 cmd/selftest/selftest.go diff --git a/acceptance/panic/output.txt b/acceptance/panic/output.txt new file mode 100644 index 000000000..9dca41c23 --- /dev/null +++ b/acceptance/panic/output.txt @@ -0,0 +1,15 @@ + +>>> [CLI] selftest panic +The Databricks CLI unexpectedly had a fatal error. +Please report this issue to Databricks in the form of a GitHub issue at: +https://github.com/databricks/cli + +CLI Version: [DEV_VERSION] + +Panic Payload: the databricks selftest panic command always panics + +Stack Trace: +goroutine 1 [running]: +runtime/debug.Stack() + +Exit code: 1 diff --git a/acceptance/panic/script b/acceptance/panic/script new file mode 100644 index 000000000..a02466923 --- /dev/null +++ b/acceptance/panic/script @@ -0,0 +1,5 @@ +# We filter anything after runtime/debug.Stack() in the output because the stack +# trace itself is hard to perform replacements on, since it can depend upon the +# exact setup of where the modules are installed in your Go setup, memory addresses +# at runtime etc. +trace $CLI selftest panic 2>&1 | sed '/runtime\/debug\.Stack()/q' diff --git a/acceptance/panic/test.toml b/acceptance/panic/test.toml new file mode 100644 index 000000000..e69de29bb diff --git a/cmd/cmd.go b/cmd/cmd.go index 5d835409f..4f5337fd3 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -12,6 +12,7 @@ import ( "github.com/databricks/cli/cmd/fs" "github.com/databricks/cli/cmd/labs" "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/cmd/selftest" "github.com/databricks/cli/cmd/sync" "github.com/databricks/cli/cmd/version" "github.com/databricks/cli/cmd/workspace" @@ -74,6 +75,7 @@ func New(ctx context.Context) *cobra.Command { cli.AddCommand(labs.New(ctx)) cli.AddCommand(sync.New()) cli.AddCommand(version.New()) + cli.AddCommand(selftest.New()) return cli } diff --git a/cmd/root/root.go b/cmd/root/root.go index d7adf47f4..04815f48b 100644 --- a/cmd/root/root.go +++ b/cmd/root/root.go @@ -6,6 +6,7 @@ import ( "fmt" "log/slog" "os" + "runtime/debug" "strings" "github.com/databricks/cli/internal/build" @@ -96,11 +97,35 @@ func flagErrorFunc(c *cobra.Command, err error) error { // Execute adds all child commands to the root command and sets flags appropriately. // This is called by main.main(). It only needs to happen once to the rootCmd. -func Execute(ctx context.Context, cmd *cobra.Command) error { - // TODO: deferred panic recovery +func Execute(ctx context.Context, cmd *cobra.Command) (err error) { + defer func() { + r := recover() + + // No panic. Return normally. + if r == nil { + return + } + + version := build.GetInfo().Version + trace := debug.Stack() + + // Set the error so that the CLI exits with a non-zero exit code. + err = fmt.Errorf("panic: %v", r) + + fmt.Fprintf(cmd.ErrOrStderr(), `The Databricks CLI unexpectedly had a fatal error. +Please report this issue to Databricks in the form of a GitHub issue at: +https://github.com/databricks/cli + +CLI Version: %s + +Panic Payload: %v + +Stack Trace: +%s`, version, r, string(trace)) + }() // Run the command - cmd, err := cmd.ExecuteContextC(ctx) + cmd, err = cmd.ExecuteContextC(ctx) if err != nil && !errors.Is(err, ErrAlreadyPrinted) { // If cmdio logger initialization succeeds, then this function logs with the // initialized cmdio logger, otherwise with the default cmdio logger diff --git a/cmd/selftest/panic.go b/cmd/selftest/panic.go new file mode 100644 index 000000000..58d8b24e5 --- /dev/null +++ b/cmd/selftest/panic.go @@ -0,0 +1,12 @@ +package selftest + +import "github.com/spf13/cobra" + +func newPanic() *cobra.Command { + return &cobra.Command{ + Use: "panic", + Run: func(cmd *cobra.Command, args []string) { + panic("the databricks selftest panic command always panics") + }, + } +} diff --git a/cmd/selftest/selftest.go b/cmd/selftest/selftest.go new file mode 100644 index 000000000..7d8cfcb76 --- /dev/null +++ b/cmd/selftest/selftest.go @@ -0,0 +1,16 @@ +package selftest + +import ( + "github.com/spf13/cobra" +) + +func New() *cobra.Command { + cmd := &cobra.Command{ + Use: "selftest", + Short: "Non functional CLI commands that are useful for testing", + Hidden: true, + } + + cmd.AddCommand(newPanic()) + return cmd +}