Fix `@uppy/tus` retry when the POST request failed.

[Murderlon]

Fixes #3277

The problem on the client is that connectToServerSocket is called instead of a POST request when a file is removed, completed, or cancelled. But if the initial POST request failed, then it would still get a serverToken, which causes it to not try a POST request again. So we need to do another check in order to let it do the request again if it failed.

Uncertainties:

• Not sure if file.response is the best check for this.
• I can't test this in Jest and I have been having trouble testing it in e2e. Would need to mock a provider, like unsplash, or use actual credentials from a provider in the test suite. The latter is not ideal because you don't want your test depend on some external third-party server.

[mifi]

Awesome that youre looking into this! I'm struggling a bit to understand how the code works, but as far as I can see it works like this:

• file.serverToken gets set after the initial POST request (POST /drive/get/xx) has succeeded (but not after fail). So file.serverToken in a way represents whether or not the file has an ongoing Upload in companion.
• file.response gets set when companion reports the file has successfully finished (this happens some time after the initial POST has succeeded)

I'm not sure if adding a check on file.response is the right way also.

I can see however that serverToken gets set to null here on failure reported from companion:

https://github.com/transloadit/uppy/blob/bfe659820e1f04f6c229c704735abf3adc3d3325/packages/%40uppy/tus/src/index.js#L492

...which would effectively let the post request restart. But that only happens if useFastRemoteRetry is set to false, but it defaults to true: https://github.com/transloadit/uppy/blob/bfe659820e1f04f6c229c704735abf3adc3d3325/packages/%40uppy/tus/src/index.js#L68

So it seems like there is a default behavior where the client expects companion to retry the file when a socket connects (sonnectToServerSocket), but I don't see how that could work because companion will only start an upload the first time it gets a socket connection: https://github.com/transloadit/uppy/blob/1eb317ba3a3115596212ac50f1d81d3188dd0619/packages/%40uppy/companion/src/server/controllers/get.js#L38 And I think retrying a companion upload that has failed server-side would not work, because once companion's connection with the file's source is broken, it is broken and cannot be resumed unless restarting it using HTTP Range, which we are not using, and I fear many providers would not support that.

The flag was introduced here: #805

I think if we completely remove the useFastRemoteRetry flag from our code and always run the set serverToken: null-code, then the problem may be solved. But I may be missing something.

Btw I think I would rewrite the uploadRemote function to async/await to reduce nesting and make it easier to understand. for example:

  async uploadRemote (file) {
    this.resetUploaderReferences(file.id)

    const opts = { ...this.opts }
    if (file.tus) {
      // Install file-specific upload overrides.
      Object.assign(opts, file.tus)
    }

    this.uppy.emit('upload-started', file)
    this.uppy.log(file.remote.url)

    // If we have a a serverToken, means we already have a upload for this file running in companion
    // If so, just connect to the socket so we can get progress updates etc
    if (file.serverToken) {
      await this.connectToServerSocket(file)
      return
    }

    try {
      const Client = file.remote.providerOptions.provider ? Provider : RequestClient
      const client = new Client(this.uppy, file.remote.providerOptions)

      // !! cancellation is NOT supported at this stage yet
      const res = await client.post(file.remote.url, {
        ...file.remote.body,
        endpoint: opts.endpoint,
        uploadUrl: opts.uploadUrl,
        protocol: 'tus',
        size: file.data.size,
        headers: opts.headers,
        metadata: file.meta,
      })

      this.uppy.setFileState(file.id, { serverToken: res.token })
      file = this.uppy.getFile(file.id)

      await this.connectToServerSocket(file)
    } catch (err) {
      this.uppy.emit('upload-error', file, err)
      throw err
    }
  }

[Acconut]

You requested a review from me but in all honesty, I cannot provide any feedback. This part of Uppy is not really my area of expertise. Is there any tus-specific portion in this PR, you want me to look at?

[Murderlon]

You requested a review from me but in all honesty, I cannot provide any feedback. This part of Uppy is not really my area of expertise. Is there any tus-specific portion in this PR, you want me to look at?

I thought maybe you might be knowledgeable on the connection between client and server and whether the approach made sense. But I'm not looking for any specific feedback.

[mifi]

as for the useFastRemoteRetry flag, maybe @goto-bus-stop (#805) can provide some insights on whether it can be removed, or why it's not working with the flag set to true.

[Murderlon]

Closing this stale PR