Skip to content

Commit 1104316

Browse files
brianterryRJ Lohan
brianterry
authored and
RJ Lohan
committed
Fix error code (#100)
* Updates the cfn error outputs * Adds handler error codes * Adds test for panics
1 parent 6310d84 commit 1104316

File tree

7 files changed

+130
-57
lines changed

7 files changed

+130
-57
lines changed

‎cfn/cfn.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ func makeEventFunc(h Handler) eventFunc {
308308
}
309309

310310
if isMutatingAction(event.Action) {
311-
callbackAdapter.ReportStatus(translateStatus(progEvt.OperationStatus), event.RequestData.ResourceProperties, progEvt.Message, progEvt.HandlerErrorCode)
311+
callbackAdapter.ReportStatus(translateStatus(progEvt.OperationStatus), event.RequestData.ResourceProperties, progEvt.Message, string(r.ErrorCode))
312312
}
313313

314314
switch r.OperationStatus {

‎cfn/cfn_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"encoding/json"
66
"errors"
7+
"fmt"
78
"log"
89
"reflect"
910
"testing"
@@ -128,6 +129,21 @@ func TestMakeEventFunc(t *testing.T) {
128129
}
129130
}
130131

132+
f6 := func(callback map[string]interface{}, s *session.Session) (response handler.ProgressEvent) {
133+
defer func() {
134+
// Catch any panics and return a failed ProgressEvent
135+
if r := recover(); r != nil {
136+
err, ok := r.(error)
137+
if !ok {
138+
err = errors.New(fmt.Sprint(r))
139+
}
140+
141+
response = handler.NewFailedEvent(err)
142+
}
143+
}()
144+
panic("error")
145+
}
146+
131147
type args struct {
132148
h Handler
133149
ctx context.Context
@@ -170,6 +186,12 @@ func TestMakeEventFunc(t *testing.T) {
170186
{"Test invalid Action", args{&MockHandler{f1}, context.Background(), loadEvent("request.invalid.json", &event{})}, response{
171187
OperationStatus: handler.Failed,
172188
}, true},
189+
{"Test wrap panic", args{&MockHandler{f6}, context.Background(), loadEvent("request.create.json", &event{})}, response{
190+
OperationStatus: handler.Failed,
191+
ErrorCode: handler.GeneralServiceException,
192+
Message: "Unable to complete request: error",
193+
BearerToken: "123456",
194+
}, false},
173195
}
174196
for _, tt := range tests {
175197
t.Run(tt.name, func(t *testing.T) {

‎cfn/cfnerr/consts.go

Lines changed: 35 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,59 +1,56 @@
11
package cfnerr
22

33
const (
4-
// Unknown ...
5-
UnknownError = "UNKNOWN"
6-
74
// NotUpdatable is when the customer tried perform an update to a property that is CreateOnly. Only
85
// applicable to Update Handler. (Terminal)
9-
NotUpdatable = "NOTUPDATABLE"
6+
NotUpdatable = "NotUpdatable"
107

118
// InvalidRequest is a generic exception caused by invalid input from the customer. (Terminal)
12-
InvalidRequest = "INVALIDREQUEST"
9+
InvalidRequest = "InvalidRequest"
1310

14-
//AccessDenied is when the customer has insufficient permissions to perform this action. (Terminal)
15-
AccessDenied = "ACCESSDENIED"
11+
// AccessDenied is when the customer has insufficient permissions to perform this action. (Terminal)
12+
AccessDenied = "AccessDenied"
1613

17-
//InvalidCredentials is when the customer's provided credentials were invalid. (Terminal)
18-
InvalidCredentials = "INVALIDCREDENTIALS"
14+
// InvalidCredentials is when the customer's provided credentials were invalid. (Terminal)
15+
InvalidCredentials = "InvalidCredentials"
1916

20-
//AlreadyExists is when the specified resource already existed prior to the execution of the handler.
21-
//Only applicable to Create Handler (Terminal) Handlers MUST return this error
22-
//when duplicate creation requests are received.
23-
AlreadyExists = "ALREADYEXISTS"
17+
// AlreadyExists is when the specified resource already existed prior to the execution of the handler.
18+
// Only applicable to Create Handler (Terminal) Handlers MUST return this error
19+
// when duplicate creation requests are received.
20+
AlreadyExists = "AlreadyExists"
2421

25-
//NotFound is when the specified resource does not exist, or is in a terminal, inoperable, and
26-
//irrecoverable state. (Terminal)
27-
NotFound = "NOTFOUND"
22+
// NotFound is when the specified resource does not exist, or is in a terminal, inoperable, and
23+
// irrecoverable state. (Terminal)
24+
NotFound = "NotFound"
2825

29-
//ResourceConflict is when the resource is temporarily unable to be acted upon; for example, if the
30-
//resource is currently undergoing an operation and cannot be acted upon until
31-
//that operation is finished (Retriable)
32-
ResourceConflict = "RESOURCECONFLICT"
26+
// ResourceConflict is when the resource is temporarily unable to be acted upon; for example, if the
27+
// resource is currently undergoing an operation and cannot be acted upon until
28+
// that operation is finished (Retriable)
29+
ResourceConflict = "ResourceConflict"
3330

34-
//Throttling is when the request was throttled by the downstream service. (Retriable)
35-
Throttling = "THROTTLING"
31+
// Throttling is when the request was throttled by the downstream service. (Retriable)
32+
Throttling = "Throttling"
3633

37-
//ServiceLimitExceeded is when a non-transient resource limit was reached on the service side. (Terminal)
38-
ServiceLimitExceeded = "SERVICELIMITEXCEEDED"
34+
// ServiceLimitExceeded is when a non-transient resource limit was reached on the service side. (Terminal)
35+
ServiceLimitExceeded = "ServiceLimitExceeded"
3936

40-
//NotStabilized is when the downstream resource failed to complete all of its ready state checks.
41-
//(Retriable)
42-
NotStabilized = "NOTSTABILIZED"
37+
// NotStabilized is when the downstream resource failed to complete all of its ready state checks.
38+
// (Retriable)
39+
NotStabilized = "NotStabilized"
4340

44-
//GeneralServiceException is an exception from the downstream service that does not map to any other error
45-
//codes. (Terminal)
46-
GeneralServiceException = "GENERALSERVICEEXCEPTION"
41+
// GeneralServiceException is an exception from the downstream service that does not map to any other error
42+
// codes. (Terminal)
43+
GeneralServiceException = "GeneralServiceException"
4744

48-
//ServiceInternalis when the downstream service returned an internal error, typically with a 5XX HTTP
49-
//code. (Retriable)
50-
ServiceInternalError = "SERVICEINTERNALERROR"
45+
// ServiceInternalError is when the downstream service returned an internal error, typically with a 5XX HTTP
46+
// code. (Retriable)
47+
ServiceInternalError = "ServiceInternalError"
5148

52-
//NetworkFailure is when the request was unable to be completed due to networking issues, such as
53-
//failure to receive a response from the server. (Retriable)
54-
NetworkFailure = "NETWORKFAILURE"
49+
// NetworkFailure is when the request was unable to be completed due to networking issues, such as
50+
// failure to receive a response from the server. (Retriable)
51+
NetworkFailure = "NetworkFailure"
5552

5653
// InternalFailure is an unexpected error occurred within the handler, such as an NPE, etc.
57-
//(Terminal)
58-
InternalFailure = "INTERNALFAILURE"
54+
// (Terminal)
55+
InternalFailure = "InternalFailure"
5956
)

‎cfn/handler/errs.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
package handler
2+
3+
//ErrorCode represents errors that can be returned by the handler.
4+
type ErrorCode string
5+
6+
const (
7+
// NotUpdatable is when the customer tried perform an update to a property that is CreateOnly. Only
8+
// applicable to Update Handler. (Terminal)
9+
NotUpdatable = "NotUpdatable"
10+
11+
// InvalidRequest is a generic exception caused by invalid input from the customer. (Terminal)
12+
InvalidRequest = "InvalidRequest"
13+
14+
// AccessDenied is when the customer has insufficient permissions to perform this action. (Terminal)
15+
AccessDenied = "AccessDenied"
16+
17+
// InvalidCredentials is when the customer's provided credentials were invalid. (Terminal)
18+
InvalidCredentials = "InvalidCredentials"
19+
20+
// AlreadyExists is when the specified resource already existed prior to the execution of the handler.
21+
// Only applicable to Create Handler (Terminal) Handlers MUST return this error
22+
// when duplicate creation requests are received.
23+
AlreadyExists = "AlreadyExists"
24+
25+
// NotFound is when the specified resource does not exist, or is in a terminal, inoperable, and
26+
// irrecoverable state. (Terminal)
27+
NotFound = "NotFound"
28+
29+
// ResourceConflict is when the resource is temporarily unable to be acted upon; for example, if the
30+
// resource is currently undergoing an operation and cannot be acted upon until
31+
// that operation is finished (Retriable)
32+
ResourceConflict = "ResourceConflict"
33+
34+
// Throttling is when the request was throttled by the downstream service. (Retriable)
35+
Throttling = "Throttling"
36+
37+
// ServiceLimitExceeded is when a non-transient resource limit was reached on the service side. (Terminal)
38+
ServiceLimitExceeded = "ServiceLimitExceeded"
39+
40+
// NotStabilized is when the downstream resource failed to complete all of its ready state checks.
41+
// (Retriable)
42+
NotStabilized = "NotStabilized"
43+
44+
// GeneralServiceException is an exception from the downstream service that does not map to any other error
45+
// codes. (Terminal)
46+
GeneralServiceException = "GeneralServiceException"
47+
48+
// ServiceInternalError is when the downstream service returned an internal error, typically with a 5XX HTTP
49+
// code. (Retriable)
50+
ServiceInternalError = "ServiceInternalError"
51+
52+
// NetworkFailure is when the request was unable to be completed due to networking issues, such as
53+
// failure to receive a response from the server. (Retriable)
54+
NetworkFailure = "NetworkFailure"
55+
56+
// InternalFailure is an unexpected error occurred within the handler, such as an NPE, etc.
57+
// (Terminal)
58+
InternalFailure = "InternalFailure"
59+
)

‎cfn/handler/event.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package handler
22

3-
import (
4-
"github.com/aws-cloudformation/cloudformation-cli-go-plugin/cfn/cfnerr"
5-
)
3+
import "github.com/aws-cloudformation/cloudformation-cli-go-plugin/cfn/cfnerr"
64

75
// ProgressEvent represent the progress of CRUD handlers.
86
type ProgressEvent struct {
@@ -11,7 +9,7 @@ type ProgressEvent struct {
119
OperationStatus Status
1210

1311
// If OperationStatus is FAILED or IN_PROGRESS, an error code should be provided.
14-
HandlerErrorCode string
12+
HandlerErrorCode ErrorCode
1513

1614
// The handler can (and should) specify a contextual information message which
1715
// can be shown to callers to indicate the nature of a progress transition or
@@ -34,7 +32,7 @@ type ProgressEvent struct {
3432
ResourceModel interface{}
3533
}
3634

37-
// NewEvent creates a new event with
35+
// NewProgressEvent creates a new event with
3836
// a default OperationStatus of Unkown
3937
func NewProgressEvent() ProgressEvent {
4038
return ProgressEvent{
@@ -54,6 +52,6 @@ func NewFailedEvent(err error) ProgressEvent {
5452
return ProgressEvent{
5553
OperationStatus: Failed,
5654
Message: cerr.Message(),
57-
HandlerErrorCode: cerr.Code(),
55+
HandlerErrorCode: GeneralServiceException,
5856
}
5957
}

‎cfn/response.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,25 @@
11
package cfn
22

33
import (
4-
"github.com/aws-cloudformation/cloudformation-cli-go-plugin/cfn/cfnerr"
54
"github.com/aws-cloudformation/cloudformation-cli-go-plugin/cfn/handler"
65
)
76

87
// response represents a response to the
98
// cloudformation service from a resource handler.
109
// The zero value is ready to use.
1110
type response struct {
12-
Message string `json:"message,omitempty"`
13-
OperationStatus handler.Status `json:"operationStatus,omitempty"`
14-
ResourceModel interface{} `json:"resourceModel,omitempty"`
15-
ErrorCode cfnerr.Error `json:"errorCode,omitempty"`
16-
BearerToken string `json:"bearerToken,omitempty"`
11+
Message string `json:"message,omitempty"`
12+
OperationStatus handler.Status `json:"operationStatus,omitempty"`
13+
ResourceModel interface{} `json:"resourceModel,omitempty"`
14+
ErrorCode handler.ErrorCode `json:"errorCode,omitempty"`
15+
BearerToken string `json:"bearerToken,omitempty"`
1716
}
1817

1918
// newFailedResponse returns a response pre-filled with the supplied error
2019
func newFailedResponse(err error, bearerToken string) response {
2120
return response{
2221
OperationStatus: handler.Failed,
23-
ErrorCode: cfnerr.New(cfnerr.InternalFailure, "Unpexected error", err),
22+
ErrorCode: handler.InternalFailure,
2423
Message: err.Error(),
2524
BearerToken: bearerToken,
2625
}
@@ -37,7 +36,7 @@ func newResponse(pevt *handler.ProgressEvent, bearerToken string) (response, err
3736
}
3837

3938
if pevt.HandlerErrorCode != "" {
40-
resp.ErrorCode = cfnerr.New(pevt.HandlerErrorCode, pevt.Message, nil)
39+
resp.ErrorCode = pevt.HandlerErrorCode
4140
}
4241

4342
return resp, nil

‎cfn/response_test.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,12 @@
11
package cfn
22

33
import (
4-
"errors"
54
"testing"
65

76
"github.com/google/go-cmp/cmp"
87

98
"encoding/json"
109

11-
"github.com/aws-cloudformation/cloudformation-cli-go-plugin/cfn/cfnerr"
1210
"github.com/aws-cloudformation/cloudformation-cli-go-plugin/cfn/encoding"
1311
"github.com/aws-cloudformation/cloudformation-cli-go-plugin/cfn/handler"
1412
)
@@ -26,11 +24,11 @@ func TestMarshalJSON(t *testing.T) {
2624
Name: encoding.NewString("Douglas"),
2725
Version: encoding.NewFloat(42.1),
2826
},
29-
ErrorCode: cfnerr.New("baz", "quux", errors.New("mooz")),
27+
ErrorCode: handler.NotUpdatable,
3028
BearerToken: "xyzzy",
3129
}
3230

33-
expected := `{"message":"foo","operationStatus":"SUCCESS","resourceModel":{"Name":"Douglas","Version":"42.1"},"errorCode":"baz","bearerToken":"xyzzy"}`
31+
expected := `{"message":"foo","operationStatus":"SUCCESS","resourceModel":{"Name":"Douglas","Version":"42.1"},"errorCode":"NotUpdatable","bearerToken":"xyzzy"}`
3432

3533
actual, err := json.Marshal(r)
3634
if err != nil {

0 commit comments

Comments
 (0)