Added RetryHandler. (bunq/sdk_csharp#80) #106
Conversation
nduijvelshoff
commented
Jun 22, 2018
•
nduijvelshoff
commented
- Closes Add middleware/handler to throttle requests to conform to API rate limit #80
- Tested
nduijvelshoff
changed the title
|
@OGKevin I don't see why my commit message is failed, the regex is a full match. |
|
Yea the regex failure seems magic to me as well. But to my eyes it loos good so we will ignore it 😉 |
| @@ -99,7 +99,7 @@ public ApiClient(ApiContext apiContext) | |||
|
|
|||
| private HttpClient CreateHttpClient() | |||
| { | |||
| return new HttpClient(CreateHttpClientHandler()) | |||
| return new HttpClient(new RetryHandler(CreateHttpClientHandler())) | |||
There was a problem hiding this comment.
can the retry handler be included inside the CreateHttpClientHandler method ?
Looks better.
There was a problem hiding this comment.
I did this based on the example, but this can be done I think.
There was a problem hiding this comment.
When I move the RetryHandler is it showing some errors.
There was a problem hiding this comment.
Kept it on the same place due to error when changing it.
BunqSdk/Http/RetryHandler.cs
Outdated
| .WaitAndRetryAsync(3, retryAttempt => TimeSpan.FromSeconds(Math.Pow(3, retryAttempt))) | ||
| .ExecuteAsync(() => base.SendAsync(request, cancellationToken)); | ||
| } | ||
| } |
| HttpRequestMessage request, | ||
| CancellationToken cancellationToken) => | ||
| Policy | ||
| .Handle<HttpRequestException>() |
There was a problem hiding this comment.
Only do this on the bunq 429 exception please.
There was a problem hiding this comment.
This is now in the code.
There was a problem hiding this comment.
Hmm i meant to only retry when this exception is thrown TooManyRequestsException Cant you use this exception to catch and handle ?
|
@nduijvelshoff are you going to finish this ? |
|
@OGKevin Hi Kevin, I can do this... Just a bit busy this week. :-) |
nduijvelshoff
force-pushed
the
throttle-requests/sdk_csharp#80
branch
from
5bda31a to
8ce7884
Compare
|
I'll check it on Monday. Off for the weekend.
On Jun 29, 2018 16:34, Kevin Hellemun <notifications@github.com> wrote:
@OGKevin commented on this pull request.
________________________________
In BunqSdk/Http/RetryHandler.cs<#106 (comment)>:
+using System.Net.Http;
+using System.Threading;
+using System.Threading.Tasks;
+using Polly;
+
+namespace Bunq.Sdk.Http
+{
+ public class RetryHandler : DelegatingHandler
+ {
+ public RetryHandler(HttpClientHandler handler) : base(handler) { }
+
+ protected override Task<HttpResponseMessage> SendAsync(
+ HttpRequestMessage request,
+ CancellationToken cancellationToken) =>
+ Policy
+ .Handle<HttpRequestException>()
Hmm i meant to only retry when this exception is thrown TooManyRequestsException Cant you use this exception to catch and handle ?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#106 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ADmub1dzoiWAyt2hlIhFrXIanwq58DQEks5uBjrhgaJpZM4UzoCV>.
|
