Improved error messages from the AppProviders (#2258)

cs3org · Nov 18, 2021 · d147381 · d147381
1 parent 4380948
commit d147381
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 24 deletions.
diff --git a/changelog/unreleased/apps-errormsg.md b/changelog/unreleased/apps-errormsg.md
@@ -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
diff --git a/internal/grpc/services/gateway/appprovider.go b/internal/grpc/services/gateway/appprovider.go
@@ -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
 	}
 
@@ -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)
@@ -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
@@ -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{

diff --git a/internal/http/services/appprovider/appprovider.go b/internal/http/services/appprovider/appprovider.go
@@ -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
 	}
 
@@ -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
 	}
 }