neofs-node icon indicating copy to clipboard operation
neofs-node copied to clipboard

Cobra command runners skip `defer`ed funcs and postruns on failures

Open cthulhu-rider opened this issue 7 months ago • 4 comments

some Cobra-based programs exit instantly on particular errors:

  • https://github.com/nspcc-dev/neofs-node/blob/450c4e5ed85f77dc77a13625337d71f7d97fa9ef/cmd/neofs-cli/internal/common/exit.go#L22
  • https://github.com/nspcc-dev/neofs-node/blob/450c4e5ed85f77dc77a13625337d71f7d97fa9ef/cmd/neofs-lens/internal/errors.go#L20

they call os.Exit due to which neither cmd postruns, nor finalizers, nor even exec defer funcs are called. The latter are usually responsible for allocating resources such as network connections, files, etc.

Steps to Reproduce

  1. add cmd/neofs-test/main.go
func main() {
	(&cobra.Command{
		Use: "test",
		Run: func(cmd *cobra.Command, _ []string) {
			cobra.OnFinalize(func() {
				cmd.Println("in finalizer")
			})
			defer func() {
				cmd.Println("in defer")
			}()
			fmt.Fprintf(os.Stderr, "err: %v\n", errors.New("any error"))
			os.Exit(1)
		},
		PostRun: func(cmd *cobra.Command, args []string) {
			cmd.Println("in PostRun")
		},
		PersistentPostRun: func(cmd *cobra.Command, args []string) {
			cmd.Println("in PersistentPostRun")
		},
	}).Execute()
}

  1. make bin/neofs-test
  2. `./bin/neofs-test

Expected Behavior

$ ./bin/neofs-test
in defer
in PostRun
in PersistentPostRun
in finalizer
err: any error
$ echo $?
1

Current Behavior

$ ./bin/neofs-test
err: any error
$ echo $?
1

Possible Solution

1. Finalizing closure in Run

the most clean solution to me in terms of code and command lifetime. At the same time, requires a lot of changes, code will be pretty "spaghetti"

func main() {
	err := (&cobra.Command{
		Use:           "test",
		SilenceErrors: true,
		SilenceUsage:  true,
		Run: func(cmd *cobra.Command, _ []string) {
			var code int
			var err error
			cobra.OnFinalize(func() {
				cmd.Println("in finalizer")
				if err != nil {
					fmt.Fprintln(os.Stderr, "err:", err)
				}
				os.Exit(code)
			})
			defer func() {
				cmd.Println("in defer")
			}()
			code = 2
			err = errors.New("any error")
		},
		PostRun: func(cmd *cobra.Command, args []string) {
			cmd.Println("in PostRun")
		},
		PersistentPostRun: func(cmd *cobra.Command, args []string) {
			cmd.Println("in PersistentPostRun")
		},
	}).Execute()
	if err != nil {
		fmt.Fprintln(os.Stderr, err)
		os.Exit(1)
	}
}

2. Specific error + RunE + unsupported *PostRun

the most correct solution to me in terms of os.Exit, but narrows the breadth of cmd lifetime use. Also requires a lot of changes but code will look less "spaghetti"

type ExitErr struct {
	Code  int
	Cause error
}

func (x ExitErr) Error() string { return x.Cause.Error() }

func main() {
	err := (&cobra.Command{
		Use:           "test",
		SilenceErrors: true,
		SilenceUsage:  true,
		RunE: func(cmd *cobra.Command, _ []string) error {
			cobra.OnFinalize(func() {
				cmd.Println("in finalizer")
			})
			defer func() {
				cmd.Println("in defer")
			}()
			return ExitErr{Code: 2, Cause: fmt.Errorf("err: %w", errors.New("any error"))}
		},
	}).Execute()
	if err != nil {
		var e ExitErr
		if !errors.As(err, &e) {
			e.Code = 1
		}
		fmt.Fprintln(os.Stderr, err)
		os.Exit(e.Code)
	}
}

3. Fake defer's + unsupported *PostRun

the worst to me but fast solution requiring less changes

var deferred []func()

func Defer(f func()) { deferred = append(deferred, f) }

func ExitOnErr(cmd *cobra.Command, errFmt string, err error) {
	...
	cmd.PrintErrln(err)
	for i := range deferred {
		deferred[len(deferred)-i-1]()
	}
	os.Exit(code)
}

Context

Cobra itself has no in-box mechanism to work with OS exit codes, only similar funcs

ive been hiding this topic in myself for a long time, but now im ready to present it

Regression

no

Your Environment

cthulhu-rider avatar Jul 10 '24 08:07 cthulhu-rider