Skip to content

Commit

Permalink
fix/add tests and prioritize tag for errorcode
Browse files Browse the repository at this point in the history 
Signed-off-by: Jake Engelberg <[email protected]>
  • Loading branch information
@jake-engelberg
jake-engelberg committed Nov 22, 2024
1 parent 705b0d0 commit a0907ae
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 8 deletions.
12 changes: 7 additions & 5 deletions errors/errors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ type Error struct {
// Tag is a string identifying the error, used with HTTP responses only.
tag string

// Category is a string identifying the category of the error, used for error code metrics only.
// Category is a string identifying the category of the error (i.e. "actor", "job", "pubsub), used for error code metrics only.
category string
}

Expand Down Expand Up @@ -87,15 +87,17 @@ func (e *Error) GrpcStatusCode() grpcCodes.Code {
return e.grpcCode
}

// ErrorCode returns the error code from the error
// ErrorCode returns the error code from the error, prioritizing the legacy Error.Tag, otherwise the ErrorInfo.Reason
func (e *Error) ErrorCode() string {
errorCode := e.tag
for _, detail := range e.details {
if _, ok := detail.(*errdetails.ErrorInfo); ok {
_, errorCode := convertErrorDetails(detail, *e)
return errorCode
if _, errInfoReason := convertErrorDetails(detail, *e); errInfoReason != "" {
return errInfoReason
}
}
}
return ""
return errorCode
}

// Category returns the error code's category
Expand Down
47 changes: 44 additions & 3 deletions errors/errors_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ func TestError_HTTPStatusCode(t *testing.T) {
httpStatusCode,
"Test Msg",
"SOME_ERROR",
"some_category",
).
WithErrorInfo("fake", map[string]string{"fake": "test"}).
Build()
Expand All @@ -60,6 +61,7 @@ func TestError_GrpcStatusCode(t *testing.T) {
http.StatusTeapot,
"Test Msg",
"SOME_ERROR",
"some_category",
).
WithErrorInfo("fake", map[string]string{"fake": "test"}).
Build()
Expand Down Expand Up @@ -125,6 +127,7 @@ func TestError_Error(t *testing.T) {
http.StatusTeapot,
"Msg",
"SOME_ERROR",
"some_category",
).WithErrorInfo("fake", map[string]string{"fake": "test"}),
fields: fields{
message: "Msg",
Expand All @@ -139,6 +142,7 @@ func TestError_Error(t *testing.T) {
http.StatusTeapot,
"Msg",
"SOME_ERROR",
"some_category",
).WithErrorInfo("fake", map[string]string{"fake": "test"}),
fields: fields{
message: "Msg",
Expand All @@ -152,6 +156,7 @@ func TestError_Error(t *testing.T) {
http.StatusTeapot,
"Msg",
"SOME_ERROR",
"some_category",
).WithErrorInfo("fake", map[string]string{"fake": "test"}),
fields: fields{
grpcCode: grpcCodes.Canceled,
Expand Down Expand Up @@ -186,6 +191,7 @@ func TestErrorBuilder_WithErrorInfo(t *testing.T) {
httpCode: http.StatusTeapot,
message: "fake_message",
tag: "DAPR_FAKE_TAG",
category: "some_category",
details: []proto.Message{
details,
},
Expand All @@ -196,6 +202,7 @@ func TestErrorBuilder_WithErrorInfo(t *testing.T) {
http.StatusTeapot,
"fake_message",
"DAPR_FAKE_TAG",
"some_category",
).
WithErrorInfo(reason, metadata)

Expand All @@ -222,6 +229,7 @@ func TestErrorBuilder_WithDetails(t *testing.T) {
httpCode int
message string
tag string
category string
}

type args struct {
Expand Down Expand Up @@ -283,6 +291,7 @@ func TestErrorBuilder_WithDetails(t *testing.T) {
test.fields.httpCode,
test.fields.message,
test.fields.tag,
test.fields.category,
).WithDetails(test.args.a...)

assert.Equal(t, test.want, kitErr.Build())
Expand All @@ -292,7 +301,7 @@ func TestErrorBuilder_WithDetails(t *testing.T) {

func TestWithErrorHelp(t *testing.T) {
// Initialize the Error struct with some default values
err := NewBuilder(grpcCodes.InvalidArgument, http.StatusBadRequest, "Internal error", "INTERNAL_ERROR")
err := NewBuilder(grpcCodes.InvalidArgument, http.StatusBadRequest, "Internal error", "INTERNAL_ERROR", "some_category")

// Define test data for the help links
links := []*errdetails.Help_Link{
Expand All @@ -319,7 +328,7 @@ func TestWithErrorHelp(t *testing.T) {

func TestWithErrorFieldViolation(t *testing.T) {
// Initialize the Error struct with some default values
err := NewBuilder(grpcCodes.InvalidArgument, http.StatusBadRequest, "Internal error", "INTERNAL_ERROR")
err := NewBuilder(grpcCodes.InvalidArgument, http.StatusBadRequest, "Internal error", "INTERNAL_ERROR", "some_category")

// Define test data for the field violation
fieldName := "testField"
Expand Down Expand Up @@ -348,6 +357,7 @@ func TestError_JSONErrorValue(t *testing.T) {
httpCode int
message string
tag string
category string
}

tests := []struct {
Expand Down Expand Up @@ -657,7 +667,7 @@ func TestError_JSONErrorValue(t *testing.T) {

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
kitErr := NewBuilder(test.fields.grpcCode, test.fields.httpCode, test.fields.message, test.fields.tag).
kitErr := NewBuilder(test.fields.grpcCode, test.fields.httpCode, test.fields.message, test.fields.tag, test.fields.category).
WithDetails(test.fields.details...)

got := kitErr.err.JSONErrorValue()
Expand Down Expand Up @@ -705,6 +715,7 @@ func TestError_GRPCStatus(t *testing.T) {
httpCode int
message string
tag string
category string
}

tests := []struct {
Expand Down Expand Up @@ -769,6 +780,7 @@ func TestError_GRPCStatus(t *testing.T) {
test.fields.httpCode,
test.fields.message,
test.fields.tag,
test.fields.category,
).WithDetails(test.fields.details...)

got := kitErr.err.GRPCStatus()
Expand All @@ -787,6 +799,7 @@ func TestErrorBuilder_Build(t *testing.T) {
http.StatusTeapot,
"Test Msg",
"SOME_ERROR",
"some_category",
).WithErrorInfo("fake", map[string]string{"fake": "test"}).Build()

builtErr, ok := built.(Error)
Expand All @@ -803,6 +816,33 @@ func TestErrorBuilder_Build(t *testing.T) {
}

assert.True(t, containsErrorInfo)
assert.Equal(t, "SOME_ERROR", builtErr.ErrorCode())
})

t.Run("With_ErrorInfo (legacy tag absent)", func(t *testing.T) {
built := NewBuilder(
grpcCodes.ResourceExhausted,
http.StatusTeapot,
"Test Msg",
"",
"some_category",
).WithErrorInfo("SOME_ERROR", map[string]string{"fake": "test"}).Build()

builtErr, ok := built.(Error)
require.True(t, ok)

containsErrorInfo := false

for _, detail := range builtErr.details {
_, isErrInfo := detail.(*errdetails.ErrorInfo)
if isErrInfo {
containsErrorInfo = true
break
}
}

assert.True(t, containsErrorInfo)
assert.Equal(t, "SOME_ERROR", builtErr.ErrorCode())
})

t.Run("Without_ErrorInfo", func(t *testing.T) {
Expand All @@ -811,6 +851,7 @@ func TestErrorBuilder_Build(t *testing.T) {
http.StatusTeapot,
"Test Msg",
"SOME_ERROR",
"some_category",
)

assert.PanicsWithValue(t, "Must include ErrorInfo in error details.", func() {
Expand Down

0 comments on commit a0907ae

Please sign in to comment.