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.

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

Tus10 auto-retry the upload when back online #135

Merged
merged 3 commits into from
May 13, 2017
Merged

Tus10 auto-retry the upload when back online #135

merged 3 commits into from
May 13, 2017

Conversation

oliverpool
Copy link
Contributor

This PR adds the option (default true) to "auto-retry" uploads when the use is back online.

Only the uploads already started, not completed and not paused are retried.

@arturi
Copy link
Contributor

arturi commented Dec 12, 2016

tus-js-client has built-in retries, but not related to navigator.online directly.

This PR seems like a good approach, but it would be cool if we had a second opinion from @Acconut and @kvz maybe. Should this be an uploader plugin setting/feature or global?

@oliverpool
Copy link
Contributor Author

oliverpool commented Dec 13, 2016

tus-js-client has built-in retries, but not related to navigator.online directly.

That's right. As you point out, it stops retrying when the navigator is offline.
Maybe this should be split into two options:

  • retryDelays behaving like the tus-js-client (abort when offline)
  • resumeWhenOnline (naming is hard) to auto-retry when the navigator is back online (if supported - try every x seconds otherwise)

@Acconut
Copy link
Member

Acconut commented Dec 13, 2016

Personally, I am in favor of the retryDelays and resumeWhenOnline options, as suggested by @oliverpool. Are you thinking of adding this feature only for the tus uploader or all uploaders?

@arturi
Copy link
Contributor

arturi commented Dec 13, 2016

Two options seem good with me too.

Are you thinking of adding this feature only for the tus uploader or all uploaders?

Yeah, that‘s what I’m asking, would it make sense for Multipart and, say future S3 uploader too?

@Acconut
Copy link
Member

Acconut commented Dec 13, 2016

For multipart uploads, only retrying makes sense as this operation is not resumable. In the case of a possible S3 uploader both could be useful.

@oliverpool
Copy link
Contributor Author

resumeWhenOnline can be renamed to retryWhenOnline (for non-resumable uploads).

For me, the main goal of those two options is to guarantee that everything will be done to complete the upload without requiring user action.

@tim-kos
Copy link
Member

tim-kos commented Feb 22, 2017

I have implemented this for Transloadit's jQuery SDK v3 the other week. A few bits to share:

  1. I named the event "reconnect", instead of "back-online". And then "disconnect" for disconnections. They go very well together naming-wise.

  2. I didn't make it an option when to retry/resume when reconnected. I just check every 3 seconds if we are back online and if so, I resume immediately. Less options/choice is more sometimes.

Just FYI. 👍

@arturi
Copy link
Contributor

arturi commented Feb 23, 2017

@tim-kos thanks for sharing! Do you listen to online and offline window events? I wonder if there is a need then to check when back online if browser emits the event? I guess there is a possibility of missing it or something.

@arturi arturi merged commit 5d4702f into transloadit:master May 13, 2017
@arturi
Copy link
Contributor

arturi commented May 13, 2017

Merged this, sorry for long delay. Thank you @oliverpool! Let’s test it in battle and add options as we go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants