Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use regular retry loop when aborted attempts are exhausted #137

Merged
merged 1 commit into from
Feb 2, 2024
Merged

Use regular retry loop when aborted attempts are exhausted #137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions Sources/SmokeHTTPClient/HTTPOperationsClient+executeRetriableWithOutput.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,9 @@ public extension HTTPOperationsClient {
let shouldRetryOnError = retryOnError(error)

// For requests that fail for transient connection issues (StreamClosed, remoteConnectionClosed)
// don't consume retry attempts and don't use expotential backoff
// don't consume retry attempts and don't use exponential backoff.
// If aborted attempts are exhausted, we'll treat the aborted attempt just like any other retriable
// error, by consuming a retry attempt and applying exponential backoff.
if self.abortedAttemptsRemaining > 0 && treatAsAbortedAttempt(cause: error.cause) {
logger.debug(
"Request aborted with error: \(error). Retrying in \(self.waitOnAbortedAttemptMs) ms.")
Expand All @@ -160,8 +162,8 @@ public extension HTTPOperationsClient {
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 {
// if there are retries remaining (and we've exhausted aborted attempts) and we should retry on this error
} else if self.retriesRemaining > 0 && shouldRetryOnError {
// determine the required interval
let retryInterval = Int(retryConfiguration.getRetryInterval(retriesRemaining: retriesRemaining))

Expand Down
8 changes: 5 additions & 3 deletions Sources/SmokeHTTPClient/HTTPOperationsClient+executeRetriableWithoutOutput.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
/**
Helper type that manages the state of a retriable async request.
*/
private class ExecuteWithoutOutputRetriable<InvocationReportingType: HTTPClientInvocationReporting, HandlerDelegateType: HTTPClientInvocationDelegate> {

Check warning on line 33 in Sources/SmokeHTTPClient/HTTPOperationsClient+executeRetriableWithoutOutput.swift

View workflow job for this annotation

GitHub Actions / SwiftLint version 3.2.1 

Line Length Violation: Line should be 150 characters or less; currently it has 156 characters (line_length)
let endpointOverride: URL?
let requestComponents: HTTPRequestComponents
let httpMethod: HTTPMethod
Expand Down Expand Up @@ -112,7 +112,7 @@
await self.onSuccess()
}

func onSuccess() async{

Check warning on line 115 in Sources/SmokeHTTPClient/HTTPOperationsClient+executeRetriableWithoutOutput.swift

View workflow job for this annotation

GitHub Actions / SwiftLint version 3.2.1 

Opening Brace Spacing Violation: Opening braces should be preceded by a single space and on the same line as the declaration (opening_brace)
// report success metric
invocationContext.reporting.successCounter?.increment()

Expand All @@ -125,7 +125,9 @@
let shouldRetryOnError = retryOnError(error)

// For requests that fail for transient connection issues (StreamClosed, remoteConnectionClosed)
// don't consume retry attempts and don't use expotential backoff
// don't consume retry attempts and don't use exponential backoff.
// If aborted attempts are exhausted, we'll treat the aborted attempt just like any other retriable
// error, by consuming a retry attempt and applying exponential backoff.
if self.abortedAttemptsRemaining > 0 && treatAsAbortedAttempt(cause: error.cause) {
logger.debug(
"Request aborted with error: \(error). Retrying in \(self.waitOnAbortedAttemptMs) ms.")
Expand All @@ -137,8 +139,8 @@
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 {
// if there are retries remaining (and we've exhausted aborted attempts) and we should retry on this error
} else if self.retriesRemaining > 0 && shouldRetryOnError {
// determine the required interval
let retryInterval = Int(retryConfiguration.getRetryInterval(retriesRemaining: retriesRemaining))

Expand Down Expand Up @@ -244,7 +246,7 @@
// use the specified event loop or pick one for the client to use for all retry attempts
let eventLoop = invocationContext.reporting.eventLoop ?? self.eventLoopGroup.next()

let retriable = ExecuteWithoutOutputRetriable<StandardHTTPClientInvocationReporting<InvocationReportingType.TraceContextType>, HandlerDelegateType>(

Check warning on line 249 in Sources/SmokeHTTPClient/HTTPOperationsClient+executeRetriableWithoutOutput.swift

View workflow job for this annotation

GitHub Actions / SwiftLint version 3.2.1 

Line Length Violation: Line should be 150 characters or less; currently it has 156 characters (line_length)
endpointOverride: endpointOverride, requestComponents: requestComponents,
httpMethod: httpMethod,
invocationContext: wrappingInvocationContext, eventLoopOverride: eventLoop, httpClient: self,
Expand Down
Loading