From 06e57c5c1194f8b3ebaa80b9f1fff9c2ad9bfb65 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Thu, 12 May 2022 11:36:42 +0200 Subject: [PATCH 01/16] Add `retryStatusCodes` to @uppy/tus --- e2e/clients/dashboard-tus/app.js | 2 +- e2e/cypress/integration/dashboard-tus.spec.ts | 25 +------------------ packages/@uppy/tus/src/index.js | 4 ++- packages/@uppy/tus/types/index.d.ts | 1 + website/src/docs/tus.md | 4 +++ 5 files changed, 10 insertions(+), 26 deletions(-) diff --git a/e2e/clients/dashboard-tus/app.js b/e2e/clients/dashboard-tus/app.js index a7c131fefc..385cf15d3a 100644 --- a/e2e/clients/dashboard-tus/app.js +++ b/e2e/clients/dashboard-tus/app.js @@ -10,7 +10,7 @@ import '@uppy/dashboard/dist/style.css' const companionUrl = 'http://localhost:3020' const uppy = new Uppy() .use(Dashboard, { target: '#app', inline: true }) - .use(Tus, { endpoint: 'https://tusd.tusdemo.net/files' }) + .use(Tus, { endpoint: 'https://tusd.tusdemo.net/files', retryStatusCodes: [429, 401] }) .use(Url, { target: Dashboard, companionUrl }) .use(Unsplash, { target: Dashboard, companionUrl }) diff --git a/e2e/cypress/integration/dashboard-tus.spec.ts b/e2e/cypress/integration/dashboard-tus.spec.ts index 50f227c0bb..96470ad25d 100644 --- a/e2e/cypress/integration/dashboard-tus.spec.ts +++ b/e2e/cypress/integration/dashboard-tus.spec.ts @@ -16,29 +16,6 @@ describe('Dashboard with Tus', () => { cy.intercept('http://localhost:3020/search/unsplash/*').as('unsplash') }) - it('should emit `error` and `upload-error` events on failed POST request', () => { - cy.get('@file-input').attachFile(['images/traffic.jpg']) - - const error = cy.spy() - const uploadError = cy.spy() - cy.window().then(({ uppy }) => { - uppy.on('upload-error', uploadError) - uppy.on('error', error) - }) - - cy.get('.uppy-StatusBar-actionBtn--upload').click() - - cy.intercept( - { method: 'POST', url: 'https://tusd.tusdemo.net/*', times: 1 }, - { statusCode: 401, body: { code: 401, message: 'Expired JWT Token' } }, - ).as('post') - - cy.wait('@post').then(() => { - expect(error).to.be.called - expect(uploadError).to.be.called - }) - }) - it('should upload cat image successfully', () => { cy.get('@file-input').attachFile('images/cat.jpg') cy.get('.uppy-StatusBar-actionBtn--upload').click() @@ -48,7 +25,7 @@ describe('Dashboard with Tus', () => { cy.get('.uppy-StatusBar-statusPrimary').should('contain', 'Complete') }) - it('should start exponential backoff when receiving HTTP 429', () => { + it('should start exponential backoff on HTTP 429 from `retryStatusCodes`', () => { cy.get('@file-input').attachFile(['images/baboon.png']) cy.get('.uppy-StatusBar-actionBtn--upload').click() diff --git a/packages/@uppy/tus/src/index.js b/packages/@uppy/tus/src/index.js index a7ca86eaa4..617aa35e41 100644 --- a/packages/@uppy/tus/src/index.js +++ b/packages/@uppy/tus/src/index.js @@ -71,6 +71,7 @@ module.exports = class Tus extends BasePlugin { useFastRemoteRetry: true, limit: 20, retryDelays: tusDefaultOptions.retryDelays, + retryStatusCodes: [429], withCredentials: false, } @@ -278,7 +279,8 @@ module.exports = class Tus extends BasePlugin { uploadOptions.onShouldRetry = (err) => { const status = err?.originalResponse?.getStatus() - if (status === 429) { + + if (this.opts.retryStatusCodes.includes(status)) { // HTTP 429 Too Many Requests => to avoid the whole download to fail, pause all requests. if (!this.requests.isPaused) { const next = this.#retryDelayIterator?.next() diff --git a/packages/@uppy/tus/types/index.d.ts b/packages/@uppy/tus/types/index.d.ts index a140811bfe..418342fc36 100644 --- a/packages/@uppy/tus/types/index.d.ts +++ b/packages/@uppy/tus/types/index.d.ts @@ -17,6 +17,7 @@ export interface TusOptions extends PluginOptions, TusUploadOptions { limit?: number useFastRemoteRetry?: boolean withCredentials?: boolean + retryStatusCodes: Array } declare class Tus extends BasePlugin {} diff --git a/website/src/docs/tus.md b/website/src/docs/tus.md index d70f1edc60..8ffb588878 100644 --- a/website/src/docs/tus.md +++ b/website/src/docs/tus.md @@ -87,6 +87,10 @@ When uploading a chunk fails, automatically try again after the millisecond inte Set to `null` to disable automatic retries, and fail instantly if any chunk fails to upload. +### `retryStatusCodes: [429]` + +Tus can retry with [exponential backoff](https://en.wikipedia.org/wiki/Exponential_backoff) when receiving 429 (Too Many Requests), or other configured status codes. This can be useful to retry when you hit an API rate limit or when your authentication token is expired. + ### `metaFields: null` Pass an array of field names to limit the metadata fields that will be added to uploads as [Tus Metadata](https://tus.io/protocols/resumable-upload.html#upload-metadata). From 272aceb2cf52ecbcab9aa3d691a9fd8235c627d7 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Thu, 12 May 2022 12:01:43 +0200 Subject: [PATCH 02/16] Use image for URL test from repo --- e2e/cypress/integration/dashboard-tus.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/cypress/integration/dashboard-tus.spec.ts b/e2e/cypress/integration/dashboard-tus.spec.ts index 96470ad25d..7bc146fffc 100644 --- a/e2e/cypress/integration/dashboard-tus.spec.ts +++ b/e2e/cypress/integration/dashboard-tus.spec.ts @@ -45,7 +45,7 @@ describe('Dashboard with Tus', () => { it('should upload remote image with URL plugin', () => { cy.get('[data-cy="Url"]').click() - cy.get('.uppy-Url-input').type('https://via.placeholder.com/600x400') + cy.get('.uppy-Url-input').type('https://github.com/transloadit/uppy/raw/main/e2e/cypress/fixtures/images/cat.jpg') cy.get('.uppy-Url-importButton').click() cy.get('.uppy-StatusBar-actionBtn--upload').click() cy.wait('@url') From 4c07d08bd533b3b8e190cb5c94dee2d7971c31c0 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Mon, 16 May 2022 17:00:17 +0200 Subject: [PATCH 03/16] Revert "Add `retryStatusCodes` to @uppy/tus" This reverts commit 06e57c5c1194f8b3ebaa80b9f1fff9c2ad9bfb65. --- e2e/clients/dashboard-tus/app.js | 2 +- e2e/cypress/integration/dashboard-tus.spec.ts | 25 ++++++++++++++++++- packages/@uppy/tus/src/index.js | 4 +-- packages/@uppy/tus/types/index.d.ts | 1 - website/src/docs/tus.md | 4 --- 5 files changed, 26 insertions(+), 10 deletions(-) diff --git a/e2e/clients/dashboard-tus/app.js b/e2e/clients/dashboard-tus/app.js index 385cf15d3a..a7c131fefc 100644 --- a/e2e/clients/dashboard-tus/app.js +++ b/e2e/clients/dashboard-tus/app.js @@ -10,7 +10,7 @@ import '@uppy/dashboard/dist/style.css' const companionUrl = 'http://localhost:3020' const uppy = new Uppy() .use(Dashboard, { target: '#app', inline: true }) - .use(Tus, { endpoint: 'https://tusd.tusdemo.net/files', retryStatusCodes: [429, 401] }) + .use(Tus, { endpoint: 'https://tusd.tusdemo.net/files' }) .use(Url, { target: Dashboard, companionUrl }) .use(Unsplash, { target: Dashboard, companionUrl }) diff --git a/e2e/cypress/integration/dashboard-tus.spec.ts b/e2e/cypress/integration/dashboard-tus.spec.ts index e49058b838..e3be181121 100644 --- a/e2e/cypress/integration/dashboard-tus.spec.ts +++ b/e2e/cypress/integration/dashboard-tus.spec.ts @@ -16,6 +16,29 @@ describe('Dashboard with Tus', () => { cy.intercept('http://localhost:3020/search/unsplash/*').as('unsplash') }) + it('should emit `error` and `upload-error` events on failed POST request', () => { + cy.get('@file-input').attachFile(['images/traffic.jpg']) + + const error = cy.spy() + const uploadError = cy.spy() + cy.window().then(({ uppy }) => { + uppy.on('upload-error', uploadError) + uppy.on('error', error) + }) + + cy.get('.uppy-StatusBar-actionBtn--upload').click() + + cy.intercept( + { method: 'POST', url: 'https://tusd.tusdemo.net/*', times: 1 }, + { statusCode: 401, body: { code: 401, message: 'Expired JWT Token' } }, + ).as('post') + + cy.wait('@post').then(() => { + expect(error).to.be.called + expect(uploadError).to.be.called + }) + }) + it('should upload cat image successfully', () => { cy.get('@file-input').attachFile('images/cat.jpg') cy.get('.uppy-StatusBar-actionBtn--upload').click() @@ -25,7 +48,7 @@ describe('Dashboard with Tus', () => { cy.get('.uppy-StatusBar-statusPrimary').should('contain', 'Complete') }) - it('should start exponential backoff on HTTP 429 from `retryStatusCodes`', () => { + it('should start exponential backoff when receiving HTTP 429', () => { cy.get('@file-input').attachFile(['images/baboon.png']) cy.get('.uppy-StatusBar-actionBtn--upload').click() diff --git a/packages/@uppy/tus/src/index.js b/packages/@uppy/tus/src/index.js index 1f222599c1..5b8e9aa71e 100644 --- a/packages/@uppy/tus/src/index.js +++ b/packages/@uppy/tus/src/index.js @@ -72,7 +72,6 @@ export default class Tus extends BasePlugin { useFastRemoteRetry: true, limit: 20, retryDelays: tusDefaultOptions.retryDelays, - retryStatusCodes: [429], withCredentials: false, } @@ -280,8 +279,7 @@ export default class Tus extends BasePlugin { uploadOptions.onShouldRetry = (err) => { const status = err?.originalResponse?.getStatus() - - if (this.opts.retryStatusCodes.includes(status)) { + if (status === 429) { // HTTP 429 Too Many Requests => to avoid the whole download to fail, pause all requests. if (!this.requests.isPaused) { const next = this.#retryDelayIterator?.next() diff --git a/packages/@uppy/tus/types/index.d.ts b/packages/@uppy/tus/types/index.d.ts index 418342fc36..a140811bfe 100644 --- a/packages/@uppy/tus/types/index.d.ts +++ b/packages/@uppy/tus/types/index.d.ts @@ -17,7 +17,6 @@ export interface TusOptions extends PluginOptions, TusUploadOptions { limit?: number useFastRemoteRetry?: boolean withCredentials?: boolean - retryStatusCodes: Array } declare class Tus extends BasePlugin {} diff --git a/website/src/docs/tus.md b/website/src/docs/tus.md index 8ffb588878..d70f1edc60 100644 --- a/website/src/docs/tus.md +++ b/website/src/docs/tus.md @@ -87,10 +87,6 @@ When uploading a chunk fails, automatically try again after the millisecond inte Set to `null` to disable automatic retries, and fail instantly if any chunk fails to upload. -### `retryStatusCodes: [429]` - -Tus can retry with [exponential backoff](https://en.wikipedia.org/wiki/Exponential_backoff) when receiving 429 (Too Many Requests), or other configured status codes. This can be useful to retry when you hit an API rate limit or when your authentication token is expired. - ### `metaFields: null` Pass an array of field names to limit the metadata fields that will be added to uploads as [Tus Metadata](https://tus.io/protocols/resumable-upload.html#upload-metadata). From 2c0029bc1eb7fa1b712db4b44b76190690546de2 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Mon, 16 May 2022 17:50:10 +0200 Subject: [PATCH 04/16] Implement `onShouldRetry` as option --- packages/@uppy/tus/src/index.js | 9 ++++++++- packages/@uppy/tus/types/index.d.ts | 24 ++++++++++++++---------- website/src/docs/tus.md | 24 ++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 11 deletions(-) diff --git a/packages/@uppy/tus/src/index.js b/packages/@uppy/tus/src/index.js index 5b8e9aa71e..6a22300b9d 100644 --- a/packages/@uppy/tus/src/index.js +++ b/packages/@uppy/tus/src/index.js @@ -277,8 +277,15 @@ export default class Tus extends BasePlugin { resolve(upload) } - uploadOptions.onShouldRetry = (err) => { + if (this.opts.onShouldRetry !== null) { + uploadOptions.onShouldRetry = (err) => opts.onShouldRetry(err, defaultOnShouldRetry) + } else { + uploadOptions.onShouldRetry = defaultOnShouldRetry + } + + function defaultOnShouldRetry (err) { const status = err?.originalResponse?.getStatus() + if (status === 429) { // HTTP 429 Too Many Requests => to avoid the whole download to fail, pause all requests. if (!this.requests.isPaused) { diff --git a/packages/@uppy/tus/types/index.d.ts b/packages/@uppy/tus/types/index.d.ts index a140811bfe..063f177e4d 100644 --- a/packages/@uppy/tus/types/index.d.ts +++ b/packages/@uppy/tus/types/index.d.ts @@ -1,22 +1,26 @@ import type { PluginOptions, BasePlugin } from '@uppy/core' import type { UploadOptions } from 'tus-js-client' - type TusUploadOptions = Pick> +type TusUploadOptions = Pick> + +type Next = (err: Error) => boolean export interface TusOptions extends PluginOptions, TusUploadOptions { metaFields?: string[] | null limit?: number useFastRemoteRetry?: boolean withCredentials?: boolean + onShouldRetry: (err: Error, next: Next) => boolean } declare class Tus extends BasePlugin {} diff --git a/website/src/docs/tus.md b/website/src/docs/tus.md index d70f1edc60..c5923abbe6 100644 --- a/website/src/docs/tus.md +++ b/website/src/docs/tus.md @@ -87,6 +87,30 @@ When uploading a chunk fails, automatically try again after the millisecond inte Set to `null` to disable automatic retries, and fail instantly if any chunk fails to upload. +### `onShouldRetry: (err, next) => next(err)` + +When an upload fails `onShouldRetry` is called with the error and the default retry logic as the second argument. The default retry logic is an [exponential backoff](https://en.wikipedia.org/wiki/Exponential_backoff) algorithm triggered on HTTP 429 (Too Many Requests) errors. Meaning if your server (or proxy) returns HTTP 429 because it’s being overloaded, @uppy/tus will find the ideal sweet spot to keep uploading without overloading. + +If you want to extend this functionality, for instance to retry on unauthorized requests (to retrieve a new authentication token): + +```js +import Uppy from '@uppy/core' +import Tus from '@uppy/tus' + +const uppy = new Uppy().use(Tus, { endpoint: '', onShouldRetry }) + +async function onShouldRetry (err) { + if (err?.originalResponse?.getStatus() === 401) { + return true + } + return next(err) +} +``` + +### `onBeforeRequest: (req) => void` + +See [`onBeforeRequest`](https://github.com/tus/tus-js-client/blob/master/docs/api.md#onbeforerequest) from `tus-js-client`. + ### `metaFields: null` Pass an array of field names to limit the metadata fields that will be added to uploads as [Tus Metadata](https://tus.io/protocols/resumable-upload.html#upload-metadata). From e83830e1fd3dd0470d64d55c7a5292c6fc4185d1 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Mon, 16 May 2022 17:53:57 +0200 Subject: [PATCH 05/16] Test it in e2e --- e2e/clients/dashboard-tus/app.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/e2e/clients/dashboard-tus/app.js b/e2e/clients/dashboard-tus/app.js index a7c131fefc..b4c5a5777a 100644 --- a/e2e/clients/dashboard-tus/app.js +++ b/e2e/clients/dashboard-tus/app.js @@ -10,9 +10,16 @@ import '@uppy/dashboard/dist/style.css' const companionUrl = 'http://localhost:3020' const uppy = new Uppy() .use(Dashboard, { target: '#app', inline: true }) - .use(Tus, { endpoint: 'https://tusd.tusdemo.net/files' }) + .use(Tus, { endpoint: 'https://tusd.tusdemo.net/files', onShouldRetry }) .use(Url, { target: Dashboard, companionUrl }) .use(Unsplash, { target: Dashboard, companionUrl }) +function onShouldRetry (err, next) { + if (err?.originalResponse?.getStatus() === 418) { + return true + } + return next(err) +} + // Keep this here to access uppy in tests window.uppy = uppy From 5e6d73e40a7c1d63d44686245672542e513f0886 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Mon, 16 May 2022 17:54:20 +0200 Subject: [PATCH 06/16] Fix docs --- website/src/docs/tus.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/src/docs/tus.md b/website/src/docs/tus.md index c5923abbe6..5cc2ec3825 100644 --- a/website/src/docs/tus.md +++ b/website/src/docs/tus.md @@ -99,7 +99,7 @@ import Tus from '@uppy/tus' const uppy = new Uppy().use(Tus, { endpoint: '', onShouldRetry }) -async function onShouldRetry (err) { +async function onShouldRetry (err, next) { if (err?.originalResponse?.getStatus() === 401) { return true } From 82b8be5b525956fef42bba6243ee5c184ecc44f3 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Mon, 16 May 2022 18:05:53 +0200 Subject: [PATCH 07/16] Fix test --- e2e/cypress/integration/dashboard-tus.spec.ts | 23 ------------------- packages/@uppy/tus/src/index.js | 4 ++-- 2 files changed, 2 insertions(+), 25 deletions(-) diff --git a/e2e/cypress/integration/dashboard-tus.spec.ts b/e2e/cypress/integration/dashboard-tus.spec.ts index e3be181121..59ad67d6bc 100644 --- a/e2e/cypress/integration/dashboard-tus.spec.ts +++ b/e2e/cypress/integration/dashboard-tus.spec.ts @@ -16,29 +16,6 @@ describe('Dashboard with Tus', () => { cy.intercept('http://localhost:3020/search/unsplash/*').as('unsplash') }) - it('should emit `error` and `upload-error` events on failed POST request', () => { - cy.get('@file-input').attachFile(['images/traffic.jpg']) - - const error = cy.spy() - const uploadError = cy.spy() - cy.window().then(({ uppy }) => { - uppy.on('upload-error', uploadError) - uppy.on('error', error) - }) - - cy.get('.uppy-StatusBar-actionBtn--upload').click() - - cy.intercept( - { method: 'POST', url: 'https://tusd.tusdemo.net/*', times: 1 }, - { statusCode: 401, body: { code: 401, message: 'Expired JWT Token' } }, - ).as('post') - - cy.wait('@post').then(() => { - expect(error).to.be.called - expect(uploadError).to.be.called - }) - }) - it('should upload cat image successfully', () => { cy.get('@file-input').attachFile('images/cat.jpg') cy.get('.uppy-StatusBar-actionBtn--upload').click() diff --git a/packages/@uppy/tus/src/index.js b/packages/@uppy/tus/src/index.js index 6a22300b9d..5cc71cc382 100644 --- a/packages/@uppy/tus/src/index.js +++ b/packages/@uppy/tus/src/index.js @@ -277,8 +277,8 @@ export default class Tus extends BasePlugin { resolve(upload) } - if (this.opts.onShouldRetry !== null) { - uploadOptions.onShouldRetry = (err) => opts.onShouldRetry(err, defaultOnShouldRetry) + if (opts.onShouldRetry !== null) { + uploadOptions.onShouldRetry = async (err) => opts.onShouldRetry(err, defaultOnShouldRetry) } else { uploadOptions.onShouldRetry = defaultOnShouldRetry } From f52f63168531175275da56fdfb3eb94bff86c842 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Mon, 16 May 2022 18:13:22 +0200 Subject: [PATCH 08/16] Fix issue with `this` --- packages/@uppy/tus/src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@uppy/tus/src/index.js b/packages/@uppy/tus/src/index.js index e1d5211d71..d7c9ffb624 100644 --- a/packages/@uppy/tus/src/index.js +++ b/packages/@uppy/tus/src/index.js @@ -278,7 +278,7 @@ export default class Tus extends BasePlugin { } if (opts.onShouldRetry !== null) { - uploadOptions.onShouldRetry = async (err) => opts.onShouldRetry(err, defaultOnShouldRetry) + uploadOptions.onShouldRetry = async (err) => opts.onShouldRetry(err, defaultOnShouldRetry.bind(this)) } else { uploadOptions.onShouldRetry = defaultOnShouldRetry } From 823600f0acc9db71111c0d07df5a60d56c0d70d2 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Tue, 17 May 2022 12:40:40 +0200 Subject: [PATCH 09/16] Remove async from onShouldRetry and pass all tus-js-client args --- packages/@uppy/tus/src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@uppy/tus/src/index.js b/packages/@uppy/tus/src/index.js index d7c9ffb624..d3aa5aa1d2 100644 --- a/packages/@uppy/tus/src/index.js +++ b/packages/@uppy/tus/src/index.js @@ -278,7 +278,7 @@ export default class Tus extends BasePlugin { } if (opts.onShouldRetry !== null) { - uploadOptions.onShouldRetry = async (err) => opts.onShouldRetry(err, defaultOnShouldRetry.bind(this)) + uploadOptions.onShouldRetry = (...args) => opts.onShouldRetry(...args, defaultOnShouldRetry.bind(this)) } else { uploadOptions.onShouldRetry = defaultOnShouldRetry } From 7c0b1dba06f7478ca7f9718b8130baa239251d1c Mon Sep 17 00:00:00 2001 From: Murderlon Date: Tue, 17 May 2022 12:41:16 +0200 Subject: [PATCH 10/16] Improve docs --- website/src/docs/tus.md | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/website/src/docs/tus.md b/website/src/docs/tus.md index 5cc2ec3825..7f1c6b0849 100644 --- a/website/src/docs/tus.md +++ b/website/src/docs/tus.md @@ -37,6 +37,10 @@ const { Tus } = Uppy ## Options +**Note**: all options are passed to `tus-js-client` and we document the ones here that we added or changed. This means you can also pass functions like [`onBeforeRequest`](https://github.com/tus/tus-js-client/blob/master/docs/api.md#onbeforerequest) and [`onAfterResponse`](https://github.com/tus/tus-js-client/blob/master/docs/api.md#onafterresponse). + +We recommended taking a look at the [API reference](https://github.com/tus/tus-js-client/blob/master/docs/api.md) from `tus-js-client` to know what is supported. + ### `id: 'Tus'` A unique identifier for this plugin. It defaults to `'Tus'`. @@ -87,7 +91,7 @@ When uploading a chunk fails, automatically try again after the millisecond inte Set to `null` to disable automatic retries, and fail instantly if any chunk fails to upload. -### `onShouldRetry: (err, next) => next(err)` +### `onShouldRetry: (err, retryAttempt, options, next) => next(err)` When an upload fails `onShouldRetry` is called with the error and the default retry logic as the second argument. The default retry logic is an [exponential backoff](https://en.wikipedia.org/wiki/Exponential_backoff) algorithm triggered on HTTP 429 (Too Many Requests) errors. Meaning if your server (or proxy) returns HTTP 429 because it’s being overloaded, @uppy/tus will find the ideal sweet spot to keep uploading without overloading. @@ -97,19 +101,27 @@ If you want to extend this functionality, for instance to retry on unauthorized import Uppy from '@uppy/core' import Tus from '@uppy/tus' -const uppy = new Uppy().use(Tus, { endpoint: '', onShouldRetry }) +new Uppy().use(Tus, { endpoint: '', onBeforeRequest, onShouldRetry, onAfterResponse }) -async function onShouldRetry (err, next) { +async function onBeforeRequest (req) { + const xhr = req.getUnderlyingObject() + const token = await getAuthToken() + xhr.headers = { ...xhr.headers, authorization: `Bearer ${token}` } +} + +function onShouldRetry (err, retryAttempt, options, next) { if (err?.originalResponse?.getStatus() === 401) { return true } return next(err) } -``` -### `onBeforeRequest: (req) => void` - -See [`onBeforeRequest`](https://github.com/tus/tus-js-client/blob/master/docs/api.md#onbeforerequest) from `tus-js-client`. +async function onAfterResponse (req, res) { + if (res.getStatus() === 401) { + await refreshAuthToken() + } +} +``` ### `metaFields: null` From 8a00aef69addd532f9d38278b6a270965941370c Mon Sep 17 00:00:00 2001 From: Murderlon Date: Tue, 17 May 2022 12:46:08 +0200 Subject: [PATCH 11/16] Small doc fix --- website/src/docs/tus.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/website/src/docs/tus.md b/website/src/docs/tus.md index 7f1c6b0849..7fd3470d15 100644 --- a/website/src/docs/tus.md +++ b/website/src/docs/tus.md @@ -104,9 +104,8 @@ import Tus from '@uppy/tus' new Uppy().use(Tus, { endpoint: '', onBeforeRequest, onShouldRetry, onAfterResponse }) async function onBeforeRequest (req) { - const xhr = req.getUnderlyingObject() const token = await getAuthToken() - xhr.headers = { ...xhr.headers, authorization: `Bearer ${token}` } + req.setHeader('Authorization', `Bearer ${token}`) } function onShouldRetry (err, retryAttempt, options, next) { From 32fe09e006e73a2e45ca22744c5d7c8b603d76ab Mon Sep 17 00:00:00 2001 From: Murderlon Date: Tue, 17 May 2022 12:51:37 +0200 Subject: [PATCH 12/16] Fix dashboard-tus/app.js --- e2e/clients/dashboard-tus/app.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/clients/dashboard-tus/app.js b/e2e/clients/dashboard-tus/app.js index b4c5a5777a..2704574517 100644 --- a/e2e/clients/dashboard-tus/app.js +++ b/e2e/clients/dashboard-tus/app.js @@ -14,7 +14,7 @@ const uppy = new Uppy() .use(Url, { target: Dashboard, companionUrl }) .use(Unsplash, { target: Dashboard, companionUrl }) -function onShouldRetry (err, next) { +function onShouldRetry (err, retryAttempt, options, next) { if (err?.originalResponse?.getStatus() === 418) { return true } From ca7609822b74550758b9189c5aa78d150a43a4d3 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Tue, 17 May 2022 13:19:18 +0200 Subject: [PATCH 13/16] Update test --- e2e/cypress/integration/dashboard-tus.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/e2e/cypress/integration/dashboard-tus.spec.ts b/e2e/cypress/integration/dashboard-tus.spec.ts index 59ad67d6bc..9657f2b11f 100644 --- a/e2e/cypress/integration/dashboard-tus.spec.ts +++ b/e2e/cypress/integration/dashboard-tus.spec.ts @@ -34,6 +34,7 @@ describe('Dashboard with Tus', () => { { statusCode: 429, body: {} }, ).as('patch') + cy.wait('@patch') cy.wait('@patch') cy.window().then(({ uppy }) => { From dc5b21f952f3d3f101bf9d4d22a654c8bae89a0b Mon Sep 17 00:00:00 2001 From: Murderlon Date: Tue, 17 May 2022 16:10:15 +0200 Subject: [PATCH 14/16] Fix onShouldRetry type --- packages/@uppy/tus/types/index.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@uppy/tus/types/index.d.ts b/packages/@uppy/tus/types/index.d.ts index 063f177e4d..9cb8b6215b 100644 --- a/packages/@uppy/tus/types/index.d.ts +++ b/packages/@uppy/tus/types/index.d.ts @@ -20,7 +20,7 @@ export interface TusOptions extends PluginOptions, TusUploadOptions { limit?: number useFastRemoteRetry?: boolean withCredentials?: boolean - onShouldRetry: (err: Error, next: Next) => boolean + onShouldRetry: (err: Error | undefined, retryAttempt: number, options: TusOptions, next: Next) => boolean } declare class Tus extends BasePlugin {} From 757a49d61e9e74a25966ec87aac643a5dce96c16 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Tue, 17 May 2022 16:11:08 +0200 Subject: [PATCH 15/16] First declare, then use --- packages/@uppy/tus/src/index.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/@uppy/tus/src/index.js b/packages/@uppy/tus/src/index.js index d3aa5aa1d2..0229760882 100644 --- a/packages/@uppy/tus/src/index.js +++ b/packages/@uppy/tus/src/index.js @@ -277,12 +277,6 @@ export default class Tus extends BasePlugin { resolve(upload) } - if (opts.onShouldRetry !== null) { - uploadOptions.onShouldRetry = (...args) => opts.onShouldRetry(...args, defaultOnShouldRetry.bind(this)) - } else { - uploadOptions.onShouldRetry = defaultOnShouldRetry - } - function defaultOnShouldRetry (err) { const status = err?.originalResponse?.getStatus() @@ -323,6 +317,12 @@ export default class Tus extends BasePlugin { return true } + if (opts.onShouldRetry !== null) { + uploadOptions.onShouldRetry = (...args) => opts.onShouldRetry(...args, defaultOnShouldRetry.bind(this)) + } else { + uploadOptions.onShouldRetry = defaultOnShouldRetry + } + const copyProp = (obj, srcProp, destProp) => { if (hasProperty(obj, srcProp) && !hasProperty(obj, destProp)) { // eslint-disable-next-line no-param-reassign From 87dd3db046eadadab2b62f642966a731a81085c6 Mon Sep 17 00:00:00 2001 From: Merlijn Vos Date: Tue, 17 May 2022 16:31:12 +0200 Subject: [PATCH 16/16] Apply suggestions from code review Co-authored-by: Antoine du Hamel --- packages/@uppy/tus/src/index.js | 6 +++--- packages/@uppy/tus/types/index.d.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/@uppy/tus/src/index.js b/packages/@uppy/tus/src/index.js index 0229760882..7e5f35a2cc 100644 --- a/packages/@uppy/tus/src/index.js +++ b/packages/@uppy/tus/src/index.js @@ -277,7 +277,7 @@ export default class Tus extends BasePlugin { resolve(upload) } - function defaultOnShouldRetry (err) { + const defaultOnShouldRetry = (err) => { const status = err?.originalResponse?.getStatus() if (status === 429) { @@ -317,8 +317,8 @@ export default class Tus extends BasePlugin { return true } - if (opts.onShouldRetry !== null) { - uploadOptions.onShouldRetry = (...args) => opts.onShouldRetry(...args, defaultOnShouldRetry.bind(this)) + if (opts.onShouldRetry != null) { + uploadOptions.onShouldRetry = (...args) => opts.onShouldRetry(...args, defaultOnShouldRetry) } else { uploadOptions.onShouldRetry = defaultOnShouldRetry } diff --git a/packages/@uppy/tus/types/index.d.ts b/packages/@uppy/tus/types/index.d.ts index 9cb8b6215b..6753d750c4 100644 --- a/packages/@uppy/tus/types/index.d.ts +++ b/packages/@uppy/tus/types/index.d.ts @@ -13,7 +13,7 @@ type TusUploadOptions = Pick> -type Next = (err: Error) => boolean +type Next = (err: Error | undefined, retryAttempt?: number, options?: TusOptions) => boolean export interface TusOptions extends PluginOptions, TusUploadOptions { metaFields?: string[] | null