From b39dff814061fc40f21422428b47f208c0bf5442 Mon Sep 17 00:00:00 2001
From: Jim <jlambert@hashicorp.com>
Date: Sat, 20 Mar 2021 06:52:47 -0400
Subject: [PATCH] New error codes. (#1019)

* new error codes

* Add general system errors to API interceptor

Co-authored-by: Louis Ruch <louisruch@gmail.com>
---
 internal/errors/code.go                       |  4 ++++
 internal/errors/code_test.go                  | 10 ++++++++
 internal/errors/info.go                       |  8 +++++++
 .../servers/controller/handlers/errors.go     | 10 ++++++--
 .../controller/handlers/errors_test.go        | 23 +++++++++++++++++++
 5 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/internal/errors/code.go b/internal/errors/code.go
index 9f1ca2ff7..bc8e186af 100644
--- a/internal/errors/code.go
+++ b/internal/errors/code.go
@@ -58,6 +58,10 @@ const (
 	GenKey  Code = 304 // GenKey represents an error occurred during the underlying key generation process
 	GenCert Code = 305 // GenCert represents an error occurred during the underlying certificate generation process
 
+	// General system errors are reserved Codes 400-599 and align with http client and server error codes
+	Forbidden Code = 403 // Forbidden represents the operation is forbidden
+	Internal  Code = 500 // InternalError represents the system encountered an unexpected condition.
+
 	// DB errors are reserved Codes from 1000-1999
 	CheckConstraint      Code = 1000 // CheckConstraint represents a check constraint error
 	NotNull              Code = 1001 // NotNull represents a value must not be null error
diff --git a/internal/errors/code_test.go b/internal/errors/code_test.go
index 46ecb10ef..2737ab033 100644
--- a/internal/errors/code_test.go
+++ b/internal/errors/code_test.go
@@ -97,6 +97,16 @@ func TestCode_Both_String_Info(t *testing.T) {
 			c:    TooShort,
 			want: TooShort,
 		},
+		{
+			name: "InternalError",
+			c:    Internal,
+			want: Internal,
+		},
+		{
+			name: "Forbidden",
+			c:    Forbidden,
+			want: Forbidden,
+		},
 		{
 			name: "PasswordTooShort",
 			c:    PasswordTooShort,
diff --git a/internal/errors/info.go b/internal/errors/info.go
index 6f0e2109b..860d8cc16 100644
--- a/internal/errors/info.go
+++ b/internal/errors/info.go
@@ -112,6 +112,14 @@ var errorCodeInfo = map[Code]Info{
 		Message: "error occurred during certification generation",
 		Kind:    Encryption,
 	},
+	Internal: {
+		Message: "internal error",
+		Kind:    Other,
+	},
+	Forbidden: {
+		Message: "forbidden",
+		Kind:    Other,
+	},
 	CheckConstraint: {
 		Message: "constraint check failed",
 		Kind:    Integrity,
diff --git a/internal/servers/controller/handlers/errors.go b/internal/servers/controller/handlers/errors.go
index e16b80418..8ea725c1b 100644
--- a/internal/servers/controller/handlers/errors.go
+++ b/internal/servers/controller/handlers/errors.go
@@ -161,11 +161,17 @@ func backendErrorToApiError(inErr error) error {
 		return InvalidArgumentErrorf(genericUniquenessMsg, nil)
 	}
 
-	// We haven't been able to identify what this backend error is, return it as an internal error
+	var statusCode int32 = http.StatusInternalServerError
+	var domainErr *errors.Err
+	if errors.As(inErr, &domainErr) && domainErr.Code >= 400 && domainErr.Code <= 599 {
+		// Domain error codes 400-599 align with http client and server error codes, use the domain error code instead of 500
+		statusCode = int32(domainErr.Code)
+	}
+
 	// TODO: Don't return potentially sensitive information (like which user id an account
 	//  is already associated with when attempting to re-associate it).
 	return &apiError{
-		status: http.StatusInternalServerError,
+		status: statusCode,
 		inner:  &pb.Error{Kind: codes.Internal.String(), Message: inErr.Error()},
 	}
 }
diff --git a/internal/servers/controller/handlers/errors_test.go b/internal/servers/controller/handlers/errors_test.go
index 50daa6ace..d9d3c09d3 100644
--- a/internal/servers/controller/handlers/errors_test.go
+++ b/internal/servers/controller/handlers/errors_test.go
@@ -3,6 +3,7 @@ package handlers
 import (
 	"context"
 	stderrors "errors"
+	"fmt"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
@@ -194,6 +195,28 @@ func TestApiErrorHandler(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "Forbidden domain error",
+			err:  errors.E(errors.WithCode(errors.Forbidden), errors.WithMsg("test msg")),
+			expected: apiError{
+				status: http.StatusForbidden,
+				inner: &pb.Error{
+					Kind:    "Internal",
+					Message: "test msg: unknown: error #403",
+				},
+			},
+		},
+		{
+			name: "Wrapped forbidden domain error",
+			err:  fmt.Errorf("got error: %w", errors.E(errors.WithCode(errors.Forbidden), errors.WithMsg("test msg"))),
+			expected: apiError{
+				status: http.StatusForbidden,
+				inner: &pb.Error{
+					Kind:    "Internal",
+					Message: "got error: test msg: unknown: error #403",
+				},
+			},
+		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {