Question about panic -> `After()` behaviour

[knadh]

Version: v2.25.7 Related: https://github.com/urfave/cli/issues/451

Hi there.

After(), as documented, executes regardless of any panics in Action() functions. Naturally, any business logic in After() that depends on business logic executed by an Action() will fail.

1. Is there a way to know inside After() whether an Action() has failed so that any subsequent business logic doesn't execute?
2. Adding defer/recover inside every Action() and taking over the control flow just because After() is designed to execute despite panics may not be ideal.
3. Are there any specific reasons as to why After() was designed to suppress panics and execute regardless?
4. I had an os.Exit(0) inside After() which prevented any stack traces from being printed, so it took forever to figure out that there was a panic happening. Should it be explicitly documented that os.Exit(0) should not be used at all?

Thanks!

Sample code for context (./app.bin test)

package main

import (
    "fmt"
    "log"
    "os"

    "github.com/urfave/cli/v2"
)

func main() {
    app := cli.NewApp()

    app.After = func(c *cli.Context) error {
        // 1. ... complex business logic here that depends on sub command
        // Actions() being executed successfully. If After() always executes
        // despite panics in sub commands, how can it know if there was a problem
        // to halt and return so that a failed run doesn't continue to execute here?
        fmt.Fprintf(c.App.Writer, "AFTER()\n")

        os.Exit(0)
        // 2. If there happens to be an os.Exit() anywhere, then the stacktrace
        // of panics happening inside Action() is not printed.
        // Should it be explicitly stated that using os.Exit() anywhere is not safe
        // and cli.Exit() should be used instead?
        return nil
    }

    app.Commands = []*cli.Command{
        {
            Name: "test",
            Action: func(c *cli.Context) error {
                // ... complex business logic here.
                // Without a defer/recover in every single Action(), it is not possible
                // to know if there is a panic.
                fmt.Fprintf(c.App.Writer, "ABOUT TO PANIC ...\n")
                panic("FAKE PANIC")
                return nil
            },
        },
    }

    if err := app.Run(os.Args); err != nil {
        log.Fatal(err)
    }
}

[dearchap]

1. Is there a way to know inside After() whether an Action() has failed so that any subsequent business logic doesn't execute?

The cli library itself does not provide any mechanism to do this. You can implement it on your own.

2. Are there any specific reasons as to why After() was designed to suppress panics and execute regardless?

No panics are not suppressed. However After() is added as a deferred function to the command processing logic so it will execute even if Action() panics. That is golang standard behaviour. This allows the defer function to suppress/manage the panic and continue or abort.

3. I had an os.Exit(0) inside After() which prevented any stack traces from being printed, so it took forever to figure out that there was a panic happening. Should it be explicitly documented that os.Exit(0) should not be used at all?

Generally you shouldnt be using os.Exit() at all, you either return an err or nil to the corresponding Action()/Before()/After() funcs and the cli library will handle the rest.

[knadh]

However After() is added as a deferred function to the command processing logic so it will execute even if Action() panics

Generally you shouldnt be using os.Exit() at all, you either return an err or nil to the corresponding Action()/Before()/After() funcs and the cli library will handle the rest.

This has to be explicitly stated in the docs then. Will send a docs PR sometime soon. Thank you.