feat: add retry on file upload failure when offline

[dshukertjr]

What kind of change does this PR introduce?

Uploading large files on a spotty connection could be tough.

• Support Resumable uploads
• Retry failed requests

Resumable uploads is not yet supported on Supabase, but we can easily implement retries. This PR adds a retry logic to file uploads in a exponentially backing off manner like the following:

1. 400 ms +/- 25%
2. 800 ms +/- 25%
3. 1,600 ms +/- 25%
4. 3,200 ms +/- 25%
5. 6,400 ms +/- 25%
6. 12,800 ms +/- 25%
7. 25,600 ms +/- 25%
8. and beyond 30,000 ms +/- 25%

By default, the max attempts is set to 25, which should be roughly 10 minutes. 10 minutes was the default time set for Firebase storage. Users can change the retry count like the following. Not sure if this API would provide the best developer experience, so would love to hear comments/ suggestions about anything regarding this PR!

final uploadTask =
    client.from('public').upload('a.txt', file, maxAttempts: 8);

What is the current behavior?

If a file upload fails, it does no retry, which makes is hard to upload large files over a spotty network connection.

What is the new behavior?

File upload is automatically retried on SocketException or TimeoutException.

Additional context

This feature is not supported on JS client, but there was an feedback from customers handling large files. After some internal discussions, we thought it would make make sense to add this feature to the Flutter SDK.

[Vinzent03]

Am I Right that when I have no internet the method completes 10 mins after start with default configuration?
In addition I would make the max attempt count a config in the storage client as well so you don't have to specify it always.

[dshukertjr]

Am I Right that when I have no internet the method completes 10 mins after start with default configuration?
In addition I would make the max attempt count a config in the storage client as well so you don't have to specify it always.

Yup, that is correct. It might be a problem for some applications that are already using the library huh? I will update the default max attempts to 1.

Also, yeah let me update the code so that the user can just define the maxAttempts value when initializing the client so that they don't have to update it every single time. Do you think we should leave the option to override it in each individual functions though?

[Vinzent03]

Yeah I think setting the default to 1 is better.
I would keep the setting per call so that you can specify a default max attempts in the client, but can change that per call.

Also, is there any way to stop the retrying? For example if the user wants to abort the upload?

[dshukertjr]

Also, is there any way to stop the retrying? For example if the user wants to abort the upload?

Aborting the upload is a great point. I wonder how we can do it elegantly.

[Vinzent03]

I think we can't abort the actual request. dart-lang/http#424 So we can only abort the retrying. I don't know if this is how one would do it in Dart as well, but we could inspire from postgrest-js https://supabase.com/docs/reference/javascript/db-abortsignal

[dshukertjr]

@Vinzent03
I have updated the code! What do you think about the implementation?

final abortController = StorageAbortController();

await client.from('public').upload(
      path,
      file,
      abortController: abortController,
    );

...
// Some where else:
abortController.abort();

[Vinzent03]

Looks great! Do we want to additionally cancel the future as stated here?

How can we integrate the max attempts field with supabase-dart or supbase-flutter? Do we want to put that in each constructor? Another approach is to add a method to change the config. This would allow changing the count after object initialization as well.

[dshukertjr]

Looks great! Do we want to additionally cancel the future as stated here?

Thanks for the suggestion! Let me add it!

How can we integrate the max attempts field with supabase-dart or supbase-flutter? Do we want to put that in each constructor? Another approach is to add a method to change the config. This would allow changing the count after object initialization as well.

I want to say just put it in each constructors would be good enough, but would you think creating a separate configuration method might provide better developer experience?

[Vinzent03]

creating a separate configuration method might provide better developer experience?

I definitely think so.

One issue with cancelling the future could be though that, as far as understand this, the actual request is no cancelled so that the file may still get uploaded after abort. This should not be the case, so the abort should just stop the retrying.

[dshukertjr]

@Vinzent03

I definitely think so.

Let's do this!

One issue with cancelling the future could be though that, as far as understand this, the actual request is no cancelled so that the file may still get uploaded after abort. This should not be the case, so the abort should just stop the retrying.

With the current state, there will not be any more retry attempts once the user calls abortController.abort(). I tried to include CancelableOperation in the code, but I feel like it might not solve any issues on top of what is already implemented, so I ended up just leaving the code as is, but what do you think?

[Vinzent03]

Yeah I changed my mind and I think just aborting the retry is better than ignoring the result of the future. The dev can still do the same on the end result future. But we need to document that, because even after abort the request could go on for a long time until it finishes or fails.

Maybe abort is not the best name for this, because it doesn't really abort anything, but rather prevent/stop a new retry. What do you think?

[dshukertjr]

Maybe abort is not the best name for this, because it doesn't really abort anything, but rather prevent/stop a new retry. What do you think?

Agree. Let's rename it. What do you think about the following rename?

final retryController = StorageRetryController();

await client.from('public').upload(
      path,
      file,
      retryController: retryController,
    );

...
// Some where else:
retryController.cancel();

[Vinzent03]

That's great. Let's use these names.

[Vinzent03]

Looks great, but what about the config method to change the retry attempts count?

[dshukertjr]

@Vinzent03
Yeah, I was thinking about that one. What would the use case be when someone wants to change the retry settings after initialization? I could not think of any, so wanted to ask.

[Vinzent03]

Good question, I can't think of one currently. I just like offering the dev all freedom, when it's not complicated to implement it. I'm fine with leaving as it is as well.

[dshukertjr]

@Vinzent03
I agree that giving freedom is good, but at the same time, I don't want to confute people with different options that they might not need. I'd say let's ship this feature, and if such use case emerges, we can add the feature to update the config!

[Vinzent03]

I agree. Let's merge.