Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Deprecate using errors.New and fmt.Errorf (#1673) #1680

Merged
merged 4 commits into from
May 7, 2021
Merged

Deprecate using errors.New and fmt.Errorf (#1673) #1680

merged 4 commits into from
May 7, 2021

Conversation

gmgigi96
Copy link
Member

@gmgigi96 gmgigi96 commented May 4, 2021 •
edited by ishank011
Loading

Closes #1673

@gmgigi96 gmgigi96 requested a review from labkode as a code owner May 4, 2021 12:48
@update-docs
Copy link

update-docs bot commented May 4, 2021

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@ishank011
Copy link
Contributor

@gmgigi96 thanks a lot for the change! Can you update the appprovider unit tests to expect the updated message? We can merge it then

ishank011
ishank011 suggested changes May 6, 2021
View reviewed changes
internal/grpc/services/appprovider/appprovider.go Outdated
@@ -131,7 +132,7 @@ func (s *service) getWopiAppEndpoints(ctx context.Context) (map[string]interface
}
defer appsRes.Body.Close()
if appsRes.StatusCode != http.StatusOK {
return nil, fmt.Errorf("Request to WOPI server returned %d", appsRes.StatusCode)
return nil, errtypes.PermissionDenied(fmt.Sprintf("Request to WOPI server returned %d", appsRes.StatusCode))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change this to InternalError

internal/grpc/services/appprovider/appprovider.go Outdated
@@ -263,7 +264,7 @@ func (s *service) OpenFileInAppProvider(ctx context.Context, req *providerpb.Ope
}
defer bridgeRes.Body.Close()
if bridgeRes.StatusCode != http.StatusFound {
return nil, fmt.Errorf("Request to WOPI bridge returned %d", bridgeRes.StatusCode)
return nil, errtypes.PermissionDenied(fmt.Sprintf("Request to WOPI bridge returned %d", bridgeRes.StatusCode))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change this to InternalError

internal/grpc/services/authprovider/authprovider.go Outdated
@@ -66,12 +66,12 @@ func parseConfig(m map[string]interface{}) (*config, error) {

func getAuthManager(manager string, m map[string]map[string]interface{}) (auth.Manager, error) {
if manager == "" {
return nil, errors.New("authsvc: driver not configured for auth manager")
return nil, errtypes.NotFound("authsvc: driver not configured for auth manager")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change this to InternalError

internal/grpc/services/gateway/appprovider.go Outdated
@@ -246,7 +246,7 @@ func (s *svc) findAppProvider(ctx context.Context, ri *storageprovider.ResourceI
return nil, errtypes.NotFound("gateway: app provider not found for resource: " + ri.String())
}

return nil, errors.New("gateway: error finding a storage provider")
return nil, errtypes.NotFound("gateway: error finding a storage provider")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change this to InternalError, NotFound will be handled by the check above

internal/grpc/services/gateway/authprovider.go Outdated
@@ -191,5 +191,5 @@ func (s *svc) findAuthProvider(ctx context.Context, authType string) (provider.P
return nil, errtypes.NotFound("gateway: auth provider not found for type:" + authType)
}

return nil, errors.New("gateway: error finding an auth provider for type: " + authType)
return nil, errtypes.NotFound("gateway: error finding an auth provider for type: " + authType)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change this to InternalError, NotFound will be handled by the check above

internal/grpc/services/ocmshareprovider/ocmshareprovider.go Outdated
}, nil
}

var permissions string
permOpaque, ok := req.Opaque.Map["permissions"]
if !ok {
return &ocm.CreateOCMShareResponse{
Status: status.NewInternal(ctx, errors.New("resource permissions not set"), ""),
Status: status.NewInternal(ctx, errtypes.NotFound("resource permissions not set"), ""),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BadRequest

internal/grpc/services/ocmshareprovider/ocmshareprovider.go Outdated
@@ -131,14 +131,14 @@ func (s *service) CreateOCMShare(ctx context.Context, req *ocm.CreateOCMShareReq
nameOpaque, ok := req.Opaque.Map["name"]
if !ok {
return &ocm.CreateOCMShareResponse{
Status: status.NewInternal(ctx, errors.New("resource name not set"), ""),
Status: status.NewInternal(ctx, errtypes.NotFound("resource name not set"), ""),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BadRequest

internal/grpc/services/storageprovider/storageprovider.go Outdated
@@ -116,7 +116,7 @@ func parseXSTypes(xsTypes map[string]uint32) ([]*provider.ResourceChecksumPriori
for xs, prio := range xsTypes {
t := PKG2GRPCXS(xs)
if t == provider.ResourceChecksumType_RESOURCE_CHECKSUM_TYPE_INVALID {
return nil, fmt.Errorf("checksum type is invalid: %s", xs)
return nil, errtypes.NotFound("checksum type is invalid: " + xs)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BadRequest

internal/grpc/services/storageprovider/storageprovider.go Outdated
@@ -299,7 +299,7 @@ func (s *service) InitiateFileUpload(ctx context.Context, req *provider.Initiate
}
if newRef.GetPath() == "/" {
return &provider.InitiateFileUploadResponse{
Status: status.NewInternal(ctx, errors.New("can't upload to mount path"), "can't upload to mount path"),
Status: status.NewInternal(ctx, errtypes.InternalError("can't upload to mount path"), "can't upload to mount path"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BadRequest

internal/grpc/services/storageprovider/storageprovider.go Outdated
@@ -486,7 +486,7 @@ func (s *service) Delete(ctx context.Context, req *provider.DeleteRequest) (*pro
}
if newRef.GetPath() == "/" {
return &provider.DeleteResponse{
Status: status.NewInternal(ctx, errors.New("can't delete mount path"), "can't delete mount path"),
Status: status.NewInternal(ctx, errtypes.InternalError("can't delete mount path"), "can't delete mount path"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BadRequest

@ishank011 ishank011 merged commit a34018a into cs3org:master May 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ishank011 ishank011 ishank011 approved these changes

@labkode labkode Awaiting requested review from labkode

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Deprecate using errors.New and fmt.Errorf
2 participants
@gmgigi96 @ishank011