Skip to content

fix(store): Prevent out-of-order server responses to mess up the folder listing#62032

Open
DerDreschner wants to merge 2 commits into
masterfrom
fix/race-condition-on-sort
Open

fix(store): Prevent out-of-order server responses to mess up the folder listing#62032
DerDreschner wants to merge 2 commits into
masterfrom
fix/race-condition-on-sort

Conversation

@DerDreschner

Copy link
Copy Markdown
Contributor

Summary

This PR fixes a race condition that affects the playwright tests under high load on the CI runners. In case the race condition occurs, the tests fail with the following message:

Notice:   1 failed
    [chrome] › tests/playwright/e2e/files/files-sorting.spec.ts:138:2 › Files: Sorting the file list › Sorting works after switching view twice 
  43 passed (10.8m)

  1) [chrome] › tests/playwright/e2e/files/files-sorting.spec.ts:138:2 › Files: Sorting the file list › Sorting works after switching view twice 

    Error: expect(received).toEqual(expected) // deep equality

    - Expected  - 3
    + Received  + 3

      Array [
        "folder",
    -   "1 tiny.txt",
    -   "welcome.txt",
    -   "a medium.txt",
        "z big.txt",
    +   "a medium.txt",
    +   "welcome.txt",
    +   "1 tiny.txt",
      ]

    Call Log:
    - Timeout 15000ms exceeded while waiting on the predicate

      158 | 		await filesListPage.sortByColumn('Size')
      159 | 		await expect(filesListPage.getColumnHeader('Size')).toHaveAttribute('aria-sort', 'ascending')
    > 160 | 		await expect.poll(() => filesListPage.getRowNames()).toEqual([
          | 		                                                     ^
      161 | 			'folder',
      162 | 			'1 tiny.txt',
      163 | 			'welcome.txt',
        at /home/runner/actions-runner/_work/server/server/tests/playwright/e2e/files/files-sorting.spec.ts:160:56

    Error Context: test-results/files-files-sorting-Files--46f65--after-switching-view-twice-chrome/error-context.md

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

… state

Assisted-by: ClaudeCode:claude-fable-5
Signed-off-by: David Dreschner <david.dreschner@nextcloud.com>
@DerDreschner DerDreschner requested review from a team and skjnldsv as code owners July 13, 2026 01:11
@DerDreschner DerDreschner added bug 3. to review Waiting for reviews labels Jul 13, 2026
@DerDreschner DerDreschner requested review from nfebe and removed request for a team July 13, 2026 01:11
@DerDreschner DerDreschner requested a review from sorbaugh July 13, 2026 01:11
@DerDreschner

Copy link
Copy Markdown
Contributor Author

/backport to stable34

@DerDreschner

Copy link
Copy Markdown
Contributor Author

/backport to stable33

@DerDreschner

Copy link
Copy Markdown
Contributor Author

/backport to stable32

@DerDreschner

Copy link
Copy Markdown
Contributor Author

/compile

Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@DerDreschner DerDreschner added the tests Related to tests label Jul 13, 2026
Comment on lines +52 to +58
// Update the local store synchronously first, in call order, so the UI
// reflects the change immediately. Emitting only after the awaited server
// round-trip means rapid updates (e.g. flipping the sort direction several
// times in a row) issue concurrent PUTs that may resolve out of order,
// leaving the local state on whichever request finished last rather than
// the user's most recent action.
emit('files:view-config:updated', { view, key, value })

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This causes invalid state if the PUT fails, that why it is emitted after the PUT.

@susnux susnux left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you pre-emit the event before the actual update you must handle the error case where the update failed and revert the config update.

@susnux

susnux commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

This PR fixes a race condition that affects the playwright tests under high load on the CI runners.

If its only about the tests then make them wait for the PUT before continue. Should be an easy wait for response call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants