CollaboraOnline / online

Collabora Online is a collaborative online office suite based on LibreOffice technology. This is also the source for the Collabora Office apps for iOS and Android.
https://collaboraonline.com
Other
1.78k stars 681 forks source link

Additional failure status codes for WOPI operations #8713

Open juliushaertl opened 5 months ago

juliushaertl commented 5 months ago

Is your feature request related to a problem?

When a wopi operation like saving the file fails, there is currently no possibility to give an error message or more detailed information to the user. The integrator can just return a limited list of HTTP status codes, but there are some cases where it would be befinicial if we could report more details to the user so they know why saving has failed.

This came up in https://github.com/nextcloud/richdocuments/issues/3281

Some examples could be:

Describe the solution you'd like

For some cases it would be good to have a defined status code where Collabora could then suggest the user to download a copy of the file to avoid loosing changes (if quarantine is not enabled or to avoid admins needing to go grab the file from there)

https://github.com/nextcloud/richdocuments/blob/4cb127a868e8b6e8edc3a39c1a5b78eb94c68916/lib/Controller/WopiController.php#L482

https://github.com/CollaboraOnline/online/blob/52a039e832872ad296387ef71bb1101c3b21d2a2/wsd/Storage.hpp#L318-L321

We could extend this list with common known errors.

In addition it would also be great to still have something like a COOLStatusMessage field in the response that integrators can send and is then displayed to the user in the error message when saving fails.

Describe alternatives you've considered

None

Additional context

None

Ashod commented 5 months ago

Some of these are already supported by the WOPI protocol and supported (with proper message to the user):

We can extend these error-codes and have matching user-visible messages added. Similarly for other cases. We just need to choose sensible HTTP codes for each case and agree on them.

juliushaertl commented 5 months ago

Thanks @Ashod that sounds very reasonable. Then we could use those existing cases and in addition add a COOLStatusMessage to the json response of the wopi operations for integrator defined errors messages that should be shown in addition.

On a separate note: I've just tried returning a 413 and it seems that coolwsd is handling that properly but the frontend code base just switches over to readonly without showing any error.

juliushaertl commented 5 months ago

@pedropintosilva In addition, i think in case of a save failure it would be nice if we could improve the error message to directly offer to download a local copy to to avoid loosing any changes.

Ashod commented 5 months ago

returning a 413 and it seems that coolwsd is handling that properly but the frontend code base just switches over to readonly without showing any error

@pedropintosilva this sounds like a regression. Properly we need to only restore the message that notifies the user of the issue (the error message already says the document will be readonly. We just need to display it (for some reason this._map.fire('warn', {msg: storageError}); is not working properly, I guess).