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

fix(uploader): only monitor the queue being idle when we kow we are finishing the upload #1521

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

skjnldsv
Copy link
Contributor

@skjnldsv skjnldsv commented Dec 10, 2024

Regression from #1175
Fix for nextcloud/server#49287

Symptoms

  • When uploading with the upload method (not batchUpload), the jobQueue gets idle before the chunks gets computed and between the chunk upload and the final assembly.
  • Unfortunately, resetting clear the entirety of the uploadQueue
  • This makes the UI looks like there is no pending upload despite more requests being added to the stack afterwards

This is a fix, but we could definitely improve here again, with the batchUpload method, things got messy.
But this would be technical debt, and the bug that this PR fixes is critical.

Explanations

By moving the idle listener to the end of the method, we are ensuring that the upload's job queue is fully populated before the reset can be triggered.

The original placement in the constructor meant the reset could happen before the upload was fully set up, causing the premature queue clearing we were experiencing. By moving it to the end of the upload method, we are guaranteeing that the queue is fully initialized before any potential reset can occur.
This is a great example of how event listener placement can critically impact the behavior of asynchronous operations

…inishing the upload

Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv skjnldsv added bug Something isn't working regression Regression of a previous working feature 3. to review Waiting for reviews labels Dec 10, 2024
@skjnldsv skjnldsv requested review from artonge and Pytal December 10, 2024 17:07
@skjnldsv skjnldsv self-assigned this Dec 10, 2024
Copy link

codecov bot commented Dec 10, 2024

Bundle Report

Changes will increase total bundle size by 384 bytes (0.05%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
@nextcloud/upload-esm 390.18kB 192 bytes (0.05%) ⬆️
@nextcloud/upload-cjs 393.54kB 192 bytes (0.05%) ⬆️

@skjnldsv skjnldsv merged commit 36fafd8 into main Dec 10, 2024
18 checks passed
@skjnldsv skjnldsv deleted the fix/queue-clear branch December 10, 2024 17:12
This was referenced Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working regression Regression of a previous working feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants