mirror of
https://github.com/opencloud-eu/opencloud.git
synced 2026-02-25 06:58:59 -06:00
always use error interface when returning errors (#8816)
* always use error interface when returning errors Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de> * fix: use non pointer Error * fix: errorcode evaluation --------- Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de> Co-authored-by: Florian Schade <f.schade@icloud.com>
This commit is contained in:
Jörn Friedrich Dreyer
committed by
GitHub
GitHub
parent
fad37db840
commit
7a635738fa
@@ -17,12 +17,12 @@ import (
|
||||
//
|
||||
// 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, inerr error, 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}
|
||||
return Error{msg: inerr.Error(), errorCode: GeneralException}
|
||||
}
|
||||
|
||||
err := &Error{errorCode: GeneralException, msg: "unspecified error has occurred"}
|
||||
err := Error{errorCode: GeneralException, msg: "unspecified error has occurred"}
|
||||
|
||||
if status != nil {
|
||||
err.msg = status.GetMessage()
|
||||
@@ -33,7 +33,7 @@ func FromCS3Status(status *cs3rpc.Status, inerr error, ignore ...cs3rpc.Code) *E
|
||||
case slices.Contains(ignore, status.GetCode()):
|
||||
fallthrough
|
||||
case code == cs3rpc.Code_CODE_OK:
|
||||
err = nil
|
||||
return nil
|
||||
case code == cs3rpc.Code_CODE_NOT_FOUND:
|
||||
err.errorCode = ItemNotFound
|
||||
case code == cs3rpc.Code_CODE_PERMISSION_DENIED:
|
||||
@@ -61,11 +61,11 @@ func FromCS3Status(status *cs3rpc.Status, inerr error, ignore ...cs3rpc.Code) *E
|
||||
return err
|
||||
}
|
||||
|
||||
// FromStat transforms a *provider.StatResponse object and an error into an *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.
|
||||
// It invokes the FromCS3Status function with the StatResponse Status and the ignore codes.
|
||||
func FromStat(stat *provider.StatResponse, err error, ignore ...cs3rpc.Code) *Error {
|
||||
func FromStat(stat *provider.StatResponse, err error, ignore ...cs3rpc.Code) error {
|
||||
// TODO: look into ResourceInfo to get the postprocessing state and map that to 425 status?
|
||||
return FromCS3Status(stat.GetStatus(), err, ignore...)
|
||||
}
|
||||
|
||||
@@ -8,45 +8,44 @@ import (
|
||||
cs3rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
|
||||
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
|
||||
|
||||
"github.com/owncloud/ocis/v2/ocis-pkg/conversions"
|
||||
"github.com/owncloud/ocis/v2/services/graph/pkg/errorcode"
|
||||
)
|
||||
|
||||
func TestFromCS3Status(t *testing.T) {
|
||||
var tests = []struct {
|
||||
status *cs3rpc.Status
|
||||
err error
|
||||
ignore []cs3rpc.Code
|
||||
result *errorcode.Error
|
||||
status *cs3rpc.Status
|
||||
err error
|
||||
ignore []cs3rpc.Code
|
||||
expected error
|
||||
}{
|
||||
{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"))},
|
||||
{nil, nil, nil, errorcode.New(errorcode.GeneralException, "unspecified error has occurred")},
|
||||
{nil, errors.New("test error"), nil, 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.InvalidRequest, "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"))},
|
||||
{&cs3rpc.Status{Code: cs3rpc.Code_CODE_NOT_FOUND, Message: "msg"}, nil, nil, errorcode.New(errorcode.ItemNotFound, "msg")},
|
||||
{&cs3rpc.Status{Code: cs3rpc.Code_CODE_PERMISSION_DENIED, Message: "msg"}, nil, nil, errorcode.New(errorcode.AccessDenied, "msg")},
|
||||
{&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNAUTHENTICATED, Message: "msg"}, nil, nil, errorcode.New(errorcode.Unauthenticated, "msg")},
|
||||
{&cs3rpc.Status{Code: cs3rpc.Code_CODE_INVALID_ARGUMENT, Message: "msg"}, nil, nil, errorcode.New(errorcode.InvalidRequest, "msg")},
|
||||
{&cs3rpc.Status{Code: cs3rpc.Code_CODE_ALREADY_EXISTS, Message: "msg"}, nil, nil, errorcode.New(errorcode.NameAlreadyExists, "msg")},
|
||||
{&cs3rpc.Status{Code: cs3rpc.Code_CODE_FAILED_PRECONDITION, Message: "msg"}, nil, nil, errorcode.New(errorcode.InvalidRequest, "msg")},
|
||||
{&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNIMPLEMENTED, Message: "msg"}, nil, nil, errorcode.New(errorcode.NotSupported, "msg")},
|
||||
{&cs3rpc.Status{Code: cs3rpc.Code_CODE_INVALID, Message: "msg"}, nil, nil, errorcode.New(errorcode.GeneralException, "msg")},
|
||||
{&cs3rpc.Status{Code: cs3rpc.Code_CODE_CANCELLED, Message: "msg"}, nil, nil, errorcode.New(errorcode.GeneralException, "msg")},
|
||||
{&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNKNOWN, Message: "msg"}, nil, nil, errorcode.New(errorcode.GeneralException, "msg")},
|
||||
{&cs3rpc.Status{Code: cs3rpc.Code_CODE_RESOURCE_EXHAUSTED, Message: "msg"}, nil, nil, errorcode.New(errorcode.GeneralException, "msg")},
|
||||
{&cs3rpc.Status{Code: cs3rpc.Code_CODE_ABORTED, Message: "msg"}, nil, nil, errorcode.New(errorcode.GeneralException, "msg")},
|
||||
{&cs3rpc.Status{Code: cs3rpc.Code_CODE_OUT_OF_RANGE, Message: "msg"}, nil, nil, errorcode.New(errorcode.InvalidRange, "msg")},
|
||||
{&cs3rpc.Status{Code: cs3rpc.Code_CODE_INTERNAL, Message: "msg"}, nil, nil, errorcode.New(errorcode.GeneralException, "msg")},
|
||||
{&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNAVAILABLE, Message: "msg"}, nil, nil, errorcode.New(errorcode.ServiceNotAvailable, "msg")},
|
||||
{&cs3rpc.Status{Code: cs3rpc.Code_CODE_REDIRECTION, Message: "msg"}, nil, nil, errorcode.New(errorcode.GeneralException, "msg")},
|
||||
{&cs3rpc.Status{Code: cs3rpc.Code_CODE_INSUFFICIENT_STORAGE, Message: "msg"}, nil, nil, errorcode.New(errorcode.QuotaLimitReached, "msg")},
|
||||
{&cs3rpc.Status{Code: cs3rpc.Code_CODE_LOCKED, Message: "msg"}, nil, nil, errorcode.New(errorcode.ItemIsLocked, "msg")},
|
||||
}
|
||||
|
||||
for _, test := range tests {
|
||||
if output := errorcode.FromCS3Status(test.status, test.err, test.ignore...); !reflect.DeepEqual(output, test.result) {
|
||||
t.Error("Test Failed: {} expected, received: {}", test.result, output)
|
||||
if got := errorcode.FromCS3Status(test.status, test.err, test.ignore...); !reflect.DeepEqual(got, test.expected) {
|
||||
t.Error("Test Failed: {} expected, received: {}", test.expected, got)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -55,9 +54,9 @@ func TestFromStat(t *testing.T) {
|
||||
var tests = []struct {
|
||||
stat *provider.StatResponse
|
||||
err error
|
||||
result *errorcode.Error
|
||||
result error
|
||||
}{
|
||||
{nil, errors.New("some error"), conversions.ToPointer(errorcode.New(errorcode.GeneralException, "some error"))},
|
||||
{nil, errors.New("some error"), errorcode.New(errorcode.GeneralException, "some error")},
|
||||
{&provider.StatResponse{Status: &cs3rpc.Status{Code: cs3rpc.Code_CODE_OK}}, nil, nil},
|
||||
}
|
||||
|
||||
|
||||
@@ -166,7 +166,8 @@ func RenderError(w http.ResponseWriter, r *http.Request, err error) {
|
||||
var errcode Error
|
||||
if errors.As(err, &errcode) {
|
||||
errcode.Render(w, r)
|
||||
} else {
|
||||
GeneralException.Render(w, r, http.StatusInternalServerError, err.Error())
|
||||
return
|
||||
}
|
||||
|
||||
GeneralException.Render(w, r, http.StatusInternalServerError, err.Error())
|
||||
}
|
||||
|
||||
45
services/graph/pkg/errorcode/errorcode_test.go
Normal file
45
services/graph/pkg/errorcode/errorcode_test.go
Normal file
@@ -0,0 +1,45 @@
|
||||
package errorcode_test
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/owncloud/ocis/v2/services/graph/pkg/errorcode"
|
||||
)
|
||||
|
||||
type customErr struct{}
|
||||
|
||||
func (customErr) Error() string {
|
||||
return "some error"
|
||||
}
|
||||
|
||||
func TestRenderError(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
t.Run("errorcode.Error value error", func(t *testing.T) {
|
||||
r := httptest.NewRequest("GET", "/", nil)
|
||||
w := httptest.NewRecorder()
|
||||
err := errorcode.New(errorcode.ItemNotFound, "test error")
|
||||
errorcode.RenderError(w, r, err)
|
||||
require.Equal(t, http.StatusNotFound, w.Code)
|
||||
})
|
||||
|
||||
t.Run("errorcode.Error zero value error", func(t *testing.T) {
|
||||
r := httptest.NewRequest("GET", "/", nil)
|
||||
w := httptest.NewRecorder()
|
||||
var err errorcode.Error
|
||||
errorcode.RenderError(w, r, err)
|
||||
require.Equal(t, http.StatusForbidden, w.Code)
|
||||
})
|
||||
|
||||
t.Run("custom error", func(t *testing.T) {
|
||||
r := httptest.NewRequest("GET", "/", nil)
|
||||
w := httptest.NewRecorder()
|
||||
var err customErr
|
||||
errorcode.RenderError(w, r, err)
|
||||
require.Equal(t, http.StatusInternalServerError, w.Code)
|
||||
})
|
||||
}
|
||||
Reference in New Issue
Block a user