Better offline check instead of navigator.onLine (as it is fairly unreliable)

[ptgamr]

If you test offline mode by turning off your wifi or unplug your computer cable, it's probably be fine, the plugin tell you that "There is no internet connection".

However, I think that's not reflecting the real situation users might get. Sometimes it's because of provider issue, or just simply unplug the cable from the modem (wifi is still connected), Uppy failed to notify me that there is no internet. (mainly because navigator.onLine event is not firing), and the upload is stalled after that, the pending PATCH request never get completed (yes, I'm using Tus plugin)

There must be a better way, for example, if we're not receiving upload-progress event for certain amount of time (eg. 30 seconds), we could inform to the user: Experiencing network interuption... and then retry the upload, rather than let it hanging forever.

[arturi]

Agreed, we could probably do better with detecting network issues. Tus actually has retryDelays for this purpose, and in @uppy/xhr-upload we set a timer to abort requests when there’s no progress for X seconds.

@Acconut @goto-bus-stop what do you think, should we add a timeout and/or UI notifications about retryDelays? I‘m not sure if that’s possible with bare tus-js-client, without introducing our own times, since it only has onError callback.

[Acconut]

There must be a better way, for example, if we're not receiving upload-progress event for certain amount of time (eg. 30 seconds), we could inform to the user
in @uppy/xhr-upload we set a timer to abort requests when there’s no progress for X seconds.

That's an interesting approach. Does it work reliably or does it have issues with normally low uploads (e.g. in rural areas)? I am asking because I am considering adapting this directly into tus-js-client.

I‘m not sure if that’s possible with bare tus-js-client, without introducing our own times, since it only has onError callback.

Currently, there is no such functionality in tus-js-client. So you either have to add your own retries or try to detect network problems based on tus-js-client's progress events.

[lakesare]

Adding timeouts to tus-js-client sounds like a good idea.

I implemented timeouts in @uppy/tus using tus-js-client's progress events locally, but I think it would fit in tus-js-client better (maybe with the new timeout option that ignores the retryDelays option, and with the onError callback).

@Acconut, what do you think?

---

Update: We're considering adding timeouts to Core .on('progress') events instead of adding them separately to every provider.

[Acconut]

We're considering adding timeouts to Core .on('progress') events instead of adding them separately to every provider.

Is that decision final yet? From my perspective it would be nice to also offer timeouts over progress for tus-js-client directly :)

[lakesare]

@Acconut, we could benefit from timeouts in @uppy/aws-s3-multipart (2 places), and in @uppy/xhr-upload (3 places) apart from @uppy/tus.
So I think we should put timeout code into Core whether we add this functionality to tus-js-client or not (and we shouldn't use tus-js-client's option even if it's implemented).

@arturi, @goto-bus-stop?

[Acconut]

If you think that this is the best option for Uppy, please go ahead :)

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Murderlon]

To make this actionable:

1. Idea is to extend the existing upload-progress event in core, which is currently just calculating progress, to also add a timeout there. https://github.com/transloadit/uppy/blob/master/packages/@uppy/core/src/index.js#L1140
2. this.uppy.emit('upload-progress', file, data) should also accept a timeout argument, which is passed from plugins that support it, such as xhr-upload.
3. What happens if you add Tus to Uppy, will retryOptions ever interfere with timeout?
4. Do we go straight to error after the timeout in core is hit?

cc @arturi @Acconut @lakesare

[Acconut]

3\. What happens if you add Tus to Uppy, will `retryOptions` ever interfere with `timeout`?

Potentially yes, although I think it's unlikely. If a retry kicks in (e.g. because a PATCH request failed), a HEAD request must be sent first (with potential retries as well) before uploading resumes. During this time no progress event will be emitted.

Also, a few cents from my experience with progress events: Some implementations emit a progress event every X bytes (e.g. every 64KiB). So, if you have a 1KiB/s connection, it is normal to only have one progress event per minute :)

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.