@uppy/aws-s3-multipart: fix Golden Retriever integration #4526

aduh95 · 2023-06-23T15:28:00Z

Murderlon

Very nice to see this finally fixed ✨ Such and old and often requested fix.

Multipart has become the most complex plugin. It has very little docs. Would be nice to at least document all the private property initializers at the top. It's not clear what these do nor what their types look like: chunks, chunkState, data (very generic), file, uploadPromise.

Murderlon · 2023-06-26T09:52:22Z

packages/@uppy/aws-s3-multipart/src/MultipartUploader.js

@@ -64,6 +66,7 @@ class MultipartUploader {
    this.#onSuccess = this.options.onSuccess
    this.#onError = this.options.onError
    this.#shouldUseMultipart = this.options.shouldUseMultipart
+    this.#isRestoring = 'uploadId' in options


options is passed by the user, why are we uploadId from there to know if we are restoring?

It's not passed by the user, it's passed by index.js

uppy/packages/@uppy/aws-s3-multipart/src/index.js

Lines 611 to 627 in b6a6f83

const upload = new MultipartUploader(file.data, {

// .bind to pass the file object to each handler.

companionComm: this.#companionCommunicationQueue,

log: (...args) => this.uppy.log(...args),

getChunkSize: this.opts.getChunkSize ? this.opts.getChunkSize.bind(this) : null,

onProgress,

onError,

onSuccess,

onPartComplete,

file,

shouldUseMultipart: this.opts.shouldUseMultipart,

...file.s3Multipart,

})

Can we make it the relationship between uploadId determining whether we are restoring more clear somehow?

packages/@uppy/aws-s3-multipart/src/MultipartUploader.js

Murderlon · 2023-06-26T09:54:59Z

packages/@uppy/aws-s3-multipart/src/MultipartUploader.js

@@ -105,6 +108,12 @@ class MultipartUploader {
          onComplete: this.#onPartComplete(j),
          shouldUseMultipart,
        }
+        if (this.#isRestoring) {
+          const size = i + chunkSize > fileSize ? fileSize - i : chunkSize
+          this.#chunks[j].setAsUploaded = () => {


why a function and not setting uploaded directly, since we use size from this scope anyway?

We don't know yet if it was already uploaded

could use a comment how this is used later

packages/@uppy/aws-s3-multipart/src/MultipartUploader.js

* main: @uppy/aws-s3-multipart: fix Golden Retriever integration (#4526) examples/aws-nodejs: merge multipart and non-multipart examples (#4521) @uppy/companion: bump semver from 7.3.7 to 7.5.3 (#4529) @uppy/aws-s3-multipart: add types to internal fields (#4535) examples/aws-nodejs: update README (#4534) examples/aws-nodejs: showcase an example without preflight requests (#4516) @uppy/aws-s3-multipart: fix pause/resume (#4523) @uppy/status-bar: fix ETA when Uppy recovers its state (#4525) @uppy/aws-s3-multipart: fix resume single-chunk multipart uploads (#4528) @uppy/companion: fix part listing in s3 (#4524) example/aws-php: make it forward-compatible with the next Uppy major (#4522) @uppy/golden-retriever: refactor to modernize the codebase (#4520) examples/aws-nodejs: upgrade to AWS-SDK v3 (#4515) @uppy/companion: implement refresh for authentication tokens (#4448) @uppy/aws-s3-multipart: disable pause/resume for remote uploads in the UI (#4500) @uppy/tus: retry on 423 HTTP error code (#4512)

| Package | Version | Package | Version | | ---------------------- | ------- | ---------------------- | ------- | | @uppy/aws-s3 | 3.2.1 | @uppy/golden-retriever | 3.1.0 | | @uppy/aws-s3-multipart | 3.4.1 | @uppy/status-bar | 3.2.1 | | @uppy/companion | 4.6.0 | @uppy/tus | 3.1.2 | | @uppy/companion-client | 3.2.0 | @uppy/xhr-upload | 3.3.1 | | @uppy/core | 3.3.0 | uppy | 3.11.0 | - @uppy/companion: fix infinite recursion in uploader test (Mikael Finstad / #4536) - @uppy/xhr-upload: export `Headers` type (Masum ULU / #4549) - @uppy/aws-s3-multipart: increase priority of abort and complete (Stefan Schonert / #4542) - @uppy/aws-s3: fix remote uploads (Antoine du Hamel / #4546) - meta: use `corepack yarn` instead of `npm` to launch E2E (Antoine du Hamel / #4545) - @uppy/aws-s3-multipart: fix upload retry using an outdated ID (Antoine du Hamel / #4544) - @uppy/status-bar: remove throttled component (Artur Paikin / #4396) - @uppy/aws-s3-multipart: fix Golden Retriever integration (Antoine du Hamel / #4526) - examples/aws-nodejs: merge multipart and non-multipart examples (Antoine du Hamel / #4521) - @uppy/companion: bump semver from 7.3.7 to 7.5.3 (dependabot[bot] / #4529) - @uppy/aws-s3-multipart: add types to internal fields (Antoine du Hamel / #4535) - examples/aws-nodejs: update README (Antoine du Hamel / #4534) - examples/aws-nodejs: showcase an example without preflight requests (Antoine du Hamel / #4516) - @uppy/aws-s3-multipart: fix pause/resume (Antoine du Hamel / #4523) - @uppy/status-bar: fix ETA when Uppy recovers its state (Antoine du Hamel / #4525) - @uppy/aws-s3-multipart: fix resume single-chunk multipart uploads (Antoine du Hamel / #4528) - @uppy/companion: fix part listing in s3 (Antoine du Hamel / #4524) - example/aws-php: make it forward-compatible with the next Uppy major (Antoine du Hamel / #4522) - @uppy/golden-retriever: refactor to modernize the codebase (Antoine du Hamel / #4520) - examples/aws-nodejs: upgrade to AWS-SDK v3 (Antoine du Hamel / #4515) - @uppy/companion: implement refresh for authentication tokens (Mikael Finstad / #4448) - @uppy/aws-s3-multipart: disable pause/resume for remote uploads in the UI (Artur Paikin / #4500) - @uppy/tus: retry on 423 HTTP error code (Antoine du Hamel / #4512)

* main: (24 commits) Add missing pt-BR locales for ImageEditor plugin (#4558) Release: uppy@3.11.0 (#4550) @uppy/companion: fix infinite recursion in uploader test (#4536) @uppy/xhr-upload: export `Headers` type (#4549) @uppy/aws-s3-multipart: increase priority of abort and complete (#4542) @uppy/aws-s3: fix remote uploads (#4546) meta: use `corepack yarn` instead of `npm` to launch E2E (#4545) @uppy/aws-s3-multipart: fix upload retry using an outdated ID (#4544) @uppy/status-bar: remove throttled component (#4396) @uppy/aws-s3-multipart: fix Golden Retriever integration (#4526) examples/aws-nodejs: merge multipart and non-multipart examples (#4521) @uppy/companion: bump semver from 7.3.7 to 7.5.3 (#4529) @uppy/aws-s3-multipart: add types to internal fields (#4535) examples/aws-nodejs: update README (#4534) examples/aws-nodejs: showcase an example without preflight requests (#4516) @uppy/aws-s3-multipart: fix pause/resume (#4523) @uppy/status-bar: fix ETA when Uppy recovers its state (#4525) @uppy/aws-s3-multipart: fix resume single-chunk multipart uploads (#4528) @uppy/companion: fix part listing in s3 (#4524) example/aws-php: make it forward-compatible with the next Uppy major (#4522) ...

@uppy/aws-s3-multipart: fix Golden Retriever integration

91278fd

aduh95 requested a review from Murderlon June 23, 2023 15:28

github-actions bot added the pending end-to-end tests label Jun 23, 2023

Murderlon reviewed Jun 26, 2023

View reviewed changes

aduh95 added 3 commits June 28, 2023 16:34

s/i/offset/

effd4cd

Merge branch 'main' of https://github.com/transloadit/uppy

535b157

add comments and fix types

9940af8

Murderlon approved these changes Jun 29, 2023

View reviewed changes

aduh95 merged commit d1697b0 into transloadit:main Jun 29, 2023
15 checks passed

aduh95 deleted the aws-multipart-golden-retriever branch June 29, 2023 14:50

aduh95 removed the pending end-to-end tests label Jul 5, 2023

aduh95 mentioned this pull request Jul 5, 2023

@uppy/aws-s3-multipart: fix upload retry using an outdated ID #4544

Merged

github-actions bot mentioned this pull request Jul 6, 2023

Release: uppy@3.11.0 #4550

Merged

Murderlon mentioned this pull request Mar 11, 2024

Uppy S3 + Golden Retriever Service Worker #4985

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

@uppy/aws-s3-multipart: fix Golden Retriever integration #4526

@uppy/aws-s3-multipart: fix Golden Retriever integration #4526

aduh95 commented Jun 23, 2023

Murderlon left a comment •

edited

Murderlon Jun 26, 2023

aduh95 Jun 27, 2023

Murderlon Jun 28, 2023

Murderlon Jun 26, 2023

aduh95 Jun 26, 2023

Murderlon Jun 28, 2023

	const upload = new MultipartUploader(file.data, {
	// .bind to pass the file object to each handler.
	companionComm: this.#companionCommunicationQueue,

	log: (...args) => this.uppy.log(...args),
	getChunkSize: this.opts.getChunkSize ? this.opts.getChunkSize.bind(this) : null,

	onProgress,
	onError,
	onSuccess,
	onPartComplete,

	file,
	shouldUseMultipart: this.opts.shouldUseMultipart,

	...file.s3Multipart,
	})

@uppy/aws-s3-multipart: fix Golden Retriever integration #4526

@uppy/aws-s3-multipart: fix Golden Retriever integration #4526

Conversation

aduh95 commented Jun 23, 2023

Murderlon left a comment • edited

Choose a reason for hiding this comment

Murderlon Jun 26, 2023

Choose a reason for hiding this comment

aduh95 Jun 27, 2023

Choose a reason for hiding this comment

Murderlon Jun 28, 2023

Choose a reason for hiding this comment

Murderlon Jun 26, 2023

Choose a reason for hiding this comment

aduh95 Jun 26, 2023

Choose a reason for hiding this comment

Murderlon Jun 28, 2023

Choose a reason for hiding this comment

Murderlon left a comment •

edited