Skip to content

Commit

Permalink
Improve error handling with Polly
Browse files Browse the repository at this point in the history
  • Loading branch information
@sliekens
sliekens committed Aug 24, 2023
1 parent dad9b66 commit 147d3e1
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 97 deletions.
64 changes: 35 additions & 29 deletions GW2SDK.TestDataHelper/Container.cs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System;
using System.Net;
using System.Net.Http;
using System.Threading.Tasks;
using Microsoft.Extensions.DependencyInjection;
Expand All @@ -21,41 +20,48 @@ public Container()
client =>
{
client.BaseAddress = BaseAddress.DefaultUri;
// The default timeout is 100 seconds, but it's not always enough
// Due to rate limiting, an individual request can get stuck in a delayed retry-loop
// A better solution might be to queue up requests
// (so that new requests have to wait until there are no more delayed requests)
client.Timeout = TimeSpan.FromMinutes(5);
}
)
.ConfigurePrimaryHttpMessageHandler(
() => new SocketsHttpHandler
{
MaxConnectionsPerServer = 20,
// Creating a new connection shouldn't take more than 10 seconds
ConnectTimeout = TimeSpan.FromSeconds(10),
// Save bandwidth
AutomaticDecompression = System.Net.DecompressionMethods.GZip
}
)
// The API has rate limiting (by IP address) so wait and retry when the server indicates too many requests
.AddPolicyHandler(Policy.HandleResult<HttpResponseMessage>(response => response.StatusCode == TooManyRequests).WaitAndRetryAsync(10, _ => TimeSpan.FromSeconds(10)))

// The API can be disabled intentionally to avoid leaking spoilers, or it can be unavailable due to technical difficulties
// Since it's not easy to tell the difference, give it one retry
.AddPolicyHandler(Policy.HandleResult<HttpResponseMessage>(response => response.StatusCode == ServiceUnavailable).RetryAsync())

// Assume internal errors are retryable (within reason)
.AddPolicyHandler(Policy.HandleResult<HttpResponseMessage>(response => response.StatusCode is InternalServerError or BadGateway or GatewayTimeout).RetryAsync(5))

// Sometimes the API returns a Bad Request with an unknown error for perfectly valid requests
.AddPolicyHandler(Policy<HttpResponseMessage>.Handle<ArgumentException>(reason => reason.Message == "unknown error").RetryAsync())

// Abort each attempted request after max 20 seconds and perform retries (within reason)
.AddPolicyHandler(Policy<HttpResponseMessage>.Handle<TimeoutRejectedException>().RetryAsync(10))
.AddPolicyHandler(Policy.TimeoutAsync<HttpResponseMessage>(TimeSpan.FromSeconds(20), TimeoutStrategy.Optimistic))
.AddTypedClient<JsonAchievementService>()
.AddTypedClient<JsonItemPriceService>()
.AddTypedClient<JsonOrderBookService>()
.AddTypedClient<JsonItemService>()
.AddTypedClient<JsonRecipeService>()
.AddTypedClient<JsonSkinService>()
.ConfigurePrimaryHttpMessageHandler(
() => new SocketsHttpHandler { AutomaticDecompression = DecompressionMethods.GZip }
)
.AddPolicyHandler(
Policy.TimeoutAsync<HttpResponseMessage>(
TimeSpan.FromSeconds(100),
TimeoutStrategy.Optimistic
)
)
.AddPolicyHandler(
Policy<HttpResponseMessage>.HandleResult(
response => response.StatusCode is ServiceUnavailable
or GatewayTimeout
or BadGateway
or (HttpStatusCode)429 // TooManyRequests
)
.Or<TimeoutRejectedException>()
.WaitAndRetryForeverAsync(
retryAttempt => TimeSpan.FromSeconds(Math.Min(8, Math.Pow(2, retryAttempt)))
)
)
.AddPolicyHandler(
Policy.TimeoutAsync<HttpResponseMessage>(
TimeSpan.FromSeconds(30),
TimeoutStrategy.Optimistic
)
);
.AddTypedClient<JsonSkinService>();

serviceProvider = services.BuildServiceProvider();
}
Expand Down
73 changes: 30 additions & 43 deletions GW2SDK.Tests/TestInfrastructure/TestHttpClientFactory.cs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System;
using System.Net;
using System.Net.Http;
using System.Threading.Tasks;
using GuildWars2.Http;
Expand All @@ -8,6 +7,10 @@
using Polly.Timeout;
using static System.Net.HttpStatusCode;

#if NETFRAMEWORK
using static GuildWars2.Http.HttpStatusCodeEx;
#endif

namespace GuildWars2.Tests.TestInfrastructure;

public class TestHttpClientFactory : IHttpClientFactory, IAsyncDisposable
Expand Down Expand Up @@ -43,10 +46,9 @@ private static ServiceProvider BuildHttpClientProvider(Uri baseAddress)
http.BaseAddress = baseAddress;
// The default timeout is 100 seconds, but it's not always enough
// Due to rate limiting, an individual request can get stuck in a delayed retry-loop
// Requests can get stuck in a delayed retry-loop due to rate limiting
// A better solution might be to queue up requests
// (so that new requests have to wait until there are no more delayed requests)
// Perhaps a circuit breaker is also suitable
http.Timeout = TimeSpan.FromMinutes(5);
}
)
Expand All @@ -61,7 +63,10 @@ private static ServiceProvider BuildHttpClientProvider(Uri baseAddress)
MaxConnectionsPerServer = maxConnections,
// Creating a new connection shouldn't take more than 10 seconds
ConnectTimeout = TimeSpan.FromSeconds(10)
ConnectTimeout = TimeSpan.FromSeconds(10),
// Save bandwidth
AutomaticDecompression = System.Net.DecompressionMethods.GZip
}
)
#else
Expand All @@ -70,45 +75,27 @@ private static ServiceProvider BuildHttpClientProvider(Uri baseAddress)
)
#endif
.AddHttpMessageHandler<SchemaVersionHandler>()
.AddPolicyHandler(

// Transient errors for which we want a a delayed retry
// (currently only includes rate-limit errors)
Policy<HttpResponseMessage>.HandleResult(
response => response.StatusCode is (HttpStatusCode)429 // TooManyRequests
)
.WaitAndRetryForeverAsync(_ => TimeSpan.FromSeconds(10))
)
.AddPolicyHandler(

// Transient errors for which we want an immediate retry
Policy<HttpResponseMessage>
.HandleResult(response => response.StatusCode >= InternalServerError)
.Or<TimeoutRejectedException>()
.Or<UnauthorizedOperationException>(

// Sometimes the API fails to validate the access key
// This is a server error, real token problems result in a different error
// eg. "Invalid access token"
reason => reason.Message == "endpoint requires authentication"
)
.Or<ArgumentException>(

// Sometimes the API returns a Bad Request with an unknown error for perfectly valid requests
reason => reason.Message == "unknown error"
)
.RetryForeverAsync()
)
.AddPolicyHandler(

// An individual attempt shouldn't take more than 20 seconds to complete
// Assume longer means the API is stuck, and we should cancel the request
//
Policy.TimeoutAsync<HttpResponseMessage>(
TimeSpan.FromSeconds(20),
TimeoutStrategy.Optimistic
)
);

// The API has rate limiting (by IP address) so wait and retry when the server indicates too many requests
.AddPolicyHandler(Policy.HandleResult<HttpResponseMessage>(response => response.StatusCode == TooManyRequests).WaitAndRetryAsync(10, _ => TimeSpan.FromSeconds(10)))

// The API can be disabled intentionally to avoid leaking spoilers, or it can be unavailable due to technical difficulties
// Since it's not easy to tell the difference, give it one retry
.AddPolicyHandler(Policy.HandleResult<HttpResponseMessage>(response => response.StatusCode == ServiceUnavailable).RetryAsync())

// Assume internal errors are retryable (within reason)
.AddPolicyHandler(Policy.HandleResult<HttpResponseMessage>(response => response.StatusCode is InternalServerError or BadGateway or GatewayTimeout).RetryAsync(5))

// Sometimes the API fails to validate the access token even though the token is valid
// This is retryable because real token errors result in a different error message, eg. "Invalid access token"
.AddPolicyHandler(Policy<HttpResponseMessage>.Handle<UnauthorizedOperationException>(reason => reason.Message == "endpoint requires authentication").RetryAsync(10))

// Sometimes the API returns a Bad Request with an unknown error for perfectly valid requests
.AddPolicyHandler(Policy<HttpResponseMessage>.Handle<ArgumentException>(reason => reason.Message == "unknown error").RetryAsync())

// Abort each attempted request after max 20 seconds and perform retries (within reason)
.AddPolicyHandler(Policy<HttpResponseMessage>.Handle<TimeoutRejectedException>().RetryAsync(10))
.AddPolicyHandler(Policy.TimeoutAsync<HttpResponseMessage>(TimeSpan.FromSeconds(20), TimeoutStrategy.Optimistic));

return services.BuildServiceProvider();
}
Expand Down
49 changes: 24 additions & 25 deletions samples/PollyUsage/Program.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,36 +10,35 @@
var host = new HostBuilder().ConfigureServices(
services =>
{
// Gw2Client with the following policies applied
// * Cancel after 1 minute of waiting for a success response (including any retries)
// * In case of transient errors, retry with exponential delay (2s, 4s, 8s, 8s, 8s ...)
// * Each individual request (attempt) should complete in less than 20 seconds
services.AddHttpClient<Gw2Client>(
httpClient =>
{
httpClient.Timeout = TimeSpan.FromMinutes(1);
// Configure a timeout after which OperationCanceledException is thrown
// The default timeout is 100 seconds, but it's not always enough for background work
// because requests can get stuck in a delayed retry-loop due to rate limiting
httpClient.Timeout = TimeSpan.FromSeconds(600);
// For user interactive apps, you want to set a lower timeout
// to avoid long waiting periods when there are technical difficulties
httpClient.Timeout = TimeSpan.FromSeconds(20);
}
)
.AddPolicyHandler(
Policy<HttpResponseMessage>
.HandleResult(
response => response.StatusCode is ServiceUnavailable
or GatewayTimeout
or BadGateway
or TooManyRequests
)
.Or<TimeoutRejectedException>()
.WaitAndRetryForeverAsync(
retryAttempt =>
TimeSpan.FromSeconds(Math.Min(8, Math.Pow(2, retryAttempt)))
)
)
.AddPolicyHandler(
Policy.TimeoutAsync<HttpResponseMessage>(
TimeSpan.FromSeconds(20),
TimeoutStrategy.Optimistic
)
);
// The API has rate limiting (by IP address) so wait and retry when the server indicates too many requests
.AddPolicyHandler(Policy.HandleResult<HttpResponseMessage>(response => response.StatusCode == TooManyRequests).WaitAndRetryAsync(10, _ => TimeSpan.FromSeconds(10)))
// The API can be disabled intentionally to avoid leaking spoilers, or it can be unavailable due to technical difficulties
// Since it's not easy to tell the difference, give it one retry
.AddPolicyHandler(Policy.HandleResult<HttpResponseMessage>(response => response.StatusCode == ServiceUnavailable).RetryAsync())
// Assume internal errors are retryable (within reason)
.AddPolicyHandler(Policy.HandleResult<HttpResponseMessage>(response => response.StatusCode is InternalServerError or BadGateway or GatewayTimeout).RetryAsync(5))
// Sometimes the API returns a Bad Request with an unknown error for perfectly valid requests
.AddPolicyHandler(Policy<HttpResponseMessage>.Handle<ArgumentException>(reason => reason.Message == "unknown error").RetryAsync())
// Abort each attempted request after max 20 seconds and perform retries (within reason)
.AddPolicyHandler(Policy<HttpResponseMessage>.Handle<TimeoutRejectedException>().RetryAsync(10))
.AddPolicyHandler(Policy.TimeoutAsync<HttpResponseMessage>(TimeSpan.FromSeconds(20), TimeoutStrategy.Optimistic));
}
)
.Build();
Expand Down

0 comments on commit 147d3e1

Please sign in to comment.