Skip to content

Commit f1b5e5e

Browse files
raulbraulb
committed
handle all api errors
1 parent 3779d64 commit f1b5e5e

File tree

2 files changed

+91
-67
lines changed

2 files changed

+91
-67
lines changed

Diff for: ‎cmd/conduit/cecdysis/decorators.go

+11-12
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import (
2626
"github.com/conduitio/conduit/pkg/foundation/cerrors"
2727
"github.com/conduitio/ecdysis"
2828
"github.com/spf13/cobra"
29-
"google.golang.org/grpc/codes"
3029
"google.golang.org/grpc/status"
3130
)
3231

@@ -66,36 +65,36 @@ func (CommandWithExecuteWithClientDecorator) Decorate(_ *ecdysis.Ecdysis, cmd *c
6665

6766
client, err := api.NewClient(cmd.Context(), grpcAddress)
6867
if err != nil {
69-
// Not an error we need to escalate to the main CLI execution. We'll print it out and not execute further.
70-
_, _ = fmt.Fprintf(os.Stderr, "%v\n", err)
71-
return nil
68+
return handleError(err)
7269
}
7370
defer client.Close()
7471

7572
ctx := ecdysis.ContextWithCobraCommand(cmd.Context(), cmd)
76-
return handleExecuteError(v.ExecuteWithClient(ctx, client))
73+
return handleError(v.ExecuteWithClient(ctx, client))
7774
}
7875

7976
return nil
8077
}
8178

8279
// check what type of error is to see if it's worth showing `cmd.Usage()` or not.
83-
// if error is returned, usage will be shown automatically.
84-
func handleExecuteError(err error) error {
80+
// if any error is returned, usage will be shown automatically.
81+
func handleError(err error) error {
82+
errMsg := err.Error()
8583
st, ok := status.FromError(err)
86-
if ok && st.Code() == codes.NotFound {
87-
errMsg := st.Message()
84+
85+
// an API error, we try to parse `desc`
86+
if ok {
87+
errMsg = st.Message()
8888

8989
// st.Message() is already an entire representation of the error
9090
// need to grab the desc
9191
descIndex := strings.Index(errMsg, "desc =")
9292
if descIndex != -1 {
9393
errMsg = errMsg[descIndex+len("desc = "):]
9494
}
95-
_, _ = fmt.Fprintf(os.Stderr, "%s\n", errMsg)
96-
return nil
9795
}
98-
return err
96+
_, _ = fmt.Fprintf(os.Stderr, "%v\n", errMsg)
97+
return nil
9998
}
10099

101100
// getGRPCAddress returns the gRPC address configured by the user. If no address is found, the default address is returned.

Diff for: ‎cmd/conduit/cecdysis/decorators_test.go

+80-55
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,13 @@
1515
package cecdysis
1616

1717
import (
18+
"bytes"
19+
"io"
20+
"os"
21+
"strings"
1822
"testing"
1923

24+
"github.com/conduitio/conduit/pkg/foundation/cerrors"
2025
"github.com/matryer/is"
2126
"google.golang.org/grpc/codes"
2227
"google.golang.org/grpc/status"
@@ -26,102 +31,122 @@ func TestHandleError(t *testing.T) {
2631
is := is.New(t)
2732

2833
testCases := []struct {
29-
name string
30-
inputError error
31-
expected error
34+
name string
35+
inputError error
36+
expected error
37+
expectedStderr string
3238
}{
3339
{
34-
name: "OK error",
35-
inputError: status.Error(codes.OK, "ok error"),
36-
expected: status.Error(codes.OK, "ok error"),
40+
name: "regular error",
41+
inputError: cerrors.New("some error"),
42+
expectedStderr: "some error",
3743
},
3844
{
39-
name: "Canceled error",
40-
inputError: status.Error(codes.Canceled, "canceled error"),
41-
expected: status.Error(codes.Canceled, "canceled error"),
45+
name: "Canceled error",
46+
inputError: status.Error(codes.Canceled, "canceled error"),
47+
expectedStderr: "canceled error",
4248
},
4349
{
44-
name: "Unknown error",
45-
inputError: status.Error(codes.Unknown, "unknown error"),
46-
expected: status.Error(codes.Unknown, "unknown error"),
50+
name: "Unknown error",
51+
inputError: status.Error(codes.Unknown, "unknown error"),
52+
expectedStderr: "unknown error",
4753
},
4854
{
49-
name: "InvalidArgument error",
50-
inputError: status.Error(codes.InvalidArgument, "invalid argument error"),
51-
expected: status.Error(codes.InvalidArgument, "invalid argument error"),
55+
name: "InvalidArgument error",
56+
inputError: status.Error(codes.InvalidArgument, "invalid argument error"),
57+
expectedStderr: "invalid argument error",
5258
},
5359
{
54-
name: "DeadlineExceeded error",
55-
inputError: status.Error(codes.DeadlineExceeded, "deadline exceeded error"),
56-
expected: status.Error(codes.DeadlineExceeded, "deadline exceeded error"),
60+
name: "DeadlineExceeded error",
61+
inputError: status.Error(codes.DeadlineExceeded, "deadline exceeded error"),
62+
expectedStderr: "deadline exceeded error",
5763
},
5864
{
59-
name: "NotFound error",
60-
inputError: status.Error(codes.NotFound, "not found error"),
61-
expected: nil,
65+
name: "NotFound error",
66+
inputError: status.Error(codes.NotFound, "not found error"),
67+
expectedStderr: "not found error",
6268
},
6369
{
64-
name: "AlreadyExists error",
65-
inputError: status.Error(codes.AlreadyExists, "already exists error"),
66-
expected: status.Error(codes.AlreadyExists, "already exists error"),
70+
name: "NotFound error with description",
71+
inputError: status.Error(codes.NotFound, "failed to get pipeline: rpc error: code = NotFound "+
72+
"desc = failed to get pipeline by ID: pipeline instance not found (ID: foo): pipeline instance not found"),
73+
expectedStderr: "failed to get pipeline by ID: pipeline instance not found (ID: foo): pipeline instance not found",
6774
},
6875
{
69-
name: "PermissionDenied error",
70-
inputError: status.Error(codes.PermissionDenied, "permission denied error"),
71-
expected: status.Error(codes.PermissionDenied, "permission denied error"),
76+
name: "AlreadyExists error",
77+
inputError: status.Error(codes.AlreadyExists, "already exists error"),
78+
expectedStderr: "already exists error",
7279
},
7380
{
74-
name: "ResourceExhausted error",
75-
inputError: status.Error(codes.ResourceExhausted, "resource exhausted error"),
76-
expected: status.Error(codes.ResourceExhausted, "resource exhausted error"),
81+
name: "PermissionDenied error",
82+
inputError: status.Error(codes.PermissionDenied, "permission denied error"),
83+
expectedStderr: "permission denied error",
7784
},
7885
{
79-
name: "FailedPrecondition error",
80-
inputError: status.Error(codes.FailedPrecondition, "failed precondition error"),
81-
expected: status.Error(codes.FailedPrecondition, "failed precondition error"),
86+
name: "ResourceExhausted error",
87+
inputError: status.Error(codes.ResourceExhausted, "resource exhausted error"),
88+
expectedStderr: "resource exhausted error",
8289
},
8390
{
84-
name: "Aborted error",
85-
inputError: status.Error(codes.Aborted, "aborted error"),
86-
expected: status.Error(codes.Aborted, "aborted error"),
91+
name: "FailedPrecondition error",
92+
inputError: status.Error(codes.FailedPrecondition, "failed precondition error"),
93+
expectedStderr: "failed precondition error",
8794
},
8895
{
89-
name: "OutOfRange error",
90-
inputError: status.Error(codes.OutOfRange, "out of range error"),
91-
expected: status.Error(codes.OutOfRange, "out of range error"),
96+
name: "Aborted error",
97+
inputError: status.Error(codes.Aborted, "aborted error"),
98+
expectedStderr: "aborted error",
9299
},
93100
{
94-
name: "Unimplemented error",
95-
inputError: status.Error(codes.Unimplemented, "unimplemented error"),
96-
expected: status.Error(codes.Unimplemented, "unimplemented error"),
101+
name: "OutOfRange error",
102+
inputError: status.Error(codes.OutOfRange, "out of range error"),
103+
expectedStderr: "out of range error",
97104
},
98105
{
99-
name: "Internal error",
100-
inputError: status.Error(codes.Internal, "internal error"),
101-
expected: status.Error(codes.Internal, "internal error"),
106+
name: "Unimplemented error",
107+
inputError: status.Error(codes.Unimplemented, "unimplemented error"),
108+
expectedStderr: "unimplemented error",
102109
},
103110
{
104-
name: "Unavailable error",
105-
inputError: status.Error(codes.Unavailable, "unavailable error"),
106-
expected: status.Error(codes.Unavailable, "unavailable error"),
111+
name: "Internal error",
112+
inputError: status.Error(codes.Internal, "internal error"),
113+
expectedStderr: "internal error",
107114
},
108115
{
109-
name: "DataLoss error",
110-
inputError: status.Error(codes.DataLoss, "data loss error"),
111-
expected: status.Error(codes.DataLoss, "data loss error"),
116+
name: "Unavailable error",
117+
inputError: status.Error(codes.Unavailable, "unavailable error"),
118+
expectedStderr: "unavailable error",
112119
},
113120
{
114-
name: "Unauthenticated error",
115-
inputError: status.Error(codes.Unauthenticated, "unauthenticated error"),
116-
expected: status.Error(codes.Unauthenticated, "unauthenticated error"),
121+
name: "DataLoss error",
122+
inputError: status.Error(codes.DataLoss, "data loss error"),
123+
expectedStderr: "data loss error",
124+
},
125+
{
126+
name: "Unauthenticated error",
127+
inputError: status.Error(codes.Unauthenticated, "unauthenticated error"),
128+
expectedStderr: "unauthenticated error",
117129
},
118130
}
119131

120132
for _, tc := range testCases {
121133
t.Run(tc.name, func(t *testing.T) {
122134
is := is.New(t)
123-
result := handleExecuteError(tc.inputError)
135+
136+
oldStderr := os.Stderr
137+
r, w, _ := os.Pipe()
138+
os.Stderr = w
139+
140+
result := handleError(tc.inputError)
141+
142+
w.Close()
143+
os.Stderr = oldStderr
144+
145+
var buf bytes.Buffer
146+
io.Copy(&buf, r)
147+
124148
is.Equal(result, tc.expected)
149+
is.Equal(strings.TrimSpace(buf.String()), tc.expectedStderr)
125150
})
126151
}
127152
}

0 commit comments

Comments
 (0)