Add special handling for connection issue retries.

amzn · Nov 28, 2023 · b3254ab · b3254ab
1 parent 2f27d29
commit b3254ab
Show file tree

Hide file tree

Showing 6 changed files with 70 additions and 9 deletions.
diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml
@@ -10,6 +10,7 @@ jobs:
   run-codeql-linux:
     name: Run CodeQL on Linux
     runs-on: ubuntu-latest
+    container: swift:5.8
     permissions:
       security-events: write
 

diff --git a/.github/workflows/swift.yml b/.github/workflows/swift.yml
@@ -12,10 +12,10 @@ jobs:
     strategy:
       matrix:
         os: [ubuntu-22.04, ubuntu-20.04]
-        swift: ["5.8"]
+        swift: ["5.9"]
     runs-on: ${{ matrix.os }}
     steps:
-      - uses: swift-actions/setup-swift@v1.23.0
+      - uses: swift-actions/setup-swift@v1.25.0
         with:
           swift-version: ${{ matrix.swift }}
       - uses: actions/checkout@v2
@@ -28,10 +28,10 @@ jobs:
     strategy:
       matrix:
         os: [ubuntu-20.04]
-        swift: ["5.7.3", "5.6.3"]
+        swift: ["5.8.1", "5.7.3"]
     runs-on: ${{ matrix.os }}
     steps:
-      - uses: swift-actions/setup-swift@v1.23.0
+      - uses: swift-actions/setup-swift@v1.25.0
         with:
           swift-version: ${{ matrix.swift }}
       - uses: actions/checkout@v2

diff --git a/Package.swift b/Package.swift
@@ -42,6 +42,7 @@ let package = Package(
     ],
     dependencies: [
         .package(url: "https://github.com/apple/swift-nio.git", from: "2.33.0"),
+        .package(url: "https://github.com/apple/swift-nio-http2.git", from: "1.19.0"),
         .package(url: "https://github.com/apple/swift-nio-ssl.git", from: "2.14.0"),
         .package(url: "https://github.com/apple/swift-log.git", from: "1.0.0"),
         .package(url: "https://github.com/apple/swift-metrics.git", "1.0.0"..<"3.0.0"),
@@ -55,6 +56,7 @@ let package = Package(
                 .product(name: "Metrics", package: "swift-metrics"),
                 .product(name: "NIO", package: "swift-nio"),
                 .product(name: "NIOHTTP1", package: "swift-nio"),
+                .product(name: "NIOHTTP2", package: "swift-nio-http2"),
                 .product(name: "NIOFoundationCompat", package: "swift-nio"),
                 .product(name: "NIOSSL", package: "swift-nio-ssl"),
                 .product(name: "AsyncHTTPClient", package: "async-http-client"),

diff --git a/README.md b/README.md
@@ -3,7 +3,7 @@
 <img src="https://github.com/amzn/smoke-http/actions/workflows/swift.yml/badge.svg?branch=main" alt="Build - Main Branch">
 </a>
 <a href="http://swift.org">
-<img src="https://img.shields.io/badge/swift-5.6|5.7|5.8-orange.svg?style=flat" alt="Swift 5.6, 5.7 and 5.8 Tested">
+<img src="https://img.shields.io/badge/swift-5.7|5.8|5.9-orange.svg?style=flat" alt="Swift 5.7, 5.8 and 5.9 Tested">
 </a>
 <img src="https://img.shields.io/badge/ubuntu-18.04|20.04-yellow.svg?style=flat" alt="Ubuntu 18.04 and 20.04 Tested">
 <img src="https://img.shields.io/badge/CentOS-8-yellow.svg?style=flat" alt="CentOS 8 Tested">

diff --git a/Sources/SmokeHTTPClient/HTTPOperationsClient+executeRetriableWithOutput.swift b/Sources/SmokeHTTPClient/HTTPOperationsClient+executeRetriableWithOutput.swift
@@ -18,8 +18,10 @@
 #if (os(Linux) && compiler(>=5.5)) || (!os(Linux) && compiler(>=5.5.2)) && canImport(_Concurrency)
 
 import Foundation
+import AsyncHTTPClient
 import NIO
 import NIOHTTP1
+import NIOHTTP2
 import Metrics
 import Tracing
 
@@ -69,6 +71,11 @@ public extension HTTPOperationsClient {
 
         var retriesRemaining: Int
 
+        // For requests that fail for transient connection issues (StreamClosed, remoteConnectionClosed)
+        // don't consume retry attempts and don't use expotential backoff
+        var abortedAttemptsRemaining: Int = 5
+        let waitOnAbortedAttemptMs = 2
+
         init(endpointOverride: URL?, requestComponents: HTTPRequestComponents, httpMethod: HTTPMethod,
              invocationContext: HTTPClientInvocationContext<InvocationReportingType, HandlerDelegateType>,
              eventLoopOverride eventLoop: EventLoop,
@@ -143,8 +150,19 @@ public extension HTTPOperationsClient {
 
             let shouldRetryOnError = retryOnError(error)
 
-            // if there are retries remaining and we should retry on this error
-            if retriesRemaining > 0 && shouldRetryOnError {
+            // For requests that fail for transient connection issues (StreamClosed, remoteConnectionClosed)
+            // don't consume retry attempts and don't use expotential backoff
+            if self.abortedAttemptsRemaining > 0 && treatAsAbortedAttempt(cause: error.cause) {
+                logger.debug(
+                    "Request aborted with error: \(error). Retrying in \(self.waitOnAbortedAttemptMs) ms.")
+
+                self.abortedAttemptsRemaining -= 1
+
+                try await Task.sleep(nanoseconds: UInt64(self.waitOnAbortedAttemptMs) * millisecondsToNanoSeconds)
+
+                return try await self.executeWithOutput()
+                // if there are retries remaining (and haven't exhausted aborted attempts) and we should retry on this error
+            } else if self.abortedAttemptsRemaining > 0 && self.retriesRemaining > 0 && shouldRetryOnError {
                 // determine the required interval
                 let retryInterval = Int(retryConfiguration.getRetryInterval(retriesRemaining: retriesRemaining))
 
@@ -202,6 +220,16 @@ public extension HTTPOperationsClient {
                     retriableOutwardsRequest: StandardRetriableOutputRequestRecord(outputRequests: outputRequestRecords))
             }
         }
+
+        func treatAsAbortedAttempt(cause: Swift.Error) -> Bool {
+            if cause is NIOHTTP2Errors.StreamClosed {
+                return true
+            } else if let clientError = cause as? AsyncHTTPClient.HTTPClientError, clientError == .remoteConnectionClosed {
+                return true
+            }
+
+            return false
+        }
     }
 
     /**

diff --git a/Sources/SmokeHTTPClient/HTTPOperationsClient+executeRetriableWithoutOutput.swift b/Sources/SmokeHTTPClient/HTTPOperationsClient+executeRetriableWithoutOutput.swift
@@ -18,8 +18,10 @@
 #if (os(Linux) && compiler(>=5.5)) || (!os(Linux) && compiler(>=5.5.2)) && canImport(_Concurrency)
 
 import Foundation
+import AsyncHTTPClient
 import NIO
 import NIOHTTP1
+import NIOHTTP2
 import Metrics
 import Tracing
 
@@ -46,6 +48,11 @@ public extension HTTPOperationsClient {
 
         var retriesRemaining: Int
 
+        // For requests that fail for transient connection issues (StreamClosed, remoteConnectionClosed)
+        // don't consume retry attempts and don't use expotential backoff
+        var abortedAttemptsRemaining: Int = 5
+        let waitOnAbortedAttemptMs = 2
+
         init(endpointOverride: URL?, requestComponents: HTTPRequestComponents, httpMethod: HTTPMethod,
              invocationContext: HTTPClientInvocationContext<InvocationReportingType, HandlerDelegateType>,
              eventLoopOverride eventLoop: EventLoop,
@@ -118,8 +125,21 @@ public extension HTTPOperationsClient {
 
             let shouldRetryOnError = retryOnError(error)
 
-            // if there are retries remaining and we should retry on this error
-            if retriesRemaining > 0 && shouldRetryOnError {
+            // For requests that fail for transient connection issues (StreamClosed, remoteConnectionClosed)
+            // don't consume retry attempts and don't use expotential backoff
+            if self.abortedAttemptsRemaining > 0 && treatAsAbortedAttempt(cause: error.cause) {
+                logger.debug(
+                    "Request aborted with error: \(error). Retrying in \(self.waitOnAbortedAttemptMs) ms.")
+
+                self.abortedAttemptsRemaining -= 1
+
+                try await Task.sleep(nanoseconds: UInt64(self.waitOnAbortedAttemptMs) * millisecondsToNanoSeconds)
+
+                try await self.executeWithoutOutput()
+
+                return
+                // if there are retries remaining (and haven't exhausted aborted attempts) and we should retry on this error
+            } else if self.abortedAttemptsRemaining > 0 && self.retriesRemaining > 0 && shouldRetryOnError {
                 // determine the required interval
                 let retryInterval = Int(retryConfiguration.getRetryInterval(retriesRemaining: retriesRemaining))
 
@@ -179,6 +199,16 @@ public extension HTTPOperationsClient {
                     retriableOutwardsRequest: StandardRetriableOutputRequestRecord(outputRequests: outputRequestRecords))
             }
         }
+
+        func treatAsAbortedAttempt(cause: Swift.Error) -> Bool {
+            if cause is NIOHTTP2Errors.StreamClosed {
+                return true
+            } else if let clientError = cause as? AsyncHTTPClient.HTTPClientError, clientError == .remoteConnectionClosed {
+                return true
+            }
+
+            return false
+        }
     }
 
     /**