fix uploadRemoteFile undefined (#4814)

* fix uploadRemoteFile undefined

for some reason, only when testing locally using VITE_COMPANION_URL=https://api2.transloadit.com/companion only
we get this error...
i'm thinking for some reason, the remote file gets serialized into state and then de-serialized again
causing remote.requestClient to be gone
so I solve it by instead storing the request clients in Uppy

fixes #4791

* fix bugs

* remove unrelated space changes

* apply suggestions

* Update Uppy.js

* fix lint

---------

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

examples/custom-provider/client/MyCustomProvider.jsx

@@ -27,6 +27,8 @@ export default class MyCustomProvider extends UIPlugin {
       pluginId: this.id,
     })
 
+    uppy.registerRequestClient(MyCustomProvider.name, this.provider)
+
     this.defaultLocale = {
       strings: {
         pluginNameMyUnsplash: 'MyUnsplash',

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

@@ -878,7 +878,7 @@ export default class AwsS3Multipart extends BasePlugin {
         }
         this.uppy.on('file-removed', removedHandler)
 
-        const uploadPromise = file.remote.requestClient.uploadRemoteFile(
+        const uploadPromise = this.uppy.getRequestClientForFile(file).uploadRemoteFile(
           file,
           this.#getCompanionClientArgs(file),
           { signal: controller.signal, getQueue },

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

@@ -281,7 +281,7 @@ export default class AwsS3 extends BasePlugin {
       }
       this.uppy.on('file-removed', removedHandler)
 
-      const uploadPromise = file.remote.requestClient.uploadRemoteFile(
+      const uploadPromise = this.uppy.getRequestClientForFile(file).uploadRemoteFile(
         file,
         this.#getCompanionClientArgs(file),
         { signal: controller.signal, getQueue },

packages/@uppy/core/src/Uppy.js

@@ -1386,6 +1386,30 @@ class Uppy {
     }
   }
 
+  // We need to store request clients by a unique ID, so we can share RequestClient instances across files
+  // this allows us to do rate limiting and synchronous operations like refreshing provider tokens
+  // example: refreshing tokens: if each file has their own requestclient,
+  // we don't have any way to synchronize all requests in order to
+  // - block all requests
+  // - refresh the token
+  // - unblock all requests and allow them to run with a the new access token
+  // back when we had a requestclient per file, once an access token expired,
+  // all 6 files would go ahead and refresh the token at the same time
+  // (calling /refresh-token up to 6 times), which will probably fail for some providers
+  #requestClientById = new Map()
+
+  registerRequestClient(id, client) {
+    this.#requestClientById.set(id, client)
+  }
+
+  /** @protected */
+  getRequestClientForFile (file) {
+    if (!file.remote) throw new Error(`Tried to get RequestClient for a non-remote file ${file.id}`)
+    const requestClient = this.#requestClientById.get(file.remote.requestClientId)
+    if (requestClient == null) throw new Error(`requestClientId "${file.remote.requestClientId}" not registered for file "${file.id}"`)
+    return requestClient
+  }
+
   /**
    * Restore an upload by its ID.
    */

packages/@uppy/provider-views/src/ProviderView/ProviderView.jsx

@@ -75,6 +75,8 @@ export default class ProviderView extends View {
       isSearchVisible: false,
       currentSelection: [],
     })
+
+    this.registerRequestClient()
   }
 
   // eslint-disable-next-line class-methods-use-this
@@ -399,7 +401,7 @@ export default class ProviderView extends View {
         // finished all async operations before we add any file
         // see https://github.com/transloadit/uppy/pull/4384
         this.plugin.uppy.log('Adding files from a remote provider')
-        this.plugin.uppy.addFiles(newFiles.map((file) => this.getTagFile(file)))
+        this.plugin.uppy.addFiles(newFiles.map((file) => this.getTagFile(file, this.requestClientId)))
 
         this.plugin.setPluginState({ filterInput: '' })
         messages.forEach(message => this.plugin.uppy.info(message))

packages/@uppy/provider-views/src/SearchProviderView/SearchProviderView.jsx

@@ -54,6 +54,8 @@ export default class SearchProviderView extends View {
 
     // Set default state for the plugin
     this.plugin.setPluginState(this.defaultState)
+
+    this.registerRequestClient()
   }
 
   // eslint-disable-next-line class-methods-use-this

packages/@uppy/provider-views/src/View.js

@@ -57,6 +57,11 @@ export default class View {
     uppy.info({ message, details: error.toString() }, 'error', 5000)
   }
 
+  registerRequestClient() {
+    this.requestClientId = this.provider.provider;
+    this.plugin.uppy.registerRequestClient(this.requestClientId, this.provider)
+  }
+
   // todo document what is a "tagFile" or get rid of this concept
   getTagFile (file) {
     const tagFile = {
@@ -78,15 +83,10 @@ export default class View {
         },
         providerName: this.provider.name,
         provider: this.provider.provider,
+        requestClientId: this.requestClientId,
       },
     }
 
-    // all properties on this object get saved into the Uppy store.
-    // Some users might serialize their store (for example using JSON.stringify),
-    // or when using Golden Retriever it will serialize state into e.g. localStorage.
-    // However RequestClient is not serializable so we need to prevent it from being serialized.
-    Object.defineProperty(tagFile.remote, 'requestClient', { value: this.provider, enumerable: false })
-
     const fileType = getFileType(tagFile)
 
     // TODO Should we just always use the thumbnail URL if it exists?

packages/@uppy/tus/src/index.js

@@ -489,7 +489,7 @@ export default class Tus extends BasePlugin {
         }
         this.uppy.on('file-removed', removedHandler)
 
-        const uploadPromise = file.remote.requestClient.uploadRemoteFile(
+        const uploadPromise = this.uppy.getRequestClientForFile(file).uploadRemoteFile(
           file,
           this.#getCompanionClientArgs(file),
           { signal: controller.signal, getQueue },

packages/@uppy/url/src/Url.jsx

@@ -48,13 +48,16 @@ function getFileNameFromUrl (url) {
   const { pathname } = new URL(url)
   return pathname.substring(pathname.lastIndexOf('/') + 1)
 }
+
 /**
  * Url
  *
  */
 export default class Url extends UIPlugin {
   static VERSION = packageJson.version
 
+  static requestClientId = Url.name
+
   constructor (uppy, opts) {
     super(uppy, opts)
     this.id = this.opts.id || 'Url'
@@ -88,6 +91,8 @@ export default class Url extends UIPlugin {
       companionHeaders: this.opts.companionHeaders,
       companionCookiesRule: this.opts.companionCookiesRule,
     })
+
+    this.uppy.registerRequestClient(Url.requestClientId, this.client)
   }
 
   getMeta (url) {
@@ -132,11 +137,10 @@ export default class Url extends UIPlugin {
             fileId: url,
             url,
           },
+          requestClientId: Url.requestClientId,
         },
       }
 
-      Object.defineProperty(tagFile.remote, 'requestClient', { value: this.client, enumerable: false })
-
       this.uppy.log('[Url] Adding remote file')
       try {
         return this.uppy.addFile(tagFile)

packages/@uppy/xhr-upload/src/index.js

@@ -473,7 +473,7 @@ export default class XHRUpload extends BasePlugin {
         }
         this.uppy.on('file-removed', removedHandler)
 
-        const uploadPromise = file.remote.requestClient.uploadRemoteFile(
+        const uploadPromise = this.uppy.getRequestClientForFile(file).uploadRemoteFile(
           file,
           this.#getCompanionClientArgs(file),
           { signal: controller.signal, getQueue },