Support bailing from RunFunc (#13804)
**Background**
The result.Result type is used by our CLI implementation to communicate
how we want to exit the program.
Most `result.Result` values (built from errors with `result.FromError`)
cause the program to print the message to stderr and exit the program
with exit code -1.
The exception is `result.Bail()`, which indicates that we've already
printed the error message, and we simply need to `exit(-1)` now.
Our CLI command implementation use `cmdutil.RunResultFunc` which takes a
`func(...) result.Result` to implement this logic.
`cmdutil` additionally includes a `cmdutil.RunFunc` which takes a
`func(...) error` and wraps it in `RunResultFunc`, relying on
`result.FromError` for the conversion:
func RunFunc(run func(...) error) func(...) {
return RunResultFunc(func(...) result.Result {
if err := run(...); err != nil {
return result.FromError(err)
}
return nil
})
}
**Problem**
In CLI contexts where we're using an `error`, and we want to print an
error message to the user and exit, it's desirable to use diag.Sink to
print the message to the user with the appropriate level (error,
warning, etc.) and exit without printing anything else.
However, the only way to do that currently is by converting that
function to return `result.Result`, turn all error returns to
`result.FromError`, and then return `result.Bail()`.
**Solution**
This change introduces a `result.BailError` error that gets converted
into a `result.Bail()` when it passes through `result.FromError`.
It allows commands implementations that use `error` to continue
returning errors and still provide an ideal CLI experience.
It relies on `errors.As` for matching, so even if an intermediate layer
wraps the error with `fmt.Errorf("..: %w", ErrBail)`, we'll recognize
the request to bail.
BailError keep track of the internal error that triggered it, which
(when everything is moved off of result and onto error) means we'll
still be able to see the internal errors that triggered a bail during
debugging.
Currently debugging engine tests is pretty horrible because you often
just get back a `result.Result{err:nil}` with no information where in
the engine stack that came from.
**Testing**
Besides unit tests, this includes an end-to-end test for using
RunResultFunc with a bail error.
The test operates by putting the mock behavior in a fake test, and
re-running the test binary to execute *just that test*.
**Demonstration**
This change also ports the following commands to use BailError: cancel,
convert, env, policy rm, stack rm.
These command implementations are simple and were able to switch easily,
without bubbling into a change to a bunch of other code.
2023-08-29 07:43:40 +00:00
|
|
|
package cmdutil
|
|
|
|
|
|
|
|
import (
|
|
|
|
"bytes"
|
2023-09-01 19:01:16 +00:00
|
|
|
"errors"
|
Support bailing from RunFunc (#13804)
**Background**
The result.Result type is used by our CLI implementation to communicate
how we want to exit the program.
Most `result.Result` values (built from errors with `result.FromError`)
cause the program to print the message to stderr and exit the program
with exit code -1.
The exception is `result.Bail()`, which indicates that we've already
printed the error message, and we simply need to `exit(-1)` now.
Our CLI command implementation use `cmdutil.RunResultFunc` which takes a
`func(...) result.Result` to implement this logic.
`cmdutil` additionally includes a `cmdutil.RunFunc` which takes a
`func(...) error` and wraps it in `RunResultFunc`, relying on
`result.FromError` for the conversion:
func RunFunc(run func(...) error) func(...) {
return RunResultFunc(func(...) result.Result {
if err := run(...); err != nil {
return result.FromError(err)
}
return nil
})
}
**Problem**
In CLI contexts where we're using an `error`, and we want to print an
error message to the user and exit, it's desirable to use diag.Sink to
print the message to the user with the appropriate level (error,
warning, etc.) and exit without printing anything else.
However, the only way to do that currently is by converting that
function to return `result.Result`, turn all error returns to
`result.FromError`, and then return `result.Bail()`.
**Solution**
This change introduces a `result.BailError` error that gets converted
into a `result.Bail()` when it passes through `result.FromError`.
It allows commands implementations that use `error` to continue
returning errors and still provide an ideal CLI experience.
It relies on `errors.As` for matching, so even if an intermediate layer
wraps the error with `fmt.Errorf("..: %w", ErrBail)`, we'll recognize
the request to bail.
BailError keep track of the internal error that triggered it, which
(when everything is moved off of result and onto error) means we'll
still be able to see the internal errors that triggered a bail during
debugging.
Currently debugging engine tests is pretty horrible because you often
just get back a `result.Result{err:nil}` with no information where in
the engine stack that came from.
**Testing**
Besides unit tests, this includes an end-to-end test for using
RunResultFunc with a bail error.
The test operates by putting the mock behavior in a fake test, and
re-running the test binary to execute *just that test*.
**Demonstration**
This change also ports the following commands to use BailError: cancel,
convert, env, policy rm, stack rm.
These command implementations are simple and were able to switch easily,
without bubbling into a change to a bunch of other code.
2023-08-29 07:43:40 +00:00
|
|
|
"io"
|
|
|
|
"os"
|
|
|
|
"os/exec"
|
|
|
|
"testing"
|
|
|
|
|
2023-09-01 19:01:16 +00:00
|
|
|
"github.com/hashicorp/go-multierror"
|
Support bailing from RunFunc (#13804)
**Background**
The result.Result type is used by our CLI implementation to communicate
how we want to exit the program.
Most `result.Result` values (built from errors with `result.FromError`)
cause the program to print the message to stderr and exit the program
with exit code -1.
The exception is `result.Bail()`, which indicates that we've already
printed the error message, and we simply need to `exit(-1)` now.
Our CLI command implementation use `cmdutil.RunResultFunc` which takes a
`func(...) result.Result` to implement this logic.
`cmdutil` additionally includes a `cmdutil.RunFunc` which takes a
`func(...) error` and wraps it in `RunResultFunc`, relying on
`result.FromError` for the conversion:
func RunFunc(run func(...) error) func(...) {
return RunResultFunc(func(...) result.Result {
if err := run(...); err != nil {
return result.FromError(err)
}
return nil
})
}
**Problem**
In CLI contexts where we're using an `error`, and we want to print an
error message to the user and exit, it's desirable to use diag.Sink to
print the message to the user with the appropriate level (error,
warning, etc.) and exit without printing anything else.
However, the only way to do that currently is by converting that
function to return `result.Result`, turn all error returns to
`result.FromError`, and then return `result.Bail()`.
**Solution**
This change introduces a `result.BailError` error that gets converted
into a `result.Bail()` when it passes through `result.FromError`.
It allows commands implementations that use `error` to continue
returning errors and still provide an ideal CLI experience.
It relies on `errors.As` for matching, so even if an intermediate layer
wraps the error with `fmt.Errorf("..: %w", ErrBail)`, we'll recognize
the request to bail.
BailError keep track of the internal error that triggered it, which
(when everything is moved off of result and onto error) means we'll
still be able to see the internal errors that triggered a bail during
debugging.
Currently debugging engine tests is pretty horrible because you often
just get back a `result.Result{err:nil}` with no information where in
the engine stack that came from.
**Testing**
Besides unit tests, this includes an end-to-end test for using
RunResultFunc with a bail error.
The test operates by putting the mock behavior in a fake test, and
re-running the test binary to execute *just that test*.
**Demonstration**
This change also ports the following commands to use BailError: cancel,
convert, env, policy rm, stack rm.
These command implementations are simple and were able to switch easily,
without bubbling into a change to a bunch of other code.
2023-08-29 07:43:40 +00:00
|
|
|
"github.com/pulumi/pulumi/sdk/v3/go/common/testing/iotest"
|
|
|
|
"github.com/pulumi/pulumi/sdk/v3/go/common/util/result"
|
|
|
|
"github.com/spf13/cobra"
|
|
|
|
"github.com/stretchr/testify/assert"
|
|
|
|
"github.com/stretchr/testify/require"
|
|
|
|
)
|
|
|
|
|
|
|
|
func TestRunFunc_Bail(t *testing.T) {
|
|
|
|
t.Parallel()
|
|
|
|
|
|
|
|
// Verifies that a use of RunFunc that returns BailError
|
|
|
|
// will cause the program to exit with a non-zero exit code
|
|
|
|
// without printing an error message.
|
|
|
|
//
|
|
|
|
// Unfortunately, we can't test this directly,
|
|
|
|
// because the `os.Exit` call in RunResultFunc.
|
|
|
|
//
|
|
|
|
// Instead, we'll re-run the test binary,
|
|
|
|
// and have it run TestFakeCommand.
|
|
|
|
// We'll verify the output of that binary instead.
|
|
|
|
|
|
|
|
exe, err := os.Executable()
|
|
|
|
require.NoError(t, err)
|
|
|
|
|
|
|
|
cmd := exec.Command(exe, "-test.run=^TestFakeCommand$")
|
|
|
|
cmd.Env = append(os.Environ(), "TEST_FAKE=1")
|
|
|
|
|
|
|
|
// Write output to the buffer and to the test logger.
|
|
|
|
var buff bytes.Buffer
|
|
|
|
output := io.MultiWriter(&buff, iotest.LogWriter(t))
|
|
|
|
cmd.Stdout = output
|
|
|
|
cmd.Stderr = output
|
|
|
|
|
|
|
|
err = cmd.Run()
|
2023-12-08 06:40:14 +00:00
|
|
|
exitErr := new(exec.ExitError)
|
|
|
|
require.ErrorAs(t, err, &exitErr)
|
|
|
|
assert.NotZero(t, exitErr.ExitCode())
|
Support bailing from RunFunc (#13804)
**Background**
The result.Result type is used by our CLI implementation to communicate
how we want to exit the program.
Most `result.Result` values (built from errors with `result.FromError`)
cause the program to print the message to stderr and exit the program
with exit code -1.
The exception is `result.Bail()`, which indicates that we've already
printed the error message, and we simply need to `exit(-1)` now.
Our CLI command implementation use `cmdutil.RunResultFunc` which takes a
`func(...) result.Result` to implement this logic.
`cmdutil` additionally includes a `cmdutil.RunFunc` which takes a
`func(...) error` and wraps it in `RunResultFunc`, relying on
`result.FromError` for the conversion:
func RunFunc(run func(...) error) func(...) {
return RunResultFunc(func(...) result.Result {
if err := run(...); err != nil {
return result.FromError(err)
}
return nil
})
}
**Problem**
In CLI contexts where we're using an `error`, and we want to print an
error message to the user and exit, it's desirable to use diag.Sink to
print the message to the user with the appropriate level (error,
warning, etc.) and exit without printing anything else.
However, the only way to do that currently is by converting that
function to return `result.Result`, turn all error returns to
`result.FromError`, and then return `result.Bail()`.
**Solution**
This change introduces a `result.BailError` error that gets converted
into a `result.Bail()` when it passes through `result.FromError`.
It allows commands implementations that use `error` to continue
returning errors and still provide an ideal CLI experience.
It relies on `errors.As` for matching, so even if an intermediate layer
wraps the error with `fmt.Errorf("..: %w", ErrBail)`, we'll recognize
the request to bail.
BailError keep track of the internal error that triggered it, which
(when everything is moved off of result and onto error) means we'll
still be able to see the internal errors that triggered a bail during
debugging.
Currently debugging engine tests is pretty horrible because you often
just get back a `result.Result{err:nil}` with no information where in
the engine stack that came from.
**Testing**
Besides unit tests, this includes an end-to-end test for using
RunResultFunc with a bail error.
The test operates by putting the mock behavior in a fake test, and
re-running the test binary to execute *just that test*.
**Demonstration**
This change also ports the following commands to use BailError: cancel,
convert, env, policy rm, stack rm.
These command implementations are simple and were able to switch easily,
without bubbling into a change to a bunch of other code.
2023-08-29 07:43:40 +00:00
|
|
|
|
|
|
|
assert.Empty(t, buff.String())
|
|
|
|
}
|
|
|
|
|
|
|
|
//nolint:paralleltest // not a real test
|
|
|
|
func TestFakeCommand(t *testing.T) {
|
|
|
|
if os.Getenv("TEST_FAKE") != "1" {
|
|
|
|
// This is not a real test.
|
|
|
|
// It's a fake test that we'll run as a subprocess
|
|
|
|
// to verify that the RunFunc function works correctly.
|
|
|
|
// See TestRunFunc_Bail for more details.
|
|
|
|
return
|
|
|
|
}
|
|
|
|
|
|
|
|
cmd := &cobra.Command{
|
|
|
|
Run: RunFunc(func(cmd *cobra.Command, args []string) error {
|
|
|
|
return result.BailErrorf("bail")
|
|
|
|
}),
|
|
|
|
}
|
|
|
|
err := cmd.Execute()
|
|
|
|
// Unreachable: RunFunc should have called os.Exit.
|
|
|
|
assert.Fail(t, "unreachable", "RunFunc should have called os.Exit: %v", err)
|
|
|
|
}
|
2023-09-01 19:01:16 +00:00
|
|
|
|
|
|
|
func TestErrorMessage(t *testing.T) {
|
|
|
|
t.Parallel()
|
|
|
|
|
|
|
|
tests := []struct {
|
|
|
|
desc string
|
|
|
|
give error
|
|
|
|
want string
|
|
|
|
}{
|
|
|
|
{
|
|
|
|
desc: "simple error",
|
|
|
|
give: errors.New("great sadness"),
|
|
|
|
want: "great sadness",
|
|
|
|
},
|
|
|
|
{
|
|
|
|
desc: "hashi multi error",
|
|
|
|
give: multierror.Append(
|
|
|
|
errors.New("foo"),
|
|
|
|
errors.New("bar"),
|
|
|
|
errors.New("baz"),
|
|
|
|
),
|
|
|
|
want: "3 errors occurred:" +
|
|
|
|
"\n 1) foo" +
|
|
|
|
"\n 2) bar" +
|
|
|
|
"\n 3) baz",
|
|
|
|
},
|
|
|
|
{
|
|
|
|
desc: "std errors.Join",
|
|
|
|
give: errors.Join(
|
|
|
|
errors.New("foo"),
|
|
|
|
errors.New("bar"),
|
|
|
|
errors.New("baz"),
|
|
|
|
),
|
|
|
|
want: "3 errors occurred:" +
|
|
|
|
"\n 1) foo" +
|
|
|
|
"\n 2) bar" +
|
|
|
|
"\n 3) baz",
|
|
|
|
},
|
|
|
|
{
|
|
|
|
desc: "empty multi error",
|
|
|
|
// This is technically invalid,
|
|
|
|
// but we guard against it,
|
|
|
|
// so let's test it too.
|
|
|
|
give: &invalidEmptyMultiError{},
|
|
|
|
want: "invalid empty multi error",
|
|
|
|
},
|
|
|
|
{
|
|
|
|
desc: "single wrapped error",
|
|
|
|
give: &multierror.Error{
|
|
|
|
Errors: []error{
|
|
|
|
errors.New("great sadness"),
|
|
|
|
},
|
|
|
|
},
|
|
|
|
want: "great sadness",
|
|
|
|
},
|
|
|
|
{
|
|
|
|
desc: "multi error inside single wrapped error",
|
|
|
|
give: &multierror.Error{
|
|
|
|
Errors: []error{
|
|
|
|
errors.Join(
|
|
|
|
errors.New("foo"),
|
|
|
|
errors.New("bar"),
|
|
|
|
errors.New("baz"),
|
|
|
|
),
|
|
|
|
},
|
|
|
|
},
|
|
|
|
want: "3 errors occurred:" +
|
|
|
|
"\n 1) foo" +
|
|
|
|
"\n 2) bar" +
|
|
|
|
"\n 3) baz",
|
|
|
|
},
|
|
|
|
}
|
|
|
|
|
|
|
|
for _, tt := range tests {
|
|
|
|
tt := tt
|
|
|
|
t.Run(tt.desc, func(t *testing.T) {
|
|
|
|
t.Parallel()
|
|
|
|
|
|
|
|
got := errorMessage(tt.give)
|
|
|
|
assert.Equal(t, tt.want, got)
|
|
|
|
})
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
// invalidEmptyMultiError is an invalid error type
|
|
|
|
// that implements Unwrap() []error, but returns an empty slice.
|
|
|
|
// This is invalid per the contract for that method.
|
|
|
|
type invalidEmptyMultiError struct{}
|
|
|
|
|
|
|
|
func (*invalidEmptyMultiError) Error() string {
|
|
|
|
return "invalid empty multi error"
|
|
|
|
}
|
|
|
|
|
|
|
|
func (*invalidEmptyMultiError) Unwrap() []error {
|
|
|
|
return []error{} // invalid
|
|
|
|
}
|