From 9d3523e310ecfb995616430e9d97e2de62975e9d Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Wed, 29 Nov 2023 11:23:15 +0100 Subject: [PATCH] graph/errocode: Rework FromCS3Status The handling of 'error' has been moved from FromStat() to FromCS3Status(). It's generally useful for other users of FromCS3Status() --- services/graph/pkg/errorcode/cs3.go | 23 +++++----- services/graph/pkg/errorcode/cs3_test.go | 48 +++++++++++---------- services/graph/pkg/service/v0/driveitems.go | 9 +--- 3 files changed, 39 insertions(+), 41 deletions(-) diff --git a/services/graph/pkg/errorcode/cs3.go b/services/graph/pkg/errorcode/cs3.go index 129e464e3d..756cf271a4 100644 --- a/services/graph/pkg/errorcode/cs3.go +++ b/services/graph/pkg/errorcode/cs3.go @@ -7,15 +7,21 @@ import ( provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" ) -// FromCS3Status converts a CS3 status code into a corresponding local Error representation. +// FromCS3Status converts a CS3 status code and an error into a corresponding local Error representation. // -// It evaluates the provided CS3 status code and returns an equivalent graph Error. +// It takes a *cs3rpc.Status, an error, and a variadic parameter of type cs3rpc.Code. +// If the error is not nil, it creates an Error object with the error message and a GeneralException code. +// If the error is nil, it evaluates the provided CS3 status code and returns an equivalent graph Error. // If the CS3 status code does not have a direct equivalent within the app, // or is ignored, a general purpose Error is returned. // // This function is particularly useful when dealing with CS3 responses, // and a unified error handling within the application is necessary. -func FromCS3Status(status *cs3rpc.Status, ignore ...cs3rpc.Code) *Error { +func FromCS3Status(status *cs3rpc.Status, inerr error, ignore ...cs3rpc.Code) *Error { + if inerr != nil { + return &Error{msg: inerr.Error(), errorCode: GeneralException} + } + err := &Error{errorCode: GeneralException, msg: "unspecified error has occurred"} if status != nil { @@ -58,13 +64,8 @@ func FromCS3Status(status *cs3rpc.Status, ignore ...cs3rpc.Code) *Error { // FromStat transforms a *provider.StatResponse object and an error into an *Error. // // It takes a stat of type *provider.StatResponse, an error, and a variadic parameter of type cs3rpc.Code. -// If the error is not nil, it creates an Error object with the error message and a GeneralException code. -// If the error is nil, it invokes the FromCS3Status function with the StatResponse Status and the ignore codes. +// It invokes the FromCS3Status function with the StatResponse Status and the ignore codes. func FromStat(stat *provider.StatResponse, err error, ignore ...cs3rpc.Code) *Error { - switch { - case err != nil: - return &Error{msg: err.Error(), errorCode: GeneralException} - default: - return FromCS3Status(stat.GetStatus(), ignore...) - } + // TODO: look into ResourceInfo to get the postprocessing state and map that to 425 status? + return FromCS3Status(stat.GetStatus(), err, ignore...) } diff --git a/services/graph/pkg/errorcode/cs3_test.go b/services/graph/pkg/errorcode/cs3_test.go index b53fe37116..22616342e5 100644 --- a/services/graph/pkg/errorcode/cs3_test.go +++ b/services/graph/pkg/errorcode/cs3_test.go @@ -15,35 +15,37 @@ import ( func TestFromCS3Status(t *testing.T) { var tests = []struct { status *cs3rpc.Status + err error ignore []cs3rpc.Code result *errorcode.Error }{ - {nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "unspecified error has occurred"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_OK}, nil, nil}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_NOT_FOUND}, []cs3rpc.Code{cs3rpc.Code_CODE_NOT_FOUND}, nil}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_PERMISSION_DENIED}, []cs3rpc.Code{cs3rpc.Code_CODE_NOT_FOUND, cs3rpc.Code_CODE_PERMISSION_DENIED}, nil}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_NOT_FOUND, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.ItemNotFound, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_PERMISSION_DENIED, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.AccessDenied, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNAUTHENTICATED, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.Unauthenticated, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INVALID_ARGUMENT, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.InvalidRequest, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_ALREADY_EXISTS, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.NameAlreadyExists, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_FAILED_PRECONDITION, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.PreconditionFailed, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNIMPLEMENTED, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.NotSupported, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INVALID, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_CANCELLED, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNKNOWN, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_RESOURCE_EXHAUSTED, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_ABORTED, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_OUT_OF_RANGE, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INTERNAL, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNAVAILABLE, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_REDIRECTION, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INSUFFICIENT_STORAGE, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_LOCKED, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, + {nil, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "unspecified error has occurred"))}, + {nil, errors.New("test error"), nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "test error"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_OK}, nil, nil, nil}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_NOT_FOUND}, nil, []cs3rpc.Code{cs3rpc.Code_CODE_NOT_FOUND}, nil}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_PERMISSION_DENIED}, nil, []cs3rpc.Code{cs3rpc.Code_CODE_NOT_FOUND, cs3rpc.Code_CODE_PERMISSION_DENIED}, nil}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_NOT_FOUND, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.ItemNotFound, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_PERMISSION_DENIED, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.AccessDenied, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNAUTHENTICATED, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.Unauthenticated, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INVALID_ARGUMENT, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.InvalidRequest, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_ALREADY_EXISTS, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.NameAlreadyExists, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_FAILED_PRECONDITION, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.PreconditionFailed, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNIMPLEMENTED, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.NotSupported, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INVALID, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_CANCELLED, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNKNOWN, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_RESOURCE_EXHAUSTED, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_ABORTED, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_OUT_OF_RANGE, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.InvalidRange, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INTERNAL, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNAVAILABLE, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.ServiceNotAvailable, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_REDIRECTION, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INSUFFICIENT_STORAGE, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.QuotaLimitReached, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_LOCKED, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.ItemIsLocked, "msg"))}, } for _, test := range tests { - if output := errorcode.FromCS3Status(test.status, test.ignore...); !reflect.DeepEqual(output, test.result) { + if output := errorcode.FromCS3Status(test.status, test.err, test.ignore...); !reflect.DeepEqual(output, test.result) { t.Error("Test Failed: {} expected, recieved: {}", test.result, output) } } diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 55b5081023..5318e632b3 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -536,13 +536,8 @@ func (g Graph) DeletePermission(w http.ResponseWriter, r *http.Request) { }, }, }) - switch { - case err != nil: - fallthrough - case getShareResp.Status.GetCode() != cs3rpc.Code_CODE_OK: - g.logger.Debug().Err(err).Interface("permissionID", permissionID).Interface("GetShare", getShareResp).Msg("GetShare failed") - errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) - return + if errCode := errorcode.FromCS3Status(getShareResp.GetStatus(), err); errCode != nil { + return nil, err } sharedResourceId := getShareResp.GetShare().GetResourceId()