Skip to content

Commit a6d1331

Browse files
sichanyooSichan Yoojbelkins
authored
fix: Delete early exit check for unsigned chunk (#796)
* Delete early exit check for unsigned chunk * Fix chunk handling in CRT client engine. It used to spam 0-byte chunks until S3 returned request timeout error (since 0 byte chunk still means 0 byte received by server). * swiftlint --------- Co-authored-by: Sichan Yoo <[email protected]> Co-authored-by: Josh Elkins <[email protected]>
1 parent 8319894 commit a6d1331

File tree

2 files changed

+23
-24
lines changed

2 files changed

+23
-24
lines changed

Sources/ClientRuntime/Networking/Http/CRT/CRTClientEngine.swift

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ public class CRTClientEngine: HTTPClient {
223223
// TICK - smithy.client.http.connections.limit
224224
telemetry.httpMetricsUsage.connectionsLimit = serialExecutor.maxConnectionsPerEndpoint
225225

226-
let connectionMgrMetrics = try connectionMgr.fetchMetrics()
226+
let connectionMgrMetrics = connectionMgr.fetchMetrics()
227227

228228
// TICK - smithy.client.http.connections.usage
229229
telemetry.httpMetricsUsage.idleConnections = connectionMgrMetrics.availableConcurrency
@@ -282,11 +282,16 @@ public class CRTClientEngine: HTTPClient {
282282
}
283283

284284
var hasMoreChunks = true
285+
var currentChunkBodyIsEmpty = false
285286
while hasMoreChunks {
286287
// Process the first chunk and determine if there are more to send
287288
hasMoreChunks = try await chunkedStream.chunkedReader.processNextChunk()
289+
currentChunkBodyIsEmpty = chunkedStream
290+
.chunkedReader
291+
.getCurrentChunkBody()
292+
.isEmpty
288293

289-
if !hasMoreChunks {
294+
if !hasMoreChunks || currentChunkBodyIsEmpty {
290295
// Send the final chunk
291296
let finalChunk = try await chunkedStream.chunkedReader.getFinalChunk()
292297
// TICK - smithy.client.http.bytes_sent
@@ -299,23 +304,22 @@ public class CRTClientEngine: HTTPClient {
299304
value: finalChunk.count,
300305
attributes: bytesSentAttributes,
301306
context: telemetry.contextManager.current())
307+
hasMoreChunks = false
302308
} else {
303-
let currentChunkBody = chunkedStream.chunkedReader.getCurrentChunkBody()
304-
if !currentChunkBody.isEmpty {
305-
// TICK - smithy.client.http.bytes_sent
306-
try await http1Stream.writeChunk(
307-
chunk: chunkedStream.chunkedReader.getCurrentChunk(),
308-
endOfStream: false
309-
)
310-
var bytesSentAttributes = Attributes()
311-
bytesSentAttributes.set(
312-
key: HttpMetricsAttributesKeys.serverAddress,
313-
value: CRTClientEngine.makeServerAddress(request: request))
314-
telemetry.bytesSent.add(
315-
value: currentChunkBody.count,
316-
attributes: bytesSentAttributes,
317-
context: telemetry.contextManager.current())
318-
}
309+
let currentChunk = chunkedStream.chunkedReader.getCurrentChunk()
310+
// TICK - smithy.client.http.bytes_sent
311+
try await http1Stream.writeChunk(
312+
chunk: currentChunk,
313+
endOfStream: false
314+
)
315+
var bytesSentAttributes = Attributes()
316+
bytesSentAttributes.set(
317+
key: HttpMetricsAttributesKeys.serverAddress,
318+
value: CRTClientEngine.makeServerAddress(request: request))
319+
telemetry.bytesSent.add(
320+
value: currentChunk.count,
321+
attributes: bytesSentAttributes,
322+
context: telemetry.contextManager.current())
319323
}
320324
}
321325
} catch {
@@ -432,7 +436,7 @@ public class CRTClientEngine: HTTPClient {
432436
// TICK - smithy.client.http.connections.limit
433437
telemetry.httpMetricsUsage.connectionsLimit = serialExecutor.maxConnectionsPerEndpoint
434438

435-
let connectionMgrMetrics = try connectionMgr.fetchMetrics()
439+
let connectionMgrMetrics = connectionMgr.fetchMetrics()
436440

437441
// TICK - smithy.client.http.connections.usage
438442
telemetry.httpMetricsUsage.idleConnections = connectionMgrMetrics.availableConcurrency

Sources/SmithyChecksums/ChunkedReader.swift

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -114,11 +114,6 @@ public class ChunkedReader {
114114

115115
self.chunkBody = chunk
116116

117-
// Early exit for empty chunk when already signed
118-
if chunk.isEmpty {
119-
return nil
120-
}
121-
122117
return constructChunk(chunk: chunk, signature: nil)
123118
}
124119

0 commit comments

Comments
 (0)