download-manager icon indicating copy to clipboard operation
download-manager copied to clipboard

Network error treatment

Open Mecharyry opened this issue 5 years ago • 4 comments

Problem

Collapsing #458 and #466 into one issue so everything is easier to deal with.

The crux of the issue is that all network issues are treated as the same when they are not. Loss of connectivity that is brief is different from a server always returning an error code that will never resolve in an asset being downloaded. We need to stop treating them like they are the same.

Potential Solution

These needs some discussion between the maintainers, @zegnus, @danybony and @Mecharyry, and the community that opened the original issues @matecode @Lingviston. We need to understand what level of control client applications need and how we can expose this in the library.

Notes

  • Alignment on the callback API when we perform network calls. Currently the pulling of a file provides more information than the size request.

Mecharyry avatar Feb 28 '19 08:02 Mecharyry

For me as a developer a download manager should be a tool. This means that any errors that result in download can not being started or finished should be reported to the client so that it can make an informed decision about what to do. That means all network errors or server errors like SSLHandShakeExceptions or HTTP40X. On the other hand, this also means that all problems that make a download possible, but not now, for example that there is no network connection or similar, should be handled by the tool itself. Maybe different configuration levels are useful.

matecode avatar Feb 28 '19 09:02 matecode

We've spent a little bit of time talking about this today and here's what I took away from our meeting:

  • FileDownloader.onError(String cause) needs to be more strongly typed so we return the network error code and an associated message.
  • DownloadError should be more strongly typed. Perhaps it should be DownloadError<T> so we type it to the particular scenario and the message becomes an instance of T instead of String. That way we provide tailed information based on what has broken.
  • Create an interface that can determine how individual network errors are handled for each batch. By default we would have an implementation that retries automatically every error. Clients can then override this information to customise how they would like each error to be treated.
  • We should follow this same process for the FileSizeRequester too. At the moment it returns just a FileSize which can be 0 in the event it fails.

@danybony @zegnus can you let me know if I have missed anything from the above?

Mecharyry avatar Feb 28 '19 14:02 Mecharyry

I've created a feature branch where we can track the current progress. I'll be branching from it to tackle the list above, will update this issue with more information as I have it.

Mecharyry avatar Mar 01 '19 08:03 Mecharyry

FileDownloader strong type -> https://github.com/novoda/download-manager/tree/DLM-483/file_downloader_strong_type

Mecharyry avatar Mar 07 '19 09:03 Mecharyry