3

I have a generic method for executing an asynchronous tasks in synchronous context with retries.

public static T RunWithRetries<T>(Task<T> task)
{
   while(...) // 3 attempts
   {
       try 
       {
          task.GetAwaiter().GetResult();
       }
       catch 
       {
         // sleep & retry later in case of some exceptions, for example 429
       }
   }
}

Then I'm passing any method from asynchronous API to run it like this.

SyncHelper.RunWithRetries(externalAPI.UploadAsync(fileRequest, fileStream));

The problem is that it works unless an exception happened during the request and we need to re-try. If an error happens, all subsequent retries are also throwing the same exception. So, my questions are

  1. Is this happening because of the fileStream object? It's in the using statement, so it's not being disposed for sure. Can the stream position after first upload attempt be a problem?
  2. Is it normal that the same Task object is being retried? Should I change the way I'm doing it to something better?
Gab
  • 471
  • 3
  • 10
  • 25
  • 1
    First of all, don't use `GetAwaiter().GetResult()`, just `await` it and make it `async`, otherwise you're going to have other problems. Second, I would suggest taking a look at [Polly](https://github.com/App-vNext/Polly), it's a library that does all the retry logic you are trying to achieve here. – DavidG Nov 26 '21 at 10:15
  • The project architecture does not allow me to use async/await unfortunately. There is a need of very big refactoring for that. Regarding Polly - I really would like to make it simple here and make it work without external tools, if possible. – Gab Nov 26 '21 at 10:18
  • Then refactor your project, this code is dangerous. – DavidG Nov 26 '21 at 10:19
  • 1
    Related: [How to use Task.WhenAny and implement retry](https://stackoverflow.com/questions/43763982/how-to-use-task-whenany-and-implement-retry) – Theodor Zoulias Nov 26 '21 at 10:21
  • @TheodorZoulias The answer which does not use Polly there seems to do the same that I'm doing. But it always fails for me when I'm doing an upload. Can it be because of the stream parameter or something related to multipart requests? – Gab Nov 26 '21 at 10:29
  • You're being told (politely) by the server that you're sending too many requests. You think the solution is to perform *more* requests? – Damien_The_Unbeliever Nov 26 '21 at 10:30
  • 3
    Reflecting to your questions: 1) If you have read the `fileStream` to issue the request to the downstream system then you have to seek back to the beginning before retrying a new request. 2) If a Task has already been finished then retrieving its result will not re-execute the task, it just returns the result. So, it would be advisable to receive a `Func>`, which can be called multiple times. `RunWithRetries(() => externalAPI.UploadAsync...` – Peter Csala Nov 26 '21 at 10:39
  • @Damien_The_Unbeliever The suggested approach in this case is exponential backoff strategy. I'm not going to do more requests. I will sleep N seconds and retry the request later when the server may be less busy. – Gab Nov 26 '21 at 10:47
  • 1
    @PeterCsala I'm trying now your approach of passing a Func and using it there. I'm also trying to create the stream right inside the passed function. I think you can convert your comment to answer, so I can accept it in case it will work. – Gab Nov 26 '21 at 11:30

1 Answers1

2

Is this happening because of the fileStream object? It's in the using statement, so it's not being disposed for sure. Can the stream position after first upload attempt be a problem?

Yes, this is one of your problems. Whenever a stream is read its Position is not reset to 0 automatically. If you try to re-read it then it will not read anything since the position is at the end of the stream.

So, you either have to create a new stream each and every time or rewind the stream to the beginning.

Is it normal that the same Task object is being retried? Should I change the way I'm doing it to something better?

Whenever a Task has been finished (either with a particular result or with an exception) then re-await-ing or retrieving its Result will not trigger a re-execution. It will simple return the value or the exception.

So, you have to create a new Task for each and every retry attempt. In order to do so you could anticipate a Func<Task<T>> in your RunWithRetries

public static T RunWithRetries<T>(Func<Task<T>> issueRequest)
{
    ...
    issueRequest().GetAwaiter().GetResult();

}

From the caller side it would look like this:

RunWithRetries(() => externalAPI.UploadAsync(fileRequest, new FileStream(...)));
//or
RunWithRetries(() => { fileStream.Position = 0; externalAPI.UploadAsync(fileRequest, fileStream); });
Peter Csala
  • 17,736
  • 16
  • 35
  • 75
  • 1
    I used re-create the stream in the passed function itself and it worked (but I had to use async-await to not get it disposed). RunWithRetries(async () => using(FileStream fileStream=...) { await externalAPI.UploadAsync(fileRequest, fileStream); }); – Gab Nov 26 '21 at 12:13