diff --git a/changelog/unreleased/app-errors.md b/changelog/unreleased/app-errors.md new file mode 100644 index 0000000000..7600ecb651 --- /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 8dd446d704..d764a94c85 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 a3aa60c468..30f949a714 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 f81575f497..ec0c689805 100644 --- 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,26 @@ 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 { + // WOPI returned failure: body contains a user-friendly error message (yet perform a sanity check) + sbody := "" + if body != nil { + sbody = string(body) + } + log.Warn().Msg(fmt.Sprintf("wopi: WOPI server returned HTTP %s, error was: %s", openRes.Status, sbody)) + return nil, errors.New(sbody) + } var result map[string]interface{} err = json.Unmarshal(body, &result)