Report errors thrown by the WOPI servers

cs3org · Sep 24, 2021 · 8af8e6d · 8af8e6d
1 parent 2efc9c6
commit 8af8e6d
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 11 deletions.
diff --git a/changelog/unreleased/app-errors.md b/changelog/unreleased/app-errors.md
@@ -0,0 +1,4 @@
+Bugfix: AppProvider: propagate back errors reported by WOPI
+on /app/open and return base64-encoded fileids on /app/new
+
+https://github.com/cs3org/reva/pull/2103
diff --git a/internal/grpc/services/appprovider/appprovider.go b/internal/grpc/services/appprovider/appprovider.go
@@ -20,6 +20,7 @@ package appprovider
 
 import (
 	"context"
+	"errors"
 	"os"
 	"time"
 
@@ -35,7 +36,6 @@ import (
 	"github.com/cs3org/reva/pkg/rgrpc/todo/pool"
 	"github.com/cs3org/reva/pkg/sharedconf"
 	"github.com/mitchellh/mapstructure"
-	"github.com/pkg/errors"
 	"google.golang.org/grpc"
 )
 
@@ -145,9 +145,8 @@ func getProvider(c *config) (app.Provider, error) {
 func (s *service) OpenInApp(ctx context.Context, req *providerpb.OpenInAppRequest) (*providerpb.OpenInAppResponse, error) {
 	appURL, err := s.provider.GetAppURL(ctx, req.ResourceInfo, req.ViewMode, req.AccessToken)
 	if err != nil {
-		err := errors.Wrap(err, "appprovider: error calling GetAppURL")
 		res := &providerpb.OpenInAppResponse{
-			Status: status.NewInternal(ctx, err, "error getting app URL"),
+			Status: status.NewInternal(ctx, errors.New("appprovider: error calling GetAppURL"), err.Error()),
 		}
 		return res, nil
 	}

diff --git a/internal/http/services/appprovider/appprovider.go b/internal/http/services/appprovider/appprovider.go
@@ -281,12 +281,13 @@ func (s *svc) handleOpen(w http.ResponseWriter, r *http.Request) {
 	}
 	openRes, err := client.OpenInApp(ctx, &openReq)
 	if err != nil {
-		ocmd.WriteError(w, r, ocmd.APIErrorServerError, "error opening resource", err)
+		log.Error().Err(err).Msg("error calling OpenInApp")
+		ocmd.WriteError(w, r, ocmd.APIErrorServerError, err.Error(), err)
 		return
 	}
 	if openRes.Status.Code != rpc.Code_CODE_OK {
-		ocmd.WriteError(w, r, ocmd.APIErrorServerError, "error opening resource information",
-			status.NewErrorFromCode(openRes.Status.Code, "appprovider"))
+		ocmd.WriteError(w, r, ocmd.APIErrorServerError, openRes.Status.Message,
+			status.NewErrorFromCode(openRes.Status.Code, "error calling OpenInApp"))
 		return
 	}
 

diff --git a/pkg/app/provider/wopi/wopi.go b/pkg/app/provider/wopi/wopi.go
@@ -137,7 +137,7 @@ func (p *wopiProvider) GetAppURL(ctx context.Context, resource *provider.Resourc
 	q.Add("endpoint", resource.GetId().StorageId)
 	q.Add("viewmode", viewMode.String())
 	u, ok := ctxpkg.ContextGetUser(ctx)
-	if ok { // else defaults to "Anonymous Guest"
+	if ok { // else defaults to "Guest xyz"
 		q.Add("username", u.Username)
 	}
 
@@ -161,20 +161,22 @@ func (p *wopiProvider) GetAppURL(ctx context.Context, resource *provider.Resourc
 	httpReq.Header.Set("Authorization", "Bearer "+p.conf.IOPSecret)
 	httpReq.Header.Set("TokenHeader", token)
 
+	// Call the WOPI server and parse the response (body will always contain a payload)
 	openRes, err := p.wopiClient.Do(httpReq)
 	if err != nil {
 		return nil, errors.Wrap(err, "wopi: error performing open request to WOPI server")
 	}
 	defer openRes.Body.Close()
 
-	if openRes.StatusCode != http.StatusOK {
-		return nil, errtypes.InternalError("wopi: unexpected status from WOPI server: " + openRes.Status)
-	}
-
 	body, err := ioutil.ReadAll(openRes.Body)
 	if err != nil {
 		return nil, err
 	}
+	if openRes.StatusCode != http.StatusOK {
+		// /openinapp failed, body contains a user-friendly error message
+		log.Warn().Msg(fmt.Sprintf("wopi: WOPI server returned HTTP %s, error was: %s", openRes.Status, string(body)))
+		return nil, errors.New(string(body))
+	}
 
 	var result map[string]interface{}
 	err = json.Unmarshal(body, &result)