Skip to content

Commit a98dca6

Browse files
burak-pensaharsh62thisisabhash
authored
feat(storage): add progress stall timeout for S3 uploads (#4162)
* feat(storage): add progress stall timeout for S3 uploads - Add progressStallTimeoutInterval to StorageConfiguration and AWSS3StoragePluginConfiguration - Single upload: stall timer in StorageServiceSessionDelegate, resets on didSendBodyData - Multipart upload: stall timer in StorageMultipartUploadSession - New error: AWSS3TransferUtilityErrorDomain, code 10 (makeProgressStallTimeoutError) - Dispatch .failed events on main thread for faster UI feedback - Unit tests: StorageErrorConstants, StorageConfiguration, PluginConfiguration, StorageServiceSessionDelegate, StorageMultipartUploadSession - Default: 0 (disabled), opt-in via configuration Made-with: Cursor * docs: add progress stall timeout documentation - Storage plugin README with configuration and error handling - Enhanced doc comments for AWSS3StoragePluginConfiguration - StorageErrorConstants documentation for stall timeout error - CHANGELOG entry - .gitignore for local changelog Made-with: Cursor * refactor(storage): ProgressStallTimeout type and per-operation override Introduce ProgressStallTimeout (.disabled, .interval) to replace raw TimeInterval naming. Plugin configuration uses progressStallTimeout; upload options can override per operation. Update tests, README, and CHANGELOG. Made-with: Cursor * test(storage): serialize Amplify reset/configure in operation unit tests Route reset and configure through a shared actor so concurrent test setUp/tearDown cannot leave Hub in pendingConfiguration when operations use Amplify.Hub. Update UploadFileOperationTests2 for async setUp. * fix(storage): address stall timeout review (errors, tests, Hub lifecycle) - Use StorageError.unknown for stall timeouts; remove NSError helper constants - Resolve stall interval on StorageTransferTask (-1 defers to StorageConfiguration) - Stall path: fail, unregister, then cancel; reset timer on multipart part progress - Restore upload operation failure dispatch threading; rename tests per review - Add integration tests for progressStallTimeout on small and multipart uploads - Fix OperationTestBase vs XCTest async ordering (Hub pendingConfiguration) - Reset Amplify in BaseConfigTests setUp when add() requires clean framework Made-with: Cursor * test(storage): rename progress stall timeout config test to camelCase Made-with: Cursor * Revert Storage README and .gitignore per review Made-with: Cursor * chore: revert Unreleased CHANGELOG entry; rename multipart stall test Made-with: Cursor * test(storage): add integration test for multipart stall timeout failure Made-with: Cursor * test(storage): stabilize stall timeout test on slow simulators Increase the stall interval and fulfillment timeout so the GCD timer has enough slack to fire reliably on watchOS/tvOS/iOS simulators in CI. Made-with: Cursor * refactor(storage): remove redundant StorageConfiguration init Drop the single-arg init(forBucket:) overload; the init(forBucket:progressStallTimeout:) variant already provides the same behavior via the default progressStallTimeout value. Made-with: Cursor * test(storage): rename stall tests to camelCase and stabilize mock session - Rename progress stall unit and integration tests from snake_case to camelCase to match review feedback and the rest of the suite. - Retain ARC ownership of the mock service/delegate in the async stall test so the delegate's weak reference to the service survives until the stall timer fires on slow simulators. - Configure the storage service mock with URLSessionConfiguration.default so the underlying URLSession initializes cleanly on iOS/watchOS/tvOS simulators that lack the application-identifier entitlement required for background sessions. Made-with: Cursor * refactor(storage): use optional TimeInterval for stall timeout sentinel Replace the -1 sentinel in StorageTransferTask.progressStallTimeoutSeconds with an optional TimeInterval. `nil` now signals "resolve from StorageConfiguration" (used for tasks restored from persistence), and resolvedProgressStallTimeoutSeconds falls back to the configuration value via nil-coalescing. Made-with: Cursor * style(storage): apply swiftformat to StorageMultipartUploadSession Drop redundant self. references inside the stall-timer closure to satisfy SwiftFormat 0.60.1 rules used by CI. Made-with: Cursor * test(storage): remove flaky multipart stall timeout failure test The sub-second interval used to force a stall during multipart uploads is too timing-sensitive for CI. Per reviewer feedback, drop the test and its associated error-chain helper; the unit-level coverage in StorageServiceSessionDelegateTests is sufficient to exercise the stall path without relying on network timing. Made-with: Cursor --------- Co-authored-by: Harsh <6162866+harsh62@users.noreply.github.com> Co-authored-by: Abhash Kumar Singh <thisisabhash@gmail.com>
1 parent 4a75795 commit a98dca6

26 files changed

Lines changed: 509 additions & 41 deletions

‎Amplify/Categories/Storage/Operation/Request/StorageUploadDataRequest.swift‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,50 +92,59 @@ public extension StorageUploadDataRequest {
9292
/// - Tag: StorageUploadDataRequestOptions.pluginOptions
9393
public let pluginOptions: Any?
9494

95+
/// Override plugin default progress stall timeout for this upload only. `nil` uses the plugin default.
96+
public let progressStallTimeout: ProgressStallTimeout?
97+
9598
/// - Tag: StorageUploadDataRequestOptions.init
9699
@available(*, deprecated, message: "Use init(metadata:contentType:options)")
97100
public init(
98101
accessLevel: StorageAccessLevel = .guest,
99102
targetIdentityId: String? = nil,
100103
metadata: [String: String]? = nil,
101104
contentType: String? = nil,
102-
pluginOptions: Any? = nil
105+
pluginOptions: Any? = nil,
106+
progressStallTimeout: ProgressStallTimeout? = nil
103107
) {
104108
self.accessLevel = accessLevel
105109
self.targetIdentityId = targetIdentityId
106110
self.metadata = metadata
107111
self.bucket = nil
108112
self.contentType = contentType
109113
self.pluginOptions = pluginOptions
114+
self.progressStallTimeout = progressStallTimeout
110115
}
111116

112117
/// - Tag: StorageUploadDataRequestOptions.init
113118
public init(
114119
metadata: [String: String]? = nil,
115120
contentType: String? = nil,
116-
pluginOptions: Any? = nil
121+
pluginOptions: Any? = nil,
122+
progressStallTimeout: ProgressStallTimeout? = nil
117123
) {
118124
self.accessLevel = .guest
119125
self.targetIdentityId = nil
120126
self.metadata = metadata
121127
self.bucket = nil
122128
self.contentType = contentType
123129
self.pluginOptions = pluginOptions
130+
self.progressStallTimeout = progressStallTimeout
124131
}
125132

126133
/// - Tag: StorageUploadDataRequestOptions.init
127134
public init(
128135
metadata: [String: String]? = nil,
129136
bucket: some StorageBucket,
130137
contentType: String? = nil,
131-
pluginOptions: Any? = nil
138+
pluginOptions: Any? = nil,
139+
progressStallTimeout: ProgressStallTimeout? = nil
132140
) {
133141
self.accessLevel = .guest
134142
self.targetIdentityId = nil
135143
self.metadata = metadata
136144
self.bucket = bucket
137145
self.contentType = contentType
138146
self.pluginOptions = pluginOptions
147+
self.progressStallTimeout = progressStallTimeout
139148
}
140149
}
141150
}

‎Amplify/Categories/Storage/Operation/Request/StorageUploadFileRequest.swift‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,50 +89,59 @@ public extension StorageUploadFileRequest {
8989
/// - Tag: StorageUploadFileRequestOptions.pluginOptions
9090
public let pluginOptions: Any?
9191

92+
/// Override the storage plugin's default progress stall timeout for this upload only. `nil` uses the plugin default.
93+
public let progressStallTimeout: ProgressStallTimeout?
94+
9295
/// - Tag: StorageUploadFileRequestOptions.init
9396
@available(*, deprecated, message: "Use init(metadata:contentType:pluginOptions)")
9497
public init(
9598
accessLevel: StorageAccessLevel = .guest,
9699
targetIdentityId: String? = nil,
97100
metadata: [String: String]? = nil,
98101
contentType: String? = nil,
99-
pluginOptions: Any? = nil
102+
pluginOptions: Any? = nil,
103+
progressStallTimeout: ProgressStallTimeout? = nil
100104
) {
101105
self.accessLevel = accessLevel
102106
self.targetIdentityId = targetIdentityId
103107
self.metadata = metadata
104108
self.bucket = nil
105109
self.contentType = contentType
106110
self.pluginOptions = pluginOptions
111+
self.progressStallTimeout = progressStallTimeout
107112
}
108113

109114
/// - Tag: StorageUploadFileRequestOptions.init
110115
public init(
111116
metadata: [String: String]? = nil,
112117
contentType: String? = nil,
113-
pluginOptions: Any? = nil
118+
pluginOptions: Any? = nil,
119+
progressStallTimeout: ProgressStallTimeout? = nil
114120
) {
115121
self.accessLevel = .guest
116122
self.targetIdentityId = nil
117123
self.metadata = metadata
118124
self.bucket = nil
119125
self.contentType = contentType
120126
self.pluginOptions = pluginOptions
127+
self.progressStallTimeout = progressStallTimeout
121128
}
122129

123130
/// - Tag: StorageUploadFileRequestOptions.init
124131
public init(
125132
metadata: [String: String]? = nil,
126133
bucket: some StorageBucket,
127134
contentType: String? = nil,
128-
pluginOptions: Any? = nil
135+
pluginOptions: Any? = nil,
136+
progressStallTimeout: ProgressStallTimeout? = nil
129137
) {
130138
self.accessLevel = .guest
131139
self.targetIdentityId = nil
132140
self.metadata = metadata
133141
self.bucket = bucket
134142
self.contentType = contentType
135143
self.pluginOptions = pluginOptions
144+
self.progressStallTimeout = progressStallTimeout
136145
}
137146
}
138147
}

‎Amplify/Categories/Storage/ProgressStallTimeout.swift‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
//
2+
// Copyright Amazon.com Inc. or its affiliates.
3+
// All Rights Reserved.
4+
//
5+
// SPDX-License-Identifier: Apache-2.0
6+
//
7+
8+
import Foundation
9+
10+
/// Strategy for cancelling uploads when progress stops advancing.
11+
///
12+
/// Aligns with the pattern used by other Amplify clients (for example flush intervals in the Kinesis client).
13+
/// Configure a default on the S3 storage plugin and optionally override per upload using
14+
/// ``StorageUploadFileRequest/Options`` or ``StorageUploadDataRequest/Options``.
15+
///
16+
/// - Tag: ProgressStallTimeout
17+
public enum ProgressStallTimeout: Sendable, Equatable {
18+
/// Do not cancel uploads when progress stalls.
19+
/// Named `disabled` (not `none`) so it does not collide with `Optional.none` when used as `ProgressStallTimeout?`.
20+
case disabled
21+
/// Cancel the upload if progress does not advance within this interval (seconds).
22+
case interval(TimeInterval)
23+
}
24+
25+
public extension ProgressStallTimeout {
26+
/// Duration in seconds used by the stall timer, or `0` when disabled.
27+
var secondsForStallTimer: TimeInterval {
28+
switch self {
29+
case .disabled:
30+
return 0
31+
case .interval(let seconds):
32+
return max(0, seconds)
33+
}
34+
}
35+
}

‎AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+Configure.swift‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,16 @@ extension AWSS3StoragePlugin {
104104
authService: AWSAuthCredentialsProviderBehavior,
105105
bucketInfo: BucketInfo
106106
) throws -> AWSS3StorageServiceBehavior {
107+
let storageConfig = StorageConfiguration(
108+
forBucket: bucketInfo.bucketName,
109+
progressStallTimeout: storageConfiguration.progressStallTimeout
110+
)
107111
let storageService = try AWSS3StorageService(
108112
authService: authService,
109113
region: bucketInfo.region,
110114
bucket: bucketInfo.bucketName,
111-
httpClientEngineProxy: httpClientEngineProxy
115+
httpClientEngineProxy: httpClientEngineProxy,
116+
storageConfiguration: storageConfig
112117
)
113118
storageService.urlRequestDelegate = urlRequestDelegate
114119
return storageService

‎AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Configuration/AWSS3StoragePluginConfiguration.swift‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
// SPDX-License-Identifier: Apache-2.0
66
//
77

8+
import Amplify
89
import Foundation
910

1011
/// Plugin specific configuration
@@ -16,9 +17,23 @@ public struct AWSS3StoragePluginConfiguration {
1617
@available(*, deprecated)
1718
public let prefixResolver: AWSS3PluginPrefixResolver?
1819

20+
/// Default strategy for cancelling uploads when progress stops advancing.
21+
/// Override per upload with ``StorageUploadFileRequest/Options/progressStallTimeout`` or
22+
/// ``StorageUploadDataRequest/Options/progressStallTimeout``.
23+
public let progressStallTimeout: ProgressStallTimeout
24+
1925
/// - Tag: AWSS3StoragePluginConfiguration.init
20-
public init(prefixResolver: AWSS3PluginPrefixResolver? = nil) {
26+
/// - Parameters:
27+
/// - prefixResolver: Deprecated. Use `StoragePath` instead.
28+
/// - progressStallTimeout: Stall detection strategy. Default is ``ProgressStallTimeout/disabled``.
29+
public init(prefixResolver: AWSS3PluginPrefixResolver? = nil, progressStallTimeout: ProgressStallTimeout = .disabled) {
2130
self.prefixResolver = prefixResolver
31+
self.progressStallTimeout = progressStallTimeout
32+
}
33+
34+
/// Resolves stall timeout seconds for an upload: per-operation override when non-`nil`, otherwise plugin default.
35+
func resolvedStallTimeoutSeconds(operationOverride: ProgressStallTimeout?) -> TimeInterval {
36+
(operationOverride ?? progressStallTimeout).secondsForStallTimer
2237
}
2338

2439
/// - Tag: AWSS3StoragePluginConfiguration.prefixResolverFunc

‎AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageUploadDataOperation.swift‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,13 +111,17 @@ class AWSS3StorageUploadDataOperation: AmplifyInProcessReportingOperation<
111111
}
112112

113113
let accelerate = try AWSS3PluginOptions.accelerateValue(pluginOptions: request.options.pluginOptions)
114+
let stallSeconds = storageConfiguration.resolvedStallTimeoutSeconds(
115+
operationOverride: request.options.progressStallTimeout
116+
)
114117
if request.data.count > StorageUploadDataRequest.Options.multiPartUploadSizeThreshold {
115118
try storageService.multiPartUpload(
116119
serviceKey: serviceKey,
117120
uploadSource: .data(request.data),
118121
contentType: request.options.contentType,
119122
metadata: request.options.metadata,
120-
accelerate: accelerate
123+
accelerate: accelerate,
124+
progressStallTimeoutSeconds: stallSeconds
121125
) { [weak self] event in
122126
self?.onServiceEvent(event: event)
123127
}
@@ -127,7 +131,8 @@ class AWSS3StorageUploadDataOperation: AmplifyInProcessReportingOperation<
127131
uploadSource: .data(request.data),
128132
contentType: request.options.contentType,
129133
metadata: request.options.metadata,
130-
accelerate: accelerate
134+
accelerate: accelerate,
135+
progressStallTimeoutSeconds: stallSeconds
131136
) { [weak self] event in
132137
self?.onServiceEvent(event: event)
133138
}

‎AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageUploadFileOperation.swift‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,13 +135,17 @@ class AWSS3StorageUploadFileOperation: AmplifyInProcessReportingOperation<
135135
}
136136

137137
let accelerate = try AWSS3PluginOptions.accelerateValue(pluginOptions: request.options.pluginOptions)
138+
let stallSeconds = storageConfiguration.resolvedStallTimeoutSeconds(
139+
operationOverride: request.options.progressStallTimeout
140+
)
138141
if uploadSize > StorageUploadFileRequest.Options.multiPartUploadSizeThreshold {
139142
try storageService.multiPartUpload(
140143
serviceKey: serviceKey,
141144
uploadSource: .local(request.local),
142145
contentType: request.options.contentType,
143146
metadata: request.options.metadata,
144-
accelerate: accelerate
147+
accelerate: accelerate,
148+
progressStallTimeoutSeconds: stallSeconds
145149
) { [weak self] event in
146150
self?.onServiceEvent(event: event)
147151
}
@@ -151,7 +155,8 @@ class AWSS3StorageUploadFileOperation: AmplifyInProcessReportingOperation<
151155
uploadSource: .local(request.local),
152156
contentType: request.options.contentType,
153157
metadata: request.options.metadata,
154-
accelerate: accelerate
158+
accelerate: accelerate,
159+
progressStallTimeoutSeconds: stallSeconds
155160
) { [weak self] event in
156161
self?.onServiceEvent(event: event)
157162
}

‎AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Service/Storage/AWSS3StorageService+MultiPartUploadBehavior.swift‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ extension AWSS3StorageService {
1616
contentType: String?,
1717
metadata: [String: String]?,
1818
accelerate: Bool?,
19+
progressStallTimeoutSeconds: TimeInterval,
1920
onEvent: @escaping StorageServiceMultiPartUploadEventHandler
2021
) {
2122
let fail: (Error) -> Void = { error in
@@ -44,7 +45,8 @@ extension AWSS3StorageService {
4445
key: serviceKey,
4546
contentType: contentType,
4647
requestHeaders: requestHeaders,
47-
onEvent: onEvent
48+
onEvent: onEvent,
49+
progressStallTimeoutSeconds: progressStallTimeoutSeconds
4850
)
4951

5052
register(multipartUploadSession: multipartUploadSession)

‎AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Service/Storage/AWSS3StorageService+UploadBehavior.swift‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ extension AWSS3StorageService {
1616
contentType: String?,
1717
metadata: [String: String]?,
1818
accelerate: Bool?,
19+
progressStallTimeoutSeconds: TimeInterval,
1920
onEvent: @escaping StorageServiceUploadEventHandler
2021
) {
2122
let fail: (Error) -> Void = { error in
@@ -29,7 +30,8 @@ extension AWSS3StorageService {
2930
let transferTask = createTransferTask(
3031
transferType: .upload(onEvent: onEvent),
3132
bucket: bucket,
32-
key: serviceKey
33+
key: serviceKey,
34+
progressStallTimeoutSeconds: progressStallTimeoutSeconds
3335
)
3436
let uploadFileURL: URL
3537
guard let uploadFile = try attempt(uploadSource.getFile(), fail: fail) else { return }
@@ -88,6 +90,8 @@ extension AWSS3StorageService {
8890
// register task so it can be accessed in URLSession delegate functions
8991
register(task: transferTask)
9092

93+
(urlSession.delegate as? StorageServiceSessionDelegate)?.startProgressStallTimerIfNeeded(taskIdentifier: uploadTask.taskIdentifier)
94+
9195
if startTransfer {
9296
transferTask.resume()
9397
}

‎AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Service/Storage/AWSS3StorageService.swift‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ class AWSS3StorageService: AWSS3StorageServiceBehavior, StorageServiceProxy {
179179

180180
func resetURLSession() {
181181
let delegate = StorageServiceSessionDelegate(identifier: storageConfiguration.sessionIdentifier, logger: logger)
182+
delegate.storageService = self
182183
urlSession = URLSession(configuration: sessionConfiguration, delegate: delegate, delegateQueue: delegateQueue)
183184
}
184185

@@ -205,6 +206,9 @@ class AWSS3StorageService: AWSS3StorageServiceBehavior, StorageServiceProxy {
205206
client: client,
206207
transferTask: pair.transferTask,
207208
multipartUpload: multipartUpload,
209+
progressStallTimeoutSeconds: pair.transferTask.resolvedProgressStallTimeoutSeconds(
210+
storageConfiguration: storageConfiguration
211+
),
208212
logger: logger
209213
) else {
210214
return
@@ -280,14 +284,16 @@ class AWSS3StorageService: AWSS3StorageServiceBehavior, StorageServiceProxy {
280284
bucket: String,
281285
key: String,
282286
location: URL? = nil,
283-
requestHeaders: [String: String]? = nil
287+
requestHeaders: [String: String]? = nil,
288+
progressStallTimeoutSeconds: TimeInterval = 0
284289
) -> StorageTransferTask {
285290
let transferTask = StorageTransferTask(
286291
transferType: transferType,
287292
bucket: bucket,
288293
key: key,
289294
location: location,
290295
requestHeaders: requestHeaders,
296+
progressStallTimeoutSeconds: progressStallTimeoutSeconds,
291297
storageTransferDatabase: storageTransferDatabase,
292298
logger: logger
293299
)

0 commit comments

Comments
 (0)