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

Errors that happen in onProgress result in repeated HEAD requests #696

Open
tdewolff opened this issue Jun 6, 2024 · 2 comments
Open

Errors that happen in onProgress result in repeated HEAD requests #696

tdewolff opened this issue Jun 6, 2024 · 2 comments
Labels

Comments

@tdewolff
Copy link

tdewolff commented Jun 6, 2024

If there is an error in the onProgress callback, instead of reporting the error in the console, the JS client sends out HEAD requests (instead of PATCH requests) which is quite confusing!

Example:

      let upload = new tus.Upload(file, {
        endpoint: '/url',
        retryDelays: [0, 3000, 5000, 10000, 20000],
         metadata: {
          name: file.name,
          type: file.type,
          modtime: file.lastModified,
        },
        onError: function (error) {
          console.log(error);
        },
        onProgress: function (bytesUploaded, bytesTotal) {
          (null).method; // ERROR
        },
        onSuccess: function (e) {
          console.log('Download %s from %s', upload.file.name, upload.url)
        },
      });
@tdewolff tdewolff added the bug label Jun 6, 2024
@Acconut
Copy link
Member

Acconut commented Jun 7, 2024

That's right, good point. We don't have errors that are thrown inside user callbacks well. This is likely not limited to onProgress, but is also the case for onSuccess, onError, and onShouldRetry. I wonder what the best way to handle those errors is:

  • errors caught from user callbacks could trigger the onError, signaling that the upload was stopped because of this error. If no onError callback is registered, the errors is directly thrown.
  • errors caught from user callbacks are logged to the console. While this allows the upload to continue, such errors could easily be missed, especially when an application produces a significant amount of logs, making debugging much harder. In addition, there would be no way for the user to handle those errors.

My first temptation would be to go with the first approach, which makes error handling more present. What do you think?

@tdewolff
Copy link
Author

tdewolff commented Jun 7, 2024

Thanks Marius for the quick reply! My initial expectation (adhering to the principle of least-surprise) would be that errors in callbacks are simply not caught/handled (and thus appear in the console as an error by default). Perhaps this would mean re-throwing the error. Otherwise, option one seems good!

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

No branches or pull requests

2 participants