cloudapi/v2: include details in case internal error is set

In case details aren't explicitly given, and the internal error is set,
include the internal error message in the details.
This commit is contained in:
Sanne Raymaekers 2024-06-25 12:03:48 +02:00
parent 5ce8f65a58
commit 135dd94de5
4 changed files with 31 additions and 34 deletions

View file

@ -177,8 +177,12 @@ func HTTPError(code ServiceErrorCode) error {
// echo.HTTPError has a message interface{} field, which can be used to include the ServiceErrorCode // echo.HTTPError has a message interface{} field, which can be used to include the ServiceErrorCode
func HTTPErrorWithInternal(code ServiceErrorCode, internalErr error) error { func HTTPErrorWithInternal(code ServiceErrorCode, internalErr error) error {
se := find(code) de := detailsError{code, ""}
he := echo.NewHTTPError(se.httpStatus, detailsError{code, ""}) if internalErr != nil {
de.details = internalErr.Error()
}
he := echo.NewHTTPError(find(code).httpStatus, de)
if internalErr != nil { if internalErr != nil {
he.Internal = internalErr he.Internal = internalErr
} }
@ -202,28 +206,26 @@ func HTTPErrorWithDetails(code ServiceErrorCode, internalErr error, details stri
// Convert a ServiceErrorCode into an Error as defined in openapi.v2.yml // Convert a ServiceErrorCode into an Error as defined in openapi.v2.yml
// serviceError is optional, prevents multiple find() calls // serviceError is optional, prevents multiple find() calls
func APIError(code ServiceErrorCode, serviceError *serviceError, c echo.Context, details *interface{}) *Error { func APIError(serviceError *serviceError, c echo.Context, details interface{}) *Error {
se := serviceError
if se == nil {
se = find(code)
}
operationID, ok := c.Get("operationID").(string) operationID, ok := c.Get("operationID").(string)
if !ok || operationID == "" { if !ok || operationID == "" {
se = find(ErrorMalformedOperationID) serviceError = find(ErrorMalformedOperationID)
} }
return &Error{ apiErr := &Error{
ObjectReference: ObjectReference{ ObjectReference: ObjectReference{
Href: fmt.Sprintf("%s/%d", ErrorHREF, se.code), Href: fmt.Sprintf("%s/%d", ErrorHREF, serviceError.code),
Id: fmt.Sprintf("%d", se.code), Id: fmt.Sprintf("%d", serviceError.code),
Kind: "Error", Kind: "Error",
}, },
Code: fmt.Sprintf("%s%d", ErrorCodePrefix, se.code), Code: fmt.Sprintf("%s%d", ErrorCodePrefix, serviceError.code),
OperationId: operationID, // set operation id from context OperationId: operationID, // set operation id from context
Reason: se.reason, Reason: serviceError.reason,
Details: details,
} }
if details != nil {
apiErr.Details = &details
}
return apiErr
} }
// Helper to make the ErrorList as defined in openapi.v2.yml // Helper to make the ErrorList as defined in openapi.v2.yml
@ -253,7 +255,7 @@ func APIErrorList(page int, pageSize int, c echo.Context) *ErrorList {
for _, e := range errs { for _, e := range errs {
// Implicit memory alasing doesn't couse any bug in this case // Implicit memory alasing doesn't couse any bug in this case
/* #nosec G601 */ /* #nosec G601 */
list.Items = append(list.Items, *APIError(e.code, &e, c, nil)) list.Items = append(list.Items, *APIError(&e, c, nil))
} }
list.Size = len(list.Items) list.Size = len(list.Items)
return list return list
@ -274,14 +276,13 @@ func apiErrorFromEchoError(echoError *echo.HTTPError) ServiceErrorCode {
// Convert an echo error into an AOC compliant one so we send a correct json error response // Convert an echo error into an AOC compliant one so we send a correct json error response
func (s *Server) HTTPErrorHandler(echoError error, c echo.Context) { func (s *Server) HTTPErrorHandler(echoError error, c echo.Context) {
doResponse := func(details *interface{}, code ServiceErrorCode, c echo.Context, internal error) { doResponse := func(details interface{}, code ServiceErrorCode, c echo.Context, internal error) {
// don't anticipate serviceerrorcode, instead check what type it is
if !c.Response().Committed { if !c.Response().Committed {
var err error var err error
sec := find(code) se := find(code)
apiErr := APIError(code, sec, c, details) apiErr := APIError(se, c, details)
if sec.httpStatus == http.StatusInternalServerError { if se.httpStatus == http.StatusInternalServerError {
errMsg := fmt.Sprintf("Internal server error. Code: %s, OperationId: %s", apiErr.Code, apiErr.OperationId) errMsg := fmt.Sprintf("Internal server error. Code: %s, OperationId: %s", apiErr.Code, apiErr.OperationId)
if internal != nil { if internal != nil {
@ -292,9 +293,9 @@ func (s *Server) HTTPErrorHandler(echoError error, c echo.Context) {
} }
if c.Request().Method == http.MethodHead { if c.Request().Method == http.MethodHead {
err = c.NoContent(sec.httpStatus) err = c.NoContent(se.httpStatus)
} else { } else {
err = c.JSON(sec.httpStatus, apiErr) err = c.JSON(se.httpStatus, apiErr)
} }
if err != nil { if err != nil {
c.Logger().Errorf("Failed to return error response: %v", err) c.Logger().Errorf("Failed to return error response: %v", err)
@ -317,9 +318,5 @@ func (s *Server) HTTPErrorHandler(echoError error, c echo.Context) {
doResponse(nil, apiErrorFromEchoError(he), c, he.Internal) doResponse(nil, apiErrorFromEchoError(he), c, he.Internal)
return return
} }
var det *interface{} doResponse(err.details, err.errorCode, c, he.Internal)
if err.details != nil {
det = &err.details
}
doResponse(det, err.errorCode, c, he.Internal)
} }

View file

@ -25,7 +25,7 @@ func TestAPIError(t *testing.T) {
for _, se := range getServiceErrors() { for _, se := range getServiceErrors() {
ctx := e.NewContext(nil, nil) ctx := e.NewContext(nil, nil)
ctx.Set("operationID", "test-operation-id") ctx.Set("operationID", "test-operation-id")
apiError := APIError(se.code, nil, ctx, nil) apiError := APIError(&se, ctx, nil)
require.Equal(t, fmt.Sprintf("/api/image-builder-composer/v2/errors/%d", se.code), apiError.Href) require.Equal(t, fmt.Sprintf("/api/image-builder-composer/v2/errors/%d", se.code), apiError.Href)
require.Equal(t, fmt.Sprintf("%d", se.code), apiError.Id) require.Equal(t, fmt.Sprintf("%d", se.code), apiError.Id)
require.Equal(t, "Error", apiError.Kind) require.Equal(t, "Error", apiError.Kind)
@ -38,15 +38,15 @@ func TestAPIError(t *testing.T) {
func TestAPIErrorOperationID(t *testing.T) { func TestAPIErrorOperationID(t *testing.T) {
ctx := echo.New().NewContext(nil, nil) ctx := echo.New().NewContext(nil, nil)
apiError := APIError(ErrorUnauthenticated, nil, ctx, nil) apiError := APIError(find(ErrorUnauthenticated), ctx, nil)
require.Equal(t, "IMAGE-BUILDER-COMPOSER-10003", apiError.Code) require.Equal(t, "IMAGE-BUILDER-COMPOSER-10003", apiError.Code)
ctx.Set("operationID", 5) ctx.Set("operationID", 5)
apiError = APIError(ErrorUnauthenticated, nil, ctx, nil) apiError = APIError(find(ErrorUnauthenticated), ctx, nil)
require.Equal(t, "IMAGE-BUILDER-COMPOSER-10003", apiError.Code) require.Equal(t, "IMAGE-BUILDER-COMPOSER-10003", apiError.Code)
ctx.Set("operationID", "test-operation-id") ctx.Set("operationID", "test-operation-id")
apiError = APIError(ErrorUnauthenticated, nil, ctx, nil) apiError = APIError(find(ErrorUnauthenticated), ctx, nil)
require.Equal(t, "IMAGE-BUILDER-COMPOSER-401", apiError.Code) require.Equal(t, "IMAGE-BUILDER-COMPOSER-401", apiError.Code)
} }

View file

@ -77,7 +77,7 @@ func (h *apiHandlers) GetError(ctx echo.Context, id string) error {
return HTTPError(ErrorInvalidErrorId) return HTTPError(ErrorInvalidErrorId)
} }
apiError := APIError(ServiceErrorCode(errorId), nil, ctx, nil) apiError := APIError(find(ServiceErrorCode(errorId)), ctx, nil)
// If the service error wasn't found, it's a 404 in this instance // If the service error wasn't found, it's a 404 in this instance
if apiError.Id == fmt.Sprintf("%d", ErrorServiceErrorNotFound) { if apiError.Id == fmt.Sprintf("%d", ErrorServiceErrorNotFound) {
return HTTPError(ErrorErrorNotFound) return HTTPError(ErrorErrorNotFound)

View file

@ -597,6 +597,6 @@ func TestKojiJobTypeValidation(t *testing.T) {
} }
badID := uuid.New() badID := uuid.New()
test.TestRoute(t, handler, false, "GET", fmt.Sprintf("/api/image-builder-composer/v2/composes/%s%s", badID, path), ``, http.StatusNotFound, `{"code":"IMAGE-BUILDER-COMPOSER-15", "details": "", "href":"/api/image-builder-composer/v2/errors/15","id":"15","kind":"Error","reason":"Compose with given id not found"}`, `operation_id`) test.TestRoute(t, handler, false, "GET", fmt.Sprintf("/api/image-builder-composer/v2/composes/%s%s", badID, path), ``, http.StatusNotFound, `{"code":"IMAGE-BUILDER-COMPOSER-15", "details": "job does not exist", "href":"/api/image-builder-composer/v2/errors/15","id":"15","kind":"Error","reason":"Compose with given id not found"}`, `operation_id`)
} }
} }