feat(storage): add progress stall timeout for S3 uploads#3306
Open
burak-initialcode wants to merge 2 commits intoaws-amplify:mainfrom
Open
feat(storage): add progress stall timeout for S3 uploads#3306burak-initialcode wants to merge 2 commits intoaws-amplify:mainfrom
burak-initialcode wants to merge 2 commits intoaws-amplify:mainfrom
Conversation
Add a configurable progress stall timeout to S3 storage uploads. When upload progress does not advance for the specified interval (e.g. due to network issues), the upload is automatically cancelled and the operation's onError consumer receives a typed StorageException whose cause is ProgressStallTimeoutException, instead of hanging indefinitely. ProgressStallTimeout (sealed: Disabled / Interval(seconds)) is exposed on AWSS3StoragePluginConfiguration as the plugin-wide default and can be overridden per upload via AWSS3StorageUploadFileOptions and AWSS3StorageUploadInputStreamOptions. The resolved interval is threaded through the upload pipeline into WorkData so SinglePartUploadWorker and PartUploadTransferWorker can decorate their ProgressListeners with a StallDetectingProgressListener; on stall the worker cancels the upload job, marks the transfer FAILED (no retry), and surfaces the typed cause. The default remains Disabled (opt-in), preserving existing behavior. fixes aws-amplify#3302
Cover the happy path for both single-part and multipart S3 uploads when a ProgressStallTimeout is set on the upload options. The stall timer must not break a successful upload; these tests assert that the option flows end-to-end through the worker pipeline without affecting normal completion. Mirrors the unit-tested integration coverage retained in the iOS PR.
96a423c to
c2ae0e3
Compare
Author
|
@harsh62 any progress on this one? |
Contributor
|
@burak-initialcode I am currently looking into the PR. Will let you know once I have an update. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
#3302
Description
Adds a configurable progress stall timeout to S3 storage uploads. When upload progress does not advance for the configured interval (e.g. due to network issues), the upload is automatically cancelled and the operation's
onErrorconsumer receives an error instead of hanging indefinitely.Problem: On poor or unstable networks, S3 uploads can stall indefinitely without the client receiving any progress updates.
Solution:
ProgressStallTimeoutsealed class in:core(Disabled/Interval(seconds)), mirroring the iOS API.ProgressStallTimeoutException(AmplifyExceptionsubclass) in:coreso consumers can pattern-match on thecauseof the surfacedStorageException.AWSS3StoragePluginConfiguration.progressStallTimeoutand can be overridden per upload viaAWSS3StorageUploadFileOptions/AWSS3StorageUploadInputStreamOptions(nulldefers to the plugin default;Disabledexplicitly opts out for that operation).WorkData.SinglePartUploadWorkerandPartUploadTransferWorkerdecorate theirProgressListeners with a coroutine-basedStallDetectingProgressListener. On stall, the worker cancels the upload job, marks the transferFAILED(workers explicitly skip retry forProgressStallTimeoutException), and surfaces the typed cause viaThrowable.toStorageUploadException(...).Disabled(opt-in). Existing behavior is preserved.Configuration:
How did you test these changes?
StallDetectingProgressListenerTest— coroutine-based timer logic (no progress, progress resets timer, close cancels, disabled threshold no-op, etc.) usingkotlinx-coroutines-test(StandardTestDispatcher+advanceTimeBy).StorageExceptionExtensionsTest— verifiestoStorageUploadExceptionpreserves the typedProgressStallTimeoutExceptioncause (direct and nested) and falls back to the default message for unrelated throwables.AWSS3StoragePluginConfigurationTest— defaults, builder, and override semantics forprogressStallTimeout.AWSS3StorageUploadFileOptionsTest/AWSS3StorageUploadInputStreamOptionsTest— option propagation,null-defers-to-plugin semantics, builder + Java interop.StorageService.uploadFile/uploadInputStreamoverloads.AWSS3StoragePathUploadTest(mirrors the iOS PR coverage retained after reviewer feedback):testUploadSmallFileWithProgressStallTimeoutOptionCompletesSuccessfully— single-part happy path with the stall timer set.testUploadLargeFileWithProgressStallTimeoutOptionCompletesSuccessfully— multipart happy path with the stall timer set../gradlew :core:test :aws-storage-s3:test— all passing locally../gradlew ktlintCheck checkstyle apiDump— all passing; API dump shows additive-only changes (minor bump).Documentation update required?
General Checklist
fix(storage): message,feat(auth): message,chore(all): message)By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.