From a0269233a74fbf7a5dfd04a0d32858adc9c4fdfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Wed, 17 Apr 2019 15:54:17 +0200 Subject: [PATCH 1/6] transloadit: add error reporting --- packages/@uppy/transloadit/src/Client.js | 51 +++++++++++++++++++++--- 1 file changed, 45 insertions(+), 6 deletions(-) diff --git a/packages/@uppy/transloadit/src/Client.js b/packages/@uppy/transloadit/src/Client.js index 031abfd0e..fb3dacb8b 100644 --- a/packages/@uppy/transloadit/src/Client.js +++ b/packages/@uppy/transloadit/src/Client.js @@ -4,6 +4,8 @@ module.exports = class Client { constructor (opts = {}) { this.opts = opts + + this._reportError = this._reportError.bind(this) } /** @@ -31,7 +33,8 @@ module.exports = class Client { }) data.append('num_expected_upload_files', expectedFiles) - return fetch(`${this.opts.service}/assemblies`, { + const url = `${this.opts.service}/assemblies` + return fetch(url, { method: 'post', body: data }).then((response) => response.json()).then((assembly) => { @@ -43,7 +46,7 @@ module.exports = class Client { } return assembly - }) + }).catch((err) => this._reportError(err, { url })) } /** @@ -54,8 +57,10 @@ module.exports = class Client { */ reserveFile (assembly, file) { const size = encodeURIComponent(file.size) - return fetch(`${assembly.assembly_ssl_url}/reserve_file?size=${size}`, { method: 'post' }) + const url = `${assembly.assembly_ssl_url}/reserve_file?size=${size}` + return fetch(url, { method: 'post' }) .then((response) => response.json()) + .catch((err) => this._reportError(err, { assembly, file, url })) } /** @@ -69,13 +74,15 @@ module.exports = class Client { return Promise.reject(new Error('File does not have an `uploadURL`.')) } const size = encodeURIComponent(file.size) - const url = encodeURIComponent(file.uploadURL) + const uploadUrl = encodeURIComponent(file.uploadURL) const filename = encodeURIComponent(file.name) const fieldname = 'file' - const qs = `size=${size}&filename=${filename}&fieldname=${fieldname}&s3Url=${url}` - return fetch(`${assembly.assembly_ssl_url}/add_file?${qs}`, { method: 'post' }) + const qs = `size=${size}&filename=${filename}&fieldname=${fieldname}&s3Url=${uploadUrl}` + const url = `${assembly.assembly_ssl_url}/add_file?${qs}` + return fetch(url, { method: 'post' }) .then((response) => response.json()) + .catch((err) => this._reportError(err, { assembly, file, url })) } /** @@ -96,5 +103,37 @@ module.exports = class Client { getAssemblyStatus (url) { return fetch(url) .then((response) => response.json()) + .catch((err) => this._reportError(err, { url })) + } + + submitError (err, { endpoint, instance, assembly }) { + return fetch('https://status.transloadit.com/client_error', { + method: 'post', + body: JSON.stringify({ + endpoint, + instance, + assembly_id: assembly, + ip: null, // can't this just be done by statuspage? + agent: typeof navigator !== 'undefined' ? navigator.userAgent : '', + error: err.message + }) + }).then((response) => response.json()) + } + + _reportError (err, params = {}) { + const opts = {} + if (params.assembly) { + opts.assembly = params.assembly.assembly_id + opts.instance = params.assembly.instance + } + if (params.url) { + opts.endpoint = params.url + } + + this.submitError(err, opts).catch((_) => { + // not much we can do then is there + }) + + throw err } } From 9448b91238a22eac5884fb2b9d0478042f9928ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Wed, 17 Apr 2019 15:58:54 +0200 Subject: [PATCH 2/6] transloadit: report tus errors as well --- packages/@uppy/transloadit/src/index.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/packages/@uppy/transloadit/src/index.js b/packages/@uppy/transloadit/src/index.js index 10b15f4ba..aba9ff61b 100644 --- a/packages/@uppy/transloadit/src/index.js +++ b/packages/@uppy/transloadit/src/index.js @@ -64,6 +64,7 @@ module.exports = class Transloadit extends Plugin { this._prepareUpload = this._prepareUpload.bind(this) this._afterUpload = this._afterUpload.bind(this) this._onError = this._onError.bind(this) + this._onTusError = this._onTusError.bind(this) this._onCancelAll = this._onCancelAll.bind(this) this._onFileUploadURLAvailable = this._onFileUploadURLAvailable.bind(this) this._onRestored = this._onRestored.bind(this) @@ -668,6 +669,17 @@ module.exports = class Transloadit extends Plugin { }) } + _onTusError (err) { + if (err && /^tus: /.test(err.message)) { + const url = err.originalRequest && err.originalRequest.responseURL + ? err.originalRequest.responseURL + : null + this.client.submitError(err, { url }).then((_) => { + // if we can't report the error that sucks + }) + } + } + install () { this.uppy.addPreProcessor(this._prepareUpload) this.uppy.addPostProcessor(this._afterUpload) @@ -678,6 +690,9 @@ module.exports = class Transloadit extends Plugin { // Handle cancellation. this.uppy.on('cancel-all', this._onCancelAll) + // For error reporting. + this.uppy.on('upload-error', this._onTusError) + if (this.opts.importFromUploadURLs) { // No uploader needed when importing; instead we take the upload URL from an existing uploader. this.uppy.on('upload-success', this._onFileUploadURLAvailable) From de0a1d546b93a618c424ef7a5b1c097e761f4607 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Wed, 17 Apr 2019 16:03:15 +0200 Subject: [PATCH 3/6] transloadit: add opt out of error reporting --- packages/@uppy/transloadit/src/Client.js | 4 ++++ packages/@uppy/transloadit/src/index.js | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/@uppy/transloadit/src/Client.js b/packages/@uppy/transloadit/src/Client.js index fb3dacb8b..33b5acdc9 100644 --- a/packages/@uppy/transloadit/src/Client.js +++ b/packages/@uppy/transloadit/src/Client.js @@ -121,6 +121,10 @@ module.exports = class Client { } _reportError (err, params = {}) { + if (this.opts.errorReporting === false) { + throw err + } + const opts = {} if (params.assembly) { opts.assembly = params.assembly.assembly_id diff --git a/packages/@uppy/transloadit/src/index.js b/packages/@uppy/transloadit/src/index.js index aba9ff61b..4f0c61c25 100644 --- a/packages/@uppy/transloadit/src/index.js +++ b/packages/@uppy/transloadit/src/index.js @@ -41,6 +41,7 @@ module.exports = class Transloadit extends Plugin { const defaultOptions = { service: 'https://api2.transloadit.com', + errorReporting: true, waitForEncoding: false, waitForMetadata: false, alwaysRunAssembly: false, @@ -80,7 +81,8 @@ module.exports = class Transloadit extends Plugin { } this.client = new Client({ - service: this.opts.service + service: this.opts.service, + errorReporting: this.opts.errorReporting }) // Contains Assembly instances for in-progress Assemblies. this.activeAssemblies = {} From 5f38ece7ec2eede493bc9e2d54a1327be9a0910e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Wed, 24 Apr 2019 15:27:27 +0200 Subject: [PATCH 4/6] transloadit: remove ip, report full error message --- packages/@uppy/transloadit/src/Client.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/@uppy/transloadit/src/Client.js b/packages/@uppy/transloadit/src/Client.js index 33b5acdc9..7f6227f51 100644 --- a/packages/@uppy/transloadit/src/Client.js +++ b/packages/@uppy/transloadit/src/Client.js @@ -107,15 +107,18 @@ module.exports = class Client { } submitError (err, { endpoint, instance, assembly }) { + const message = err.details + ? `${err.message} (${err.details})` + : err.message + return fetch('https://status.transloadit.com/client_error', { method: 'post', body: JSON.stringify({ endpoint, instance, assembly_id: assembly, - ip: null, // can't this just be done by statuspage? agent: typeof navigator !== 'undefined' ? navigator.userAgent : '', - error: err.message + error: message }) }).then((response) => response.json()) } From 9644df7eeeb8084bb3a3e1cd9e73d60df37d64fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Wed, 24 Apr 2019 16:53:31 +0200 Subject: [PATCH 5/6] transloadit: add `type: TUS_ERROR|API_ERROR|STATUS_ERROR` to reports --- packages/@uppy/transloadit/src/Client.js | 18 +++++++++++------- packages/@uppy/transloadit/src/index.js | 2 +- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/@uppy/transloadit/src/Client.js b/packages/@uppy/transloadit/src/Client.js index 7f6227f51..520ace9a5 100644 --- a/packages/@uppy/transloadit/src/Client.js +++ b/packages/@uppy/transloadit/src/Client.js @@ -46,7 +46,7 @@ module.exports = class Client { } return assembly - }).catch((err) => this._reportError(err, { url })) + }).catch((err) => this._reportError(err, { url, type: 'API_ERROR' })) } /** @@ -60,7 +60,7 @@ module.exports = class Client { const url = `${assembly.assembly_ssl_url}/reserve_file?size=${size}` return fetch(url, { method: 'post' }) .then((response) => response.json()) - .catch((err) => this._reportError(err, { assembly, file, url })) + .catch((err) => this._reportError(err, { assembly, file, url, type: 'API_ERROR' })) } /** @@ -82,7 +82,7 @@ module.exports = class Client { const url = `${assembly.assembly_ssl_url}/add_file?${qs}` return fetch(url, { method: 'post' }) .then((response) => response.json()) - .catch((err) => this._reportError(err, { assembly, file, url })) + .catch((err) => this._reportError(err, { assembly, file, url, type: 'API_ERROR' })) } /** @@ -91,8 +91,10 @@ module.exports = class Client { * @param {object} assembly */ cancelAssembly (assembly) { - return fetch(assembly.assembly_ssl_url, { method: 'delete' }) + const url = assembly.assembly_ssl_url + return fetch(url, { method: 'delete' }) .then((response) => response.json()) + .catch((err) => this._reportError(err, { url, type: 'API_ERROR' })) } /** @@ -103,7 +105,7 @@ module.exports = class Client { getAssemblyStatus (url) { return fetch(url) .then((response) => response.json()) - .catch((err) => this._reportError(err, { url })) + .catch((err) => this._reportError(err, { url, type: 'STATUS_ERROR' })) } submitError (err, { endpoint, instance, assembly }) { @@ -123,12 +125,14 @@ module.exports = class Client { }).then((response) => response.json()) } - _reportError (err, params = {}) { + _reportError (err, params) { if (this.opts.errorReporting === false) { throw err } - const opts = {} + const opts = { + type: params.type + } if (params.assembly) { opts.assembly = params.assembly.assembly_id opts.instance = params.assembly.instance diff --git a/packages/@uppy/transloadit/src/index.js b/packages/@uppy/transloadit/src/index.js index 4f0c61c25..12ca6b6b8 100644 --- a/packages/@uppy/transloadit/src/index.js +++ b/packages/@uppy/transloadit/src/index.js @@ -676,7 +676,7 @@ module.exports = class Transloadit extends Plugin { const url = err.originalRequest && err.originalRequest.responseURL ? err.originalRequest.responseURL : null - this.client.submitError(err, { url }).then((_) => { + this.client.submitError(err, { url, type: 'TUS_ERROR' }).then((_) => { // if we can't report the error that sucks }) } From 05113c03a5f6ecd5abc49bfe4293609c6958f990 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Wed, 24 Apr 2019 16:54:40 +0200 Subject: [PATCH 6/6] changelog: transloadit-client note --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 02a0c0cf3..a72906131 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -101,6 +101,7 @@ PRs are welcome! Please do open an issue to discuss first if it's a big feature, - [ ] !!! dashboard: Remove the Authorization required tooltip on the authentication screen https://github.com/transloadit/uppy/issues/1425 - [ ] @uppy/companion: investigate 423 and 500 issues with React Native + Url plugin when pause/resuming an upload - [ ] docs: add docs on locales — how to use from NPM and CDN +- [ ] @uppy/transloadit: finish Transloadit-Client header on https://github.com/transloadit/uppy/tree/feature/transloadit-client ## 1.0 Goals