Skip to content

Conversation

matthieusieben
Copy link

@matthieusieben matthieusieben commented Sep 8, 2025

Issue

#6426

Description

This change improves the following aspects of the lib:

  • Options use an abortSignal instead of an abortController (still available but deprecated) in order to be more consistent with other APIs. This also allows to use Upload with AbortSignal.timeout()
      await new Upload({
        client,
        params,
        abortSignal: AbortSignal.timeout(10_000)
      }).done()
  • The code no longer returns control to the caller if operations are still pending in the background. Before, a Promise.race would cause done() to reject as soon as the abort controller is aborted, but would not wait for pending tasks to complete. This prevents user code from properly controlling concurrency. This change also avoids Upload from seemingly failing if they are aborted during the PutObjectTaggingCommand phase, and the upload acutally succeeded (since nothing actually cancelled it).
  • Uploads of individual chunks will be cancelled as soon as the upload is aborted, instead of waiting for the chunk upload to complete before cancelling the whole process. This allows to ensure that control is given back to the user code asap.
  • The type of the abortController option was updated to not cause a typing issue when used with global AbortController (no longer need to @ts-ignore).

Testing

Not tested.

Additional context

We encountered upload "hangs" in our code, similar to those described in #6426 . While investigating, I encountered several opportunities for improvement, as noted in the Description above.

Checklist

  • If the PR is a feature, add integration tests (*.integ.spec.ts).
  • If you wrote E2E tests, are they resilient to concurrent I/O?
  • If adding new public functions, did you add the @public tag and enable doc generation on the package?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@matthieusieben matthieusieben requested a review from a team as a code owner September 8, 2025 08:37
@matthieusieben matthieusieben force-pushed the lib-storage-abort-controller-fix branch 3 times, most recently from 5228307 to 3c15036 Compare September 8, 2025 09:09
@matthieusieben matthieusieben force-pushed the lib-storage-abort-controller-fix branch from 3c15036 to 68b910d Compare September 8, 2025 09:10
martinslota added a commit to martinslota/aws-sdk-lib-storage-race that referenced this pull request Sep 8, 2025
);
}
this.sent = true;
return await Promise.race([this.__doMultipartUpload(), this.__abortTimeout(this.abortController.signal)]);
Copy link
Author

Choose a reason for hiding this comment

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

The race here could cause the returned promise to be rejected when __doMultipartUpload actually succeeds (and is still working on updating tags).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant