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

@uppy/companion: fix double tus uploads #4816

Merged
merged 4 commits into from
Dec 12, 2023
Merged

@uppy/companion: fix double tus uploads #4816

merged 4 commits into from
Dec 12, 2023

Conversation

mifi
Copy link
Contributor

@mifi mifi commented Dec 8, 2023

fixes #4815
also improve error logging when canceling

also:

don't close socket when pausing:
because it prevented us from canceling an upload that is paused
note that we here sacrifice the ability to pause uploads and have other uploads take over the paused upload's rate limit queue spot.
this is sacrificed because we will soon remove the ability to pause/resume uploads anyways

fixes #4815
also improve error logging when canceling
because it prevented us from canceling an upload that is paused
note that we here sacrifice the ability to pause uploads and have other uploads take over the paused upload's rate limit queue spot.
this is sacrificed because we will soon remove the ability to pause/resume uploads anyways
@Murderlon Murderlon changed the title Fix double uploads @uppy/companion, @uppy/companion-client: fix double tus uploads and cancel all Dec 11, 2023
@arturi arturi self-requested a review December 11, 2023 13:52
@arturi
Copy link
Contributor

arturi commented Dec 11, 2023

We agreed to only fix the double tus uploads issue and address the socket cancel after pause issue in a separate PR.

@mifi
Copy link
Contributor Author

mifi commented Dec 12, 2023

have now reversed commit e4dbe77 - I believe this can be merged

created a new PR for that commit:

@mifi mifi changed the title @uppy/companion, @uppy/companion-client: fix double tus uploads and cancel all @uppy/companion, @uppy/companion-client: fix double tus uploads Dec 12, 2023
@Murderlon Murderlon changed the title @uppy/companion, @uppy/companion-client: fix double tus uploads @uppy/companion: fix double tus uploads Dec 12, 2023
@arturi
Copy link
Contributor

arturi commented Dec 12, 2023

Companion now immediately receives a resume event when the download begins. With slow connection the upload (downloads) sits at 0% for quite some time (I'm pretty sure unrelated to this PR). Otherwise the upload (via Google Drive and tus) works.

@arturi arturi merged commit df36e12 into main Dec 12, 2023
22 checks passed
@arturi arturi deleted the fix-double-uploads branch December 12, 2023 19:20
github-actions bot added a commit that referenced this pull request Dec 12, 2023
| Package                | Version | Package                | Version |
| ---------------------- | ------- | ---------------------- | ------- |
| @uppy/aws-s3           |   3.6.0 | @uppy/instagram        |   3.2.0 |
| @uppy/aws-s3-multipart |  3.10.0 | @uppy/onedrive         |   3.2.0 |
| @uppy/box              |   2.2.0 | @uppy/provider-views   |   3.8.0 |
| @uppy/companion        |  4.12.0 | @uppy/store-default    |   3.2.0 |
| @uppy/companion-client |   3.7.0 | @uppy/tus              |   3.5.0 |
| @uppy/core             |   3.8.0 | @uppy/url              |   3.5.0 |
| @uppy/dropbox          |   3.2.0 | @uppy/utils            |   5.7.0 |
| @uppy/facebook         |   3.2.0 | @uppy/xhr-upload       |   3.6.0 |
| @uppy/google-drive     |   3.4.0 | @uppy/zoom             |   2.2.0 |
| @uppy/image-editor     |   2.4.0 | uppy                   |  3.21.0 |

- @uppy/provider-views: fix uploadRemoteFile undefined (Mikael Finstad / #4814)
- @uppy/companion: fix double tus uploads (Mikael Finstad / #4816)
- @uppy/companion: fix accelerated endpoints for presigned POST  (Mikael Finstad / #4817)
- @uppy/companion: fix `authProvider` property inconsistency (Mikael Finstad / #4672)
- @uppy/companion:  send certain onedrive errors to the user (Mikael Finstad / #4671)
- meta: fix typo in `lockfile_check.yml` name (Antoine du Hamel)
- @uppy/aws-s3: change Companion URL in tests (Antoine du Hamel)
- @uppy/set-state: fix types (Antoine du Hamel)
- @uppy/companion: Provider user sessions (Mikael Finstad / #4619)
- meta: fix `js2ts` script on Node.js 20+ (Merlijn Vos / #4802)
- @uppy/companion-client: avoid unnecessary preflight requests (Antoine du Hamel / #4462)
- meta: Migrate to AWS-SDK V3 syntax (Artur Paikin / #4810)
- @uppy/utils: fix import in test files (Antoine du Hamel / #4806)
- @uppy/core: Fix onBeforeFileAdded with Golden Retriever (Merlijn Vos / #4799)
- @uppy/image-editor: respect `cropperOptions.initialAspectRatio` (Lucklj521 / #4805)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants