2024-09-09 12:05:45 +00:00
|
|
|
// Copyright 2023-2024, Pulumi Corporation.
|
|
|
|
//
|
|
|
|
// Licensed under the Apache License, Version 2.0 (the "License");
|
|
|
|
// you may not use this file except in compliance with the License.
|
|
|
|
// You may obtain a copy of the License at
|
|
|
|
//
|
|
|
|
// http://www.apache.org/licenses/LICENSE-2.0
|
|
|
|
//
|
|
|
|
// Unless required by applicable law or agreed to in writing, software
|
|
|
|
// distributed under the License is distributed on an "AS IS" BASIS,
|
|
|
|
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
|
|
|
// See the License for the specific language governing permissions and
|
|
|
|
// limitations under the License.
|
|
|
|
|
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",
|
|
|
|
},
|
Clean up multi-error rendering (#17039)
When an error occurs in Go, there is often a need to "wrap" the error
and propagate it upwards. For instance, if a "file not found" error
occurs while trying to read a `Pulumi.yaml`, we want to surface both
those pieces of information to the consumer of the error. Since 1.13, Go
provides support for natively wrapping errors using e.g. the `%w` format
specified in `fmt.Errorf` and the `Unwrap() []error` interface method.
Prior to 1.13. libraries such as `multierror` provided a similar
function in the absence of standard-library support.
Due to its age, the `pulumi/pulumi` codebase uses both Go-native error
wrapping and `multierror`. It would likely be good to converge on the
former and lose a dependency/remove extraneous code paths. One key
difference between the two approaches is in how multiple errors are
combined into a new parent. In `multierror`, the `Append` function takes
a set of errors and combines them, _flattening any existing trees of
errors_ so that the result is always a flat list of errors. In contrast,
Go's built-in `errors.Join` does not flatten and so can produce trees of
errors. Practically speaking, this is unlikely to be a huge problem for
this codebase, but there are some changes we can make proactively to
reduce the risks of trees-vs-lists if and when we migrate. Specifically,
when rendering errors, we don't consider the possibility of there being
a tree at present. This means that an input such as:
```go
errors.Join(
errors.Join(
errors.New("foo"),
errors.New("bar"),
),
errors.New("baz"),
)
```
results in something like:
```
1) 2 errors occurred:
1) foo
2) bar
2) baz
```
This commit fixes this so that errors are flattened at the point of
rendering, meaning that it shouldn't matter as we move from
`multierror.Append` to `errors.Join` whether or not an error chain is a
tree or a list.
2024-08-22 10:32:45 +00:00
|
|
|
{
|
|
|
|
desc: "error trees (left-nested)",
|
|
|
|
give: errors.Join(
|
|
|
|
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: "error trees (right-nested)",
|
|
|
|
give: errors.Join(
|
|
|
|
errors.New("foo"),
|
|
|
|
errors.Join(
|
|
|
|
errors.New("bar"),
|
|
|
|
errors.New("baz"),
|
|
|
|
),
|
|
|
|
),
|
|
|
|
want: "3 errors occurred:" +
|
|
|
|
"\n 1) foo" +
|
|
|
|
"\n 2) bar" +
|
|
|
|
"\n 3) baz",
|
|
|
|
},
|
|
|
|
{
|
|
|
|
desc: "error trees (mixed)",
|
|
|
|
give: errors.Join(
|
|
|
|
errors.Join(
|
|
|
|
errors.New("foo"),
|
|
|
|
errors.Join(
|
|
|
|
errors.New("bar"),
|
|
|
|
errors.New("baz"),
|
|
|
|
),
|
|
|
|
),
|
|
|
|
errors.Join(
|
|
|
|
errors.Join(
|
|
|
|
errors.New("quux"),
|
|
|
|
errors.New("frob"),
|
|
|
|
),
|
|
|
|
errors.New("urk"),
|
|
|
|
errors.New("blog"),
|
|
|
|
),
|
|
|
|
),
|
|
|
|
want: "7 errors occurred:" +
|
|
|
|
"\n 1) foo" +
|
|
|
|
"\n 2) bar" +
|
|
|
|
"\n 3) baz" +
|
|
|
|
"\n 4) quux" +
|
|
|
|
"\n 5) frob" +
|
|
|
|
"\n 6) urk" +
|
|
|
|
"\n 7) blog",
|
|
|
|
},
|
2023-09-01 19:01:16 +00:00
|
|
|
{
|
|
|
|
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
|
|
|
|
}
|