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.

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

Improved error messages from the AppProviders #2258

Merged
merged 1 commit into from
Nov 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions changelog/unreleased/apps-errormsg.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Enhancement: Improved error messages from the AppProviders

Some rather cryptic messages are now hidden to users, and
some others are made more user-friendly. Support for multiple
locales is still missing and out of scope for now.

https://github.com/cs3org/reva/pull/2258
26 changes: 9 additions & 17 deletions internal/grpc/services/gateway/appprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (s *svc) OpenInApp(ctx context.Context, req *gateway.OpenInAppRequest) (*pr

if s.isSharedFolder(ctx, p) {
return &providerpb.OpenInAppResponse{
Status: status.NewInvalid(ctx, "gateway: can't open shares folder"),
Status: status.NewInvalid(ctx, "gateway: can't open shared folder"),
}, nil
}

Expand Down Expand Up @@ -151,10 +151,8 @@ func (s *svc) openFederatedShares(ctx context.Context, targetURL string, vm gate

conn, err := getConn(gatewayEP, insecure, skipVerify)
if err != nil {
err = errors.Wrap(err, "gateway: error connecting to remote reva")
return &providerpb.OpenInAppResponse{
Status: status.NewInternal(ctx, err, "error error connecting to remote reva"),
}, nil
log.Err(err).Msg("error connecting to remote reva")
return nil, errors.Wrap(err, "gateway: error connecting to remote reva")
}

gatewayClient := gateway.NewGatewayAPIClient(conn)
Expand All @@ -163,7 +161,7 @@ func (s *svc) openFederatedShares(ctx context.Context, targetURL string, vm gate

res, err := gatewayClient.OpenInApp(remoteCtx, appProviderReq)
if err != nil {
log.Err(err).Msg("error reaching remote reva")
log.Err(err).Msg("error returned by remote OpenInApp call")
return nil, errors.Wrap(err, "gateway: error calling OpenInApp")
}
return res, nil
Expand All @@ -182,23 +180,17 @@ func (s *svc) openLocalResources(ctx context.Context, ri *storageprovider.Resour
provider, err := s.findAppProvider(ctx, ri, app)
if err != nil {
err = errors.Wrap(err, "gateway: error calling findAppProvider")
var st *rpc.Status
if _, ok := err.(errtypes.IsNotFound); ok {
st = status.NewNotFound(ctx, "app provider not found")
} else {
st = status.NewInternal(ctx, err, "error searching for app provider")
return &providerpb.OpenInAppResponse{
Status: status.NewNotFound(ctx, "Could not find the requested app provider"),
}, nil
}
return &providerpb.OpenInAppResponse{
Status: st,
}, nil
return nil, err
}

appProviderClient, err := pool.GetAppProviderClient(provider.Address)
if err != nil {
err = errors.Wrap(err, "gateway: error calling GetAppProviderClient")
return &providerpb.OpenInAppResponse{
Status: status.NewInternal(ctx, err, "error getting appprovider client"),
}, nil
return nil, errors.Wrap(err, "gateway: error calling GetAppProviderClient")
}

appProviderReq := &providerpb.OpenInAppRequest{
Expand Down
16 changes: 9 additions & 7 deletions internal/http/services/appprovider/appprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,13 +264,13 @@ func (s *svc) handleOpen(w http.ResponseWriter, r *http.Request) {

client, err := pool.GetGatewayServiceClient(s.conf.GatewaySvc)
if err != nil {
ocmd.WriteError(w, r, ocmd.APIErrorServerError, "error getting grpc gateway client", err)
ocmd.WriteError(w, r, ocmd.APIErrorServerError, "Internal error with the gateway, please try again later", err)
return
}

info, errCode, err := s.getStatInfo(ctx, r.URL.Query().Get("file_id"), client)
if err != nil {
ocmd.WriteError(w, r, errCode, "error statting file", err)
ocmd.WriteError(w, r, errCode, "Internal error accessing the file, please try again later", err)
return
}

Expand All @@ -281,25 +281,27 @@ func (s *svc) handleOpen(w http.ResponseWriter, r *http.Request) {
}
openRes, err := client.OpenInApp(ctx, &openReq)
if err != nil {
log.Error().Err(err).Msg("error calling OpenInApp")
ocmd.WriteError(w, r, ocmd.APIErrorServerError, err.Error(), err)
ocmd.WriteError(w, r, ocmd.APIErrorServerError,
"Error contacting the requested application, please use a different one or try again later", err)
return
}
if openRes.Status.Code != rpc.Code_CODE_OK {
ocmd.WriteError(w, r, ocmd.APIErrorServerError, openRes.Status.Message,
status.NewErrorFromCode(openRes.Status.Code, "error calling OpenInApp"))
status.NewErrorFromCode(openRes.Status.Code, "Error calling OpenInApp"))
return
}

js, err := json.Marshal(openRes.AppUrl)
if err != nil {
ocmd.WriteError(w, r, ocmd.APIErrorServerError, "error marshalling JSON response", err)
ocmd.WriteError(w, r, ocmd.APIErrorServerError, "Internal error with JSON payload",
errors.Wrap(err, "error marshalling JSON response"))
return
}

w.Header().Set("Content-Type", "application/json")
if _, err = w.Write(js); err != nil {
ocmd.WriteError(w, r, ocmd.APIErrorServerError, "error writing JSON response", err)
ocmd.WriteError(w, r, ocmd.APIErrorServerError, "Internal error with JSON payload",
errors.Wrap(err, "error writing JSON response"))
return
}
}
Expand Down