From cf8c6156bd2e7a9497eff8c4586e1b6d245a186c Mon Sep 17 00:00:00 2001
From: Giuseppe Lo Presti <giuseppe.lopresti@cern.ch>
Date: Fri, 24 Sep 2021 10:10:39 +0200
Subject: [PATCH] Report errors thrown by the WOPI servers

---
 changelog/unreleased/app-errors.md                | 4 ++++
 internal/grpc/services/appprovider/appprovider.go | 5 ++---
 internal/http/services/appprovider/appprovider.go | 7 ++++---
 pkg/app/provider/wopi/wopi.go                     | 9 +++++----
 4 files changed, 15 insertions(+), 10 deletions(-)
 create mode 100644 changelog/unreleased/app-errors.md

diff --git a/changelog/unreleased/app-errors.md b/changelog/unreleased/app-errors.md
new file mode 100644
index 00000000000..7600ecb6512
--- /dev/null
+++ 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
index 8dd446d704c..d764a94c85f 100644
--- 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
index a1c7bd67c1d..9d733015b4f 100644
--- 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
index f81575f4972..ee8aa5e4bc9 100644
--- a/pkg/app/provider/wopi/wopi.go
+++ b/pkg/app/provider/wopi/wopi.go
@@ -167,15 +167,16 @@ func (p *wopiProvider) GetAppURL(ctx context.Context, resource *provider.Resourc
 	}
 	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 {
+		log.Warn().Msg(fmt.Sprintf("wopi: WOPI server returned HTTP %s, error was: %s", openRes.Status, body))
+		return nil, errors.New(string(body))
+	}
+
 	var result map[string]interface{}
 	err = json.Unmarshal(body, &result)
 	if err != nil {