Companion+client stability fixes, error handling and retry

packages/@uppy/aws-s3-multipart/src/index.js

@@ -1,5 +1,5 @@
 import BasePlugin from '@uppy/core/lib/BasePlugin.js'
-import { Provider, RequestClient } from '@uppy/companion-client'
+import { RequestClient } from '@uppy/companion-client'
 import EventManager from '@uppy/utils/lib/EventManager'
 import { RateLimitedQueue } from '@uppy/utils/lib/RateLimitedQueue'
 import { filterNonFailedFiles, filterFilesToEmitUploadStarted } from '@uppy/utils/lib/fileFilters'
@@ -854,11 +854,7 @@ export default class AwsS3Multipart extends BasePlugin {
 
     const promises = filesFiltered.map((file) => {
       if (file.isRemote) {
-        // INFO: the url plugin needs to use RequestClient,
-        // while others use Provider
-        const Client = file.remote.providerOptions.provider ? Provider : RequestClient
         const getQueue = () => this.requests
-        const client = new Client(this.uppy, file.remote.providerOptions, getQueue)
         this.#setResumableUploadsCapability(false)
         const controller = new AbortController()
 
@@ -867,10 +863,10 @@ export default class AwsS3Multipart extends BasePlugin {
         }
         this.uppy.on('file-removed', removedHandler)
 
-        const uploadPromise = client.uploadRemoteFile(
+        const uploadPromise = file.remote[Symbol.for('requestClient')].uploadRemoteFile(
           file,
           this.#getCompanionClientArgs(file),
-          { signal: controller.signal },
+          { signal: controller.signal, getQueue },
         )
 
         this.requests.wrapSyncFunction(() => {

packages/@uppy/aws-s3/src/index.js

@@ -28,7 +28,7 @@
 import BasePlugin from '@uppy/core/lib/BasePlugin.js'
 import AwsS3Multipart from '@uppy/aws-s3-multipart'
 import { RateLimitedQueue, internalRateLimitedQueue } from '@uppy/utils/lib/RateLimitedQueue'
-import { RequestClient, Provider } from '@uppy/companion-client'
+import { RequestClient } from '@uppy/companion-client'
 import { filterNonFailedFiles, filterFilesToEmitUploadStarted } from '@uppy/utils/lib/fileFilters'
 
 import packageJson from '../package.json'
@@ -273,22 +273,18 @@ export default class AwsS3 extends BasePlugin {
     if (file.error) throw new Error(file.error)
 
     if (file.isRemote) {
-      // INFO: the url plugin needs to use RequestClient,
-      // while others use Provider
-      const Client = file.remote.providerOptions.provider ? Provider : RequestClient
       const getQueue = () => this.#requests
-      const client = new Client(this.uppy, file.remote.providerOptions, getQueue)
       const controller = new AbortController()
 
       const removedHandler = (removedFile) => {
         if (removedFile.id === file.id) controller.abort()
       }
       this.uppy.on('file-removed', removedHandler)
 
-      const uploadPromise = client.uploadRemoteFile(
+      const uploadPromise = file.remote[Symbol.for('requestClient')].uploadRemoteFile(
         file,
         this.#getCompanionClientArgs(file),
-        { signal: controller.signal },
+        { signal: controller.signal, getQueue },
       )
 
       this.#requests.wrapSyncFunction(() => {

packages/@uppy/companion-client/package.json

@@ -23,7 +23,8 @@
   },
   "dependencies": {
     "@uppy/utils": "workspace:^",
-    "namespace-emitter": "^2.0.1"
+    "namespace-emitter": "^2.0.1",
+    "p-retry": "^6.1.0"
   },
   "devDependencies": {
     "vitest": "^0.34.5"

packages/@uppy/companion-client/src/Provider.js

@@ -30,8 +30,8 @@ function isOriginAllowed (origin, allowedOrigin) {
 export default class Provider extends RequestClient {
   #refreshingTokenPromise
 
-  constructor (uppy, opts, getQueue) {
-    super(uppy, opts, getQueue)
+  constructor (uppy, opts) {
+    super(uppy, opts)
     this.provider = opts.provider
     this.id = this.provider
     this.name = this.opts.name || getName(this.id)
@@ -140,8 +140,7 @@ export default class Provider extends RequestClient {
 
         authWindow.close()
         window.removeEventListener('message', handleToken)
-        this.setAuthToken(data.token)
-        resolve()
+        this.setAuthToken(data.token).then(() => resolve()).catch(reject)
       }
       window.addEventListener('message', handleToken)
     })
@@ -160,21 +159,24 @@ export default class Provider extends RequestClient {
     await this.#refreshingTokenPromise
 
     try {
-      // throw Object.assign(new Error(), { isAuthError: true }) // testing simulate access token expired (to refresh token)
-      // A better way to test this is for example with Google Drive:
+      // to test simulate access token expired (leading to a token token refresh),
+      // see mockAccessTokenExpiredError in companion/drive.
+      // If you want to test refresh token *and* access token invalid, do this for example with Google Drive:
       // While uploading, go to your google account settings,
       // "Third-party apps & services", then click "Companion" and "Remove access".
 
       return await super.request(...args)
     } catch (err) {
       // only handle auth errors (401 from provider), and only handle them if we have a (refresh) token
-      if (!err.isAuthError || !(await this.#getAuthToken())) throw err
+      const authTokenAfter = await this.#getAuthToken()
+      if (!err.isAuthError || !authTokenAfter) throw err
 
       if (this.#refreshingTokenPromise == null) {
         // Many provider requests may be starting at once, however refresh token should only be called once.
         // Once a refresh token operation has started, we need all other request to wait for this operation (atomically)
         this.#refreshingTokenPromise = (async () => {
           try {
+            this.uppy.log(`[CompanionClient] Refreshing expired auth token`, 'info')
             const response = await super.request({ path: this.refreshTokenUrl(), method: 'POST' })
             await this.setAuthToken(response.uppyAuthToken)
           } catch (refreshTokenErr) {