owncloud / ocis

:atom_symbol: ownCloud Infinite Scale Stack
https://doc.owncloud.com/ocis/next/
Apache License 2.0
1.36k stars 180 forks source link

Only Office could not be loaded if editor is already open #4366

Closed hurradieweltgehtunter closed 2 years ago

hurradieweltgehtunter commented 2 years ago

Steps to reproduce

  1. Upload an office file (in my case xlsx)
  2. Click on it -> OnlyOffice opens in new tab
  3. Switch back to file manager, click on the file again or click any other file of the same file type (xlsx)

Expected behaviour

File should be opened again or the tab with the previously opened file should be focused (if this is at all possible)

Actual behaviour

New tab opens with error message "Sorry, editor could not be loaded. Please contact your administrator."

Environment general

https://ocis.ocis-wopi.latest.owncloud.works/

Client configuration

Browser: Firefox 103.0.1

Operating system: MacOS 12.4

Browser log

No errors visible in console

https://user-images.githubusercontent.com/16665512/183595175-30e461ae-6e5d-4b63-b780-ae231111ae4a.mov

wkloucek commented 2 years ago

~The "Sorry, editor could not be loaded. Please contact your administrator." error will be fixed by https://github.com/owncloud/ocis/pull/4329.~

Needs some investigation. Is most likely a backend issue, therefore I move this issue to the oCIS Repo

wkloucek commented 2 years ago

Seems to be a bug with OnlyOffice. With Collabora, I could open multiple tabs.

micbar commented 2 years ago

Same happens to me when opening the file with two different users.

Logs

ocis-wopiserver-1                 | ERROR:wopiserver:"module": "cs3iface", "msg": "Failed to setlock", "filepath": "d5efa44e-ff1c-4ee3-a9dd-984142b84d0e/Neue Datei.docx", "appname": "OnlyOffice", "value": "opaquelocktoken:797356a8-0500-4ceb-a8a0-c94c8cde7eba c1h4OGVWck9zeEZYMjNjWFRVQ3phZz09", "trace": "51f62c05d18b1c5e8f41f586988bf78e", "code": "12", "reason": "set lock: error: aborted: already locked"
ocis-wopiserver-1                 | {"time": "2022-08-11T09:21:57.844", "host": "dcdeacba2ed2", "level": "ERROR", "process": "wopiserver", "module": "cs3iface", "msg": "Failed to setlock", "filepath": "d5efa44e-ff1c-4ee3-a9dd-984142b84d0e/Neue Datei.docx", "appname": "OnlyOffice", "value": "opaquelocktoken:797356a8-0500-4ceb-a8a0-c94c8cde7eba c1h4OGVWck9zeEZYMjNjWFRVQ3phZz09", "trace": "51f62c05d18b1c5e8f41f586988bf78e", "code": "12", "reason": "set lock: error: aborted: already locked"}
ocis-wopiserver-1                 | ERROR:wopiserver:"module": "wopi", "msg": "Unable to store WOPI lock", "lockop": "Lock", "filename": "d5efa44e-ff1c-4ee3-a9dd-984142b84d0e/Neue Datei.docx", "token": "7mPaZ61FOCJbpKKrIwJs", "lock": "sXx8eVrOsxFX23cXTUCzag==", "error": "set lock: error: aborted: already locked"
ocis-wopiserver-1                 | {"time": "2022-08-11T09:21:57.846", "host": "dcdeacba2ed2", "level": "ERROR", "process": "wopiserver", "module": "wopi", "msg": "Unable to store WOPI lock", "lockop": "Lock", "filename": "d5efa44e-ff1c-4ee3-a9dd-984142b84d0e/Neue Datei.docx", "token": "7mPaZ61FOCJbpKKrIwJs", "lock": "sXx8eVrOsxFX23cXTUCzag==", "error": "set lock: error: aborted: already locked"}
ocis-onlyoffice-1                 | [2022-08-11T09:21:57.850] [ERROR] nodeJS - wopi error LOCK:Error: Error response: statusCode:500; headers:{"content-length":"33","content-type":"text/html; charset=utf-8","date":"Thu, 11 Aug 2022 09:21:57 GMT","server":"waitress"}; body:
ocis-onlyoffice-1                 | I/O Error, please contact support
ocis-onlyoffice-1                 |     at Request._callback (/snapshot/server/build/server/Common/sources/utils.js)
ocis-onlyoffice-1                 |     at Request.callback (/snapshot/server/build/server/Common/node_modules/request/request.js:185:22)
ocis-onlyoffice-1                 |     at Request.emit (events.js:400:28)
ocis-onlyoffice-1                 |     at Request.<anonymous> (/snapshot/server/build/server/Common/node_modules/request/request.js:1154:10)
ocis-onlyoffice-1                 |     at Request.emit (events.js:400:28)
ocis-onlyoffice-1                 |     at IncomingMessage.<anonymous> (/snapshot/server/build/server/Common/node_modules/request/request.js:1076:12)
ocis-onlyoffice-1                 |     at Object.onceWrapper (events.js:519:28)
ocis-onlyoffice-1                 |     at IncomingMessage.emit (events.js:412:35)
ocis-onlyoffice-1                 |     at endReadableNT (internal/streams/readable.js:1317:12)
ocis-onlyoffice-1                 |     at processTicksAndRejections (internal/process/task_queues.js:82:21)
wkloucek commented 2 years ago

The cs3org/wopiserver is not behaving accordingly to the wopi spec from here https://docs.microsoft.com/en-us/microsoft-365/cloud-storage-partner-program/rest/files/lock.

This is because of a response code in reva changed when trying to lock an already locked file. This would be a workaround on the wopiserver side: https://github.com/wkloucek/wopiserver/pull/3/files.

But we should fix it in REVA I think.

wkloucek commented 2 years ago

The changed return code was introduced in https://github.com/cs3org/reva/pull/3003 and is here:

https://github.com/cs3org/reva/blob/999fef2426efa3d8b46edbae8c17db61cf383d08/pkg/storage/utils/decomposedfs/node/locks.go#L70

butonic commented 2 years ago

I rember reading docs that mentioned locks and etags but can no longer find them. Hoowever, rereading https://developers.google.com/maps-booking/reference/grpc-api/status_codes I would highlight this:

Use ABORTED if the client should retry at a higher-level, such as when a client-specified test-and-set fails which indicates that the client should restart a read-modify-write sequence.

For me a client-specified test-and-set translates into the http preconditions that the client sents along, which is what WOPI does as well, does it not? That is why I would argue that the GRPC API should use tha ABORTED code.

You could argue that if the client does not send the LockID in the request the serler neets to respond with a PRECONDITION_FAILED code ... but that just means that clients need to handle both cases.

For now I'd make the wopi client treat ABORTED and PRECONDITION_FAILED the same. Which is backwards compatible.

@wkloucek does that explanation help?

butonic commented 2 years ago

hah! In https://cloud.google.com/apis/design/errors#generating_errors you can find:

HTTP gRPC Example Error Message
400 FAILED_PRECONDITION Resource xxx is a non-empty directory, so it cannot be deleted.
409 ABORTED Couldn’t acquire lock on resource ‘xxx’.
butonic commented 2 years ago

To be clear: when a client sends a request without a condition, eg. please lock file /foo, then the server MUST return a PRECONDITION_FAILED error. The client did not specify a 'test' the server should perform before creating the lock.

wkloucek commented 2 years ago

will be fixed when https://github.com/cs3org/reva/pull/3157 arrives in oCIS

wkloucek commented 2 years ago

Will be fixed with 2.0.0-beta.7

ScharfViktor commented 2 years ago

I could reproduce if same file opened in collabora and only office [Release 2.0.0-beta.7] (https://github.com/owncloud/ocis/issues/4425#top)

Steps:

or admin opens file first in collabora, second in only office

Actual: image

ocis_wopi_onlyoffice log:

[2022-08-25T12:27:42.135] [ERROR] nodeJS - wopi error LOCK:Error: Error response: statusCode:409; headers:{"content-length":"46","content-type":"application/json","date":"Thu, 25 Aug 2022 12:27:42 GMT","server":"waitress","x-wopi-lock":"cool-lockb82b20ff","x-wopi-lockfailurereason":"The file is locked by Collabora"}; body:

{"message": "The file is locked by Collabora"}

    at Request._callback (/snapshot/server/build/server/Common/sources/utils.js)

    at Request.callback (/snapshot/server/build/server/Common/node_modules/request/request.js:185:22)

    at Request.emit (events.js:400:28)

    at Request.<anonymous> (/snapshot/server/build/server/Common/node_modules/request/request.js:1154:10)

    at Request.emit (events.js:400:28)

    at IncomingMessage.<anonymous> (/snapshot/server/build/server/Common/node_modules/request/request.js:1076:12)

    at Object.onceWrapper (events.js:519:28)

    at IncomingMessage.emit (events.js:412:35)

    at endReadableNT (internal/streams/readable.js:1334:12)

    at processTicksAndRejections (internal/process/task_queues.js:82:21)