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

Failed chunk upload callback response is undefined error #142

Open
luis-tan opened this issue May 20, 2024 · 1 comment
Open

Failed chunk upload callback response is undefined error #142

luis-tan opened this issue May 20, 2024 · 1 comment

Comments

@luis-tan
Copy link

If a chunk upload fails and the failed chunk upload callback is triggered, the preceding check accounts for the response potentially being undefined, but the callback itself does not.

This actually caused an issue in our production code where we couldn't even notify the user an error had ocurred because the "error" dispatch event never happened.

Here's the offending line:
https://github.com/muxinc/upchunk/blob/master/src/upchunk.ts#L749

I'm happy to open a PR, but couldn't find a contributions document. The message could simply be ammended to something along the lines of:

      const message = res ? `Server responded with ${res.statusCode}. Stopping upload.` : 'Unable to connect with server. Stopping upload.';

      this.dispatch('error', {
        message,
        chunk: this.chunkCount,
        attempts: this.attemptCount,
        response: res,
      });

It's unclear what caused the error in the first place, but at least this way I can show a message to the user.

@cjpillsbury
Copy link
Contributor

@luis-tan thanks for the issue! Yeah looks like some of our types are lying as well. Happy to accept a PR. If you could:

  1. make the changes you propose (your msg looks fine to me)
  2. update isFailedChunkUpload to remove the res is XhrResponseLike (that's the part that's lying)
  3. update failedChunkUploadCb signature so res: XhrResponseLike | undefined (so it's no longer lying with our more accurate type signature from (2))

Ideally we'd add some tests for this case as well in upchunk.spec.ts, but it sounds like you haven't quite pinned down the root cause, so I won't treat that as a blocker for your PR, but if you could chew on that a bit, that would be appreciated! Also happy to help/available for you to bounce ideas off of.

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

No branches or pull requests

2 participants