Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conflict / overwrite issues with TUS #1294

Closed
PVince81 opened this issue May 13, 2020 · 12 comments
Closed

Conflict / overwrite issues with TUS #1294

PVince81 opened this issue May 13, 2020 · 12 comments

Comments

@PVince81
Copy link
Contributor

Run the following acceptance tests with Phoenix:

  • tests/acceptance/features/webUIUpload/uploadEdgecases.feature:123
    tests/acceptance/features/webUIUpload/upload.feature:84

The failure seems to be related to how the new TUS based implementation handles conflicts or overwrites.

@PVince81 PVince81 self-assigned this May 13, 2020
@PVince81
Copy link
Contributor Author

1) Scenario: Upload a file with the same name as already existing folder # tests/acceptance/features/webUIUpload/uploadEdgecases.feature:123                                                                                                                                  
   ✔ Before # tests/acceptance/setup.js:28
   ✔ Before # tests/acceptance/setup.js:32
   ✔ Before # tests/acceptance/setup.js:36
   ✔ Before # tests/acceptance/setup.js:46
   ✔ Before # tests/acceptance/setup.js:68
   ✔ Before # tests/acceptance/setup.js:75
   ✔ Before # tests/acceptance/stepDefinitions/filesContext.js:15
   ✔ Before # tests/acceptance/stepDefinitions/generalContext.js:191
   ✔ Before # tests/acceptance/stepDefinitions/generalContext.js:230
   ✔ Given user "user1" has been created with default attributes # tests/acceptance/stepDefinitions/provisioningContext.js:126
   ✔ And user "user1" has logged in using the webUI # tests/acceptance/stepDefinitions/loginContext.js:63
   ✔ And the user browses to the files page # tests/acceptance/stepDefinitions/filesContext.js:19
   ✔ Given the user has created folder "new-lorem.txt" # tests/acceptance/stepDefinitions/filesContext.js:743
   ✔ And the user has reloaded the current page of the webUI # tests/acceptance/stepDefinitions/filesContext.js:423
   ✔ When the user uploads overwriting file "new-lorem.txt" using the webUI # tests/acceptance/stepDefinitions/filesContext.js:853
   ✖ Then the following error message should be displayed on the webUI # tests/acceptance/stepDefinitions/generalContext.js:53
       """
       File upload failed…
       PUT is not allowed on non-files.
       """
       Timed out while waiting for element <//*[contains(@class, "uk-notification-message")]> to be present for 10000 milliseconds. - expected "visible" but got: "not found" (10016ms)
           at World.<anonymous> (/home/vincent/Private/Work/workspace/phoenix/tests/acceptance/stepDefinitions/generalContext.js:56:6)
   ✔ After # tests/acceptance/stepDefinitions/provisioningContext.js:200
   ✔ After # tests/acceptance/stepDefinitions/generalContext.js:235
   ✔ After # tests/acceptance/stepDefinitions/generalContext.js:215
   ✔ After # tests/acceptance/setup.js:110
   ✔ After # tests/acceptance/setup.js:106
   ✔ After # tests/acceptance/setup.js:102
   ✔ After # tests/acceptance/setup.js:95
   ✔ After # tests/acceptance/setup.js:86
   ✔ After # tests/acceptance/setup.js:54

1 scenario (1 failed)
7 steps (1 failed, 6 passed)
0m26.722s

@PVince81
Copy link
Contributor Author

It looks like this is due to not correctly handling errors during TUS upload.

So far the TUS client is retrying the upload several times, despite the 409 status code.
Then at the end it displays a generic error message.

We should make it fail directly in case of 409 or 403.

Also, potential issue: if we are parsing the error message from the XML it won't work as OCIS doesn't contain an error body yet: https://github.com/owncloud/ocis-reva/issues/199

@PVince81
Copy link
Contributor Author

It seems the library is retrying on 409 error codes, so our test will timeout before we reach the end result. There is currently no wait to make the TUS client abort directly: tus/tus-js-client#196

A possible workaround would be to make the retry delays configurable and have those specific tests set them to shorter values. Or increase the timeout for which we wait for errors...

@PVince81
Copy link
Contributor Author

I've sent an upstream PR to provide a callback: tus/tus-js-client#197

@PVince81
Copy link
Contributor Author

PR here: owncloud/web#3472

but requires a decision on how to return error bodies in OCIS

@PVince81
Copy link
Contributor Author

PVince81 commented Jun 2, 2020

still waiting for upstream PR to be merged

@PVince81
Copy link
Contributor Author

PVince81 commented Jul 9, 2020

now waiting for upstream to make a release

if this becomes urgent we could use the matching commit instead

@PVince81 PVince81 removed their assignment Sep 3, 2020
@butonic butonic transferred this issue from owncloud/ocis-reva Jan 18, 2021
@refs refs added the Topic:TUS label Jan 18, 2021
@settings settings bot removed the Topic:TUS label Jan 29, 2021
@settings settings bot removed the p3-medium label Apr 7, 2021
@stale
Copy link

stale bot commented Jun 6, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 10 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Status:Stale label Jun 6, 2021
@stale stale bot closed this as completed Jun 16, 2021
@stale stale bot removed the Status:Stale label Jun 18, 2021
@micbar
Copy link
Contributor

micbar commented Jul 12, 2021

fixed by #2280

@stale
Copy link

stale bot commented Sep 10, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 10 days if no further activity occurs. Thank you for your contributions.

@phil-davis
Copy link
Contributor

https://drone.owncloud.com/owncloud/ocis/9644/50/9
webUIUpload/upload.feature:94 failed.

  Scenario: conflict with a big file (when chunking is implemented this upload should be chunked) # features/webUIUpload/upload.feature:94
- Connecting to selenium on port 4444...
ℹ Connected to selenium on port 4444 (431ms).
  Using: chrome (94.0.4606.61) on Linux platform.
    Given user "Alice" has been created with default attributes and without skeleton files in the server
    And user "Alice" has created folder "simple-folder" in the server
    And user "Alice" has uploaded file with content "initial content" to "lorem.txt" in the server
    And user "Alice" has uploaded file with content "initial content" to "simple-folder/lorem.txt" in the server
    And user "Alice" has logged in using the webUI
√ Element <input[autocomplete="kopano-account username"]> was visible after 941 milliseconds.
√ Element <input[autocomplete="kopano-account username"]> was not present after 114 milliseconds.
√ Element <.app-content> was visible after 776 milliseconds.
    Given a file with the size of "30000000" bytes and the name "big-video.mp4" has been created locally in the middleware
    When the user renames file "lorem.txt" to "big-video.mp4" using the webUI
   NoSuchElementError: An error occurred while running .isVisible() command on <//*[@id="files-sidebar"]>: 
   {"sessionId":"b6543720ba07329cf2388f8d814a6ce9","status":0,"value":[]}
       at Proxy.isSideBarOpen (/srv/app/web/tests/acceptance/pageObjects/FilesPageElement/appSideBar.js:30:18)
       at Proxy.isSideBarOpenForResource (/srv/app/web/tests/acceptance/pageObjects/FilesPageElement/appSideBar.js:52:24)
       at Proxy.openSideBar (/srv/app/web/tests/acceptance/pageObjects/FilesPageElement/filesList.js:246:28)
       at Proxy.openFileActionsMenu (/srv/app/web/tests/acceptance/pageObjects/FilesPageElement/filesList.js:232:18)
       at Proxy.renameFile (/srv/app/web/tests/acceptance/pageObjects/FilesPageElement/filesList.js:88:18)
       at World.<anonymous> (/srv/app/web/tests/acceptance/stepDefinitions/filesContext.js:256:53)
   NoSuchElementError: An error occurred while running .isVisible() command on <//*[@id="files-sidebar"]>: 
   {"sessionId":"b6543720ba07329cf2388f8d814a6ce9","status":0,"value":[]}
       at Proxy.isSideBarOpen (/srv/app/web/tests/acceptance/pageObjects/FilesPageElement/appSideBar.js:30:18)
       at Proxy.closeSidebarIfOpen (/srv/app/web/tests/acceptance/pageObjects/FilesPageElement/appSideBar.js:13:24)
       at Proxy.openSideBar (/srv/app/web/tests/acceptance/pageObjects/FilesPageElement/filesList.js:252:24)
       at runMicrotasks (<anonymous>)
       at processTicksAndRejections (internal/process/task_queues.js:95:5)
       at Proxy.openFileActionsMenu (/srv/app/web/tests/acceptance/pageObjects/FilesPageElement/filesList.js:232:7)
       at Proxy.renameFile (/srv/app/web/tests/acceptance/pageObjects/FilesPageElement/filesList.js:88:7)
   NoSuchElementError: An error occurred while running .isVisible() command on <//*[@id="files-sidebar"]>: 
   {"sessionId":"b6543720ba07329cf2388f8d814a6ce9","status":0,"value":[]}
       at Proxy.isSideBarOpen (/srv/app/web/tests/acceptance/pageObjects/FilesPageElement/appSideBar.js:30:18)
       at Proxy.closeSidebarIfOpen (/srv/app/web/tests/acceptance/pageObjects/FilesPageElement/appSideBar.js:13:24)
       at Proxy.waitForFileVisible (/srv/app/web/tests/acceptance/pageObjects/FilesPageElement/filesList.js:338:24)
       at runMicrotasks (<anonymous>)
       at processTicksAndRejections (internal/process/task_queues.js:95:5)
       at Proxy.clickRow (/srv/app/web/tests/acceptance/pageObjects/FilesPageElement/filesList.js:265:7)
       at Proxy.openSideBar (/srv/app/web/tests/acceptance/pageObjects/FilesPageElement/filesList.js:255:7)
       at Proxy.openFileActionsMenu (/srv/app/web/tests/acceptance/pageObjects/FilesPageElement/filesList.js:232:7)
       at Proxy.renameFile (/srv/app/web/tests/acceptance/pageObjects/FilesPageElement/filesList.js:88:7)
√ Element <//span[contains(@class, "oc-resource-name") and (@data-test-resource-name='lorem.txt' or @data-test-resource-path='/lorem.txt')]/ancestor::tr[contains(@class, "oc-tbody-tr")]> was present after 21 milliseconds.
√ Element <.files-table .oc-tbody-tr> was present after 19 milliseconds.
waiting for 500ms ...
   NoSuchElementError: An error occurred while running .isVisible() command on <//div[contains(@id, "files-sidebar")]//*[contains(@class, "is-active-sub-panel")]//button[contains(@class, "header__back")]>: 
   {"sessionId":"b6543720ba07329cf2388f8d814a6ce9","status":0,"value":[]}
       at Proxy.activatePanel (/srv/app/web/tests/acceptance/pageObjects/FilesPageElement/appSideBar.js:80:20)
       at runMicrotasks (<anonymous>)
       at processTicksAndRejections (internal/process/task_queues.js:95:5)
       at Proxy.openFileActionsMenu (/srv/app/web/tests/acceptance/pageObjects/FilesPageElement/filesList.js:233:7)
       at Proxy.renameFile (/srv/app/web/tests/acceptance/pageObjects/FilesPageElement/filesList.js:88:7)
Timeout waiting for Ajax calls to start
waiting for 500ms ...
√ Element <//*[@id="sidebar-panel-actions-item"]//*[contains(@class, "sidebar-panel__body-content")]> was present after 9 milliseconds.
√ Element <.oc-modal> was visible after 35 milliseconds.
waiting for 500ms ...
√ Element <.oc-modal> was not present after 102 milliseconds.
    And the user reloads the current page of the webUI
    And the user uploads a created file "big-video.mp4" with overwrite using the webUI
    Then file "big-video.mp4" should be listed on the webUI
√ Element <#new-file-menu-btn:enabled> was visible after 816 milliseconds.
√ Element <#files-file-upload-button> was visible after 65 milliseconds.
waiting for 500ms ...
   NoSuchElementError: An error occurred while running .isVisible() command on <//*[@id="files-sidebar"]>: 
   {"sessionId":"b6543720ba07329cf2388f8d814a6ce9","status":0,"value":[]}
       at Proxy.isSideBarOpen (/srv/app/web/tests/acceptance/pageObjects/FilesPageElement/appSideBar.js:30:18)
       at Proxy.closeSidebarIfOpen (/srv/app/web/tests/acceptance/pageObjects/FilesPageElement/appSideBar.js:13:24)
       at Proxy.waitForFileVisible (/srv/app/web/tests/acceptance/pageObjects/FilesPageElement/filesList.js:338:24)
       at runMicrotasks (<anonymous>)
       at processTicksAndRejections (internal/process/task_queues.js:95:5)
√ Element <.oc-modal> was not present after 21 milliseconds.
Timeout waiting for Ajax calls to start
√ Element <//span[contains(@class, "oc-resource-name") and (@data-test-resource-name='big-video.mp4' or @data-test-resource-path='/big-video.mp4') and @data-test-resource-type='file']/ancestor::tr[contains(@class, "oc-tbody-tr")]> was present after 24 milliseconds.
√ Element <.files-table .oc-tbody-tr> was present after 11 milliseconds.
    And as "Alice" the content of "big-video.mp4" in the server should be the same as the content of local file "big-video.mp4"
    ✖ failed
      Error: AssertionError [ERR_ASSERTION]: asserting content of local file "/uploads/big-video.mp4"
          at assertContentOfLocalFileIs (/usr/src/app/src/stepDefinitions/filesContext.js:151:17)
          at StepDef.action (/usr/src/app/src/stepDefinitions/filesContext.js:133:5)
          at processTicksAndRejections (node:internal/process/task_queues:96:5)
          at async /usr/src/app/src/app.js:66:7
          at /srv/app/web/tests/acceptance/stepDefinitions/middlewareContext.js:38:15
          at runMicrotasks (<anonymous>)
          at processTicksAndRejections (internal/process/task_queues.js:95:5)

Maybe this fail is related?

@saw-jan
Copy link
Member

saw-jan commented Jan 31, 2024

File overwrite works and covered in webUI scenario: https://github.com/owncloud/web/blob/752ab3a836d4c44edefa925981aa759bcd11da38/tests/e2e/cucumber/features/smoke/upload.feature#L20-L22

And webUIUpload/upload.feature:95 also works

Scenario: conflict with a big file (when chunking is implemented this upload should be chunked)

So, closing this

@saw-jan saw-jan closed this as completed Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants