From 5273c46a06f58a8e40e8fed0c57c7c2988635fa1 Mon Sep 17 00:00:00 2001 From: Artur Paikin Date: Wed, 27 May 2020 09:35:21 +0200 Subject: [PATCH 1/4] =?UTF-8?q?Add=20NetworkError=20to=20Transloadit=20plu?= =?UTF-8?q?gin,=20including=20socket.io=E2=80=99s=20connect=5Ffailed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/@uppy/transloadit/src/Assembly.js | 14 +++++ packages/@uppy/transloadit/src/Client.js | 71 ++++++++++++++++++---- 2 files changed, 73 insertions(+), 12 deletions(-) diff --git a/packages/@uppy/transloadit/src/Assembly.js b/packages/@uppy/transloadit/src/Assembly.js index c97c22222..0551e56d0 100644 --- a/packages/@uppy/transloadit/src/Assembly.js +++ b/packages/@uppy/transloadit/src/Assembly.js @@ -2,6 +2,7 @@ const io = requireSocketIo const Emitter = require('component-emitter') const has = require('@uppy/utils/lib/hasProperty') const parseUrl = require('./parseUrl') +const NetworkError = require('@uppy/utils/lib/NetworkError') // Lazy load socket.io to avoid a console error // in IE 10 when the Transloadit plugin is not used. @@ -78,6 +79,12 @@ class TransloaditAssembly extends Emitter { this.emit('connect') }) + + socket.on('connect_failed', () => { + this._onError(new NetworkError('Transloadit Socket.io connection error')) + this.socket = null + }) + socket.on('error', () => { socket.disconnect() this.socket = null @@ -144,6 +151,13 @@ class TransloaditAssembly extends Emitter { */ _fetchStatus ({ diff = true } = {}) { return fetch(this.status.assembly_ssl_url) + .catch((err) => { + if (err.name === 'AbortError') { + throw err + } else { + throw new NetworkError(err) + } + }) .then((response) => response.json()) .then((status) => { // Avoid updating if we closed during this request's lifetime. diff --git a/packages/@uppy/transloadit/src/Client.js b/packages/@uppy/transloadit/src/Client.js index 506934d17..0471b4dbc 100644 --- a/packages/@uppy/transloadit/src/Client.js +++ b/packages/@uppy/transloadit/src/Client.js @@ -1,3 +1,5 @@ +const NetworkError = require('@uppy/utils/lib/NetworkError') + /** * A Barebones HTTP API client for Transloadit. */ @@ -42,19 +44,28 @@ module.exports = class Client { method: 'post', headers: this._headers, body: data - }).then((response) => response.json()).then((assembly) => { - if (assembly.error) { - const error = new Error(assembly.error) - error.details = assembly.message - error.assembly = assembly - if (assembly.assembly_id) { - error.details += ' ' + `Assembly ID: ${assembly.assembly_id}` + }) + .catch((err) => { + if (err.name === 'AbortError') { + throw err + } else { + throw new NetworkError(err) + } + }) + .then((response) => response.json()).then((assembly) => { + if (assembly.error) { + const error = new Error(assembly.error) + error.details = assembly.message + error.assembly = assembly + if (assembly.assembly_id) { + error.details += ' ' + `Assembly ID: ${assembly.assembly_id}` + } + throw error } - throw error - } - return assembly - }).catch((err) => this._reportError(err, { url, type: 'API_ERROR' })) + return assembly + }) + .catch((err) => this._reportError(err, { url, type: 'API_ERROR' })) } /** @@ -67,6 +78,13 @@ module.exports = class Client { const size = encodeURIComponent(file.size) const url = `${assembly.assembly_ssl_url}/reserve_file?size=${size}` return fetch(url, { method: 'post', headers: this._headers }) + .catch((err) => { + if (err.name === 'AbortError') { + throw err + } else { + throw new NetworkError(err) + } + }) .then((response) => response.json()) .catch((err) => this._reportError(err, { assembly, file, url, type: 'API_ERROR' })) } @@ -89,6 +107,13 @@ module.exports = class Client { const qs = `size=${size}&filename=${filename}&fieldname=${fieldname}&s3Url=${uploadUrl}` const url = `${assembly.assembly_ssl_url}/add_file?${qs}` return fetch(url, { method: 'post', headers: this._headers }) + .catch((err) => { + if (err.name === 'AbortError') { + throw err + } else { + throw new NetworkError(err) + } + }) .then((response) => response.json()) .catch((err) => this._reportError(err, { assembly, file, url, type: 'API_ERROR' })) } @@ -101,6 +126,13 @@ module.exports = class Client { cancelAssembly (assembly) { const url = assembly.assembly_ssl_url return fetch(url, { method: 'delete', headers: this._headers }) + .catch((err) => { + if (err.name === 'AbortError') { + throw err + } else { + throw new NetworkError(err) + } + }) .then((response) => response.json()) .catch((err) => this._reportError(err, { url, type: 'API_ERROR' })) } @@ -112,6 +144,13 @@ module.exports = class Client { */ getAssemblyStatus (url) { return fetch(url, { headers: this._headers }) + .catch((err) => { + if (err.name === 'AbortError') { + throw err + } else { + throw new NetworkError(err) + } + }) .then((response) => response.json()) .catch((err) => this._reportError(err, { url, type: 'STATUS_ERROR' })) } @@ -131,7 +170,15 @@ module.exports = class Client { client: this.opts.client, error: message }) - }).then((response) => response.json()) + }) + .catch((err) => { + if (err.name === 'AbortError') { + throw err + } else { + throw new NetworkError(err) + } + }) + .then((response) => response.json()) } _reportError (err, params) { From ef5977643a8cd1b751f2c61ac4d8e4a35035cd30 Mon Sep 17 00:00:00 2001 From: Artur Paikin Date: Fri, 12 Jun 2020 15:27:00 +0100 Subject: [PATCH 2/4] Refactor to use fetchWithNetworkError --- .../companion-client/src/RequestClient.js | 29 ++-------- packages/@uppy/transloadit/src/Assembly.js | 10 +--- packages/@uppy/transloadit/src/Client.js | 56 +++---------------- .../@uppy/utils/src/fetchWithNetworkError.js | 15 +++++ 4 files changed, 28 insertions(+), 82 deletions(-) create mode 100644 packages/@uppy/utils/src/fetchWithNetworkError.js diff --git a/packages/@uppy/companion-client/src/RequestClient.js b/packages/@uppy/companion-client/src/RequestClient.js index be6b19219..4337ddf04 100644 --- a/packages/@uppy/companion-client/src/RequestClient.js +++ b/packages/@uppy/companion-client/src/RequestClient.js @@ -1,7 +1,7 @@ 'use strict' const AuthError = require('./AuthError') -const NetworkError = require('@uppy/utils/lib/NetworkError') +const fetchWithNetworkError = require('@uppy/utils/lib/fetchWithNetworkError') // Remove the trailing slash so we can always safely append /xyz. function stripSlash (url) { @@ -134,18 +134,11 @@ module.exports = class RequestClient { get (path, skipPostResponse) { return new Promise((resolve, reject) => { this.preflightAndHeaders(path).then((headers) => { - fetch(this._getUrl(path), { + fetchWithNetworkError(this._getUrl(path), { method: 'get', headers: headers, credentials: 'same-origin' }) - .catch((err) => { - if (err.name === 'AbortError') { - throw err - } else { - throw new NetworkError(err) - } - }) .then(this._getPostResponseFunc(skipPostResponse)) .then((res) => this._json(res).then(resolve)) .catch((err) => { @@ -159,19 +152,12 @@ module.exports = class RequestClient { post (path, data, skipPostResponse) { return new Promise((resolve, reject) => { this.preflightAndHeaders(path).then((headers) => { - fetch(this._getUrl(path), { + fetchWithNetworkError(this._getUrl(path), { method: 'post', headers: headers, credentials: 'same-origin', body: JSON.stringify(data) }) - .catch((err) => { - if (err.name === 'AbortError') { - throw err - } else { - throw new NetworkError(err) - } - }) .then(this._getPostResponseFunc(skipPostResponse)) .then((res) => this._json(res).then(resolve)) .catch((err) => { @@ -185,19 +171,12 @@ module.exports = class RequestClient { delete (path, data, skipPostResponse) { return new Promise((resolve, reject) => { this.preflightAndHeaders(path).then((headers) => { - fetch(`${this.hostname}/${path}`, { + fetchWithNetworkError(`${this.hostname}/${path}`, { method: 'delete', headers: headers, credentials: 'same-origin', body: data ? JSON.stringify(data) : null }) - .catch((err) => { - if (err.name === 'AbortError') { - throw err - } else { - throw new NetworkError(err) - } - }) .then(this._getPostResponseFunc(skipPostResponse)) .then((res) => this._json(res).then(resolve)) .catch((err) => { diff --git a/packages/@uppy/transloadit/src/Assembly.js b/packages/@uppy/transloadit/src/Assembly.js index 0551e56d0..e32e59244 100644 --- a/packages/@uppy/transloadit/src/Assembly.js +++ b/packages/@uppy/transloadit/src/Assembly.js @@ -3,6 +3,7 @@ const Emitter = require('component-emitter') const has = require('@uppy/utils/lib/hasProperty') const parseUrl = require('./parseUrl') const NetworkError = require('@uppy/utils/lib/NetworkError') +const fetchWithNetworkError = require('@uppy/utils/lib/fetchWithNetworkError') // Lazy load socket.io to avoid a console error // in IE 10 when the Transloadit plugin is not used. @@ -150,14 +151,7 @@ class TransloaditAssembly extends Emitter { * 'status'. */ _fetchStatus ({ diff = true } = {}) { - return fetch(this.status.assembly_ssl_url) - .catch((err) => { - if (err.name === 'AbortError') { - throw err - } else { - throw new NetworkError(err) - } - }) + return fetchWithNetworkError(this.status.assembly_ssl_url) .then((response) => response.json()) .then((status) => { // Avoid updating if we closed during this request's lifetime. diff --git a/packages/@uppy/transloadit/src/Client.js b/packages/@uppy/transloadit/src/Client.js index 0471b4dbc..8b2124f49 100644 --- a/packages/@uppy/transloadit/src/Client.js +++ b/packages/@uppy/transloadit/src/Client.js @@ -1,4 +1,4 @@ -const NetworkError = require('@uppy/utils/lib/NetworkError') +const fetchWithNetworkError = require('@uppy/utils/lib/fetchWithNetworkError') /** * A Barebones HTTP API client for Transloadit. @@ -40,18 +40,11 @@ module.exports = class Client { data.append('num_expected_upload_files', expectedFiles) const url = `${this.opts.service}/assemblies` - return fetch(url, { + return fetchWithNetworkError(url, { method: 'post', headers: this._headers, body: data }) - .catch((err) => { - if (err.name === 'AbortError') { - throw err - } else { - throw new NetworkError(err) - } - }) .then((response) => response.json()).then((assembly) => { if (assembly.error) { const error = new Error(assembly.error) @@ -77,14 +70,7 @@ module.exports = class Client { reserveFile (assembly, file) { const size = encodeURIComponent(file.size) const url = `${assembly.assembly_ssl_url}/reserve_file?size=${size}` - return fetch(url, { method: 'post', headers: this._headers }) - .catch((err) => { - if (err.name === 'AbortError') { - throw err - } else { - throw new NetworkError(err) - } - }) + return fetchWithNetworkError(url, { method: 'post', headers: this._headers }) .then((response) => response.json()) .catch((err) => this._reportError(err, { assembly, file, url, type: 'API_ERROR' })) } @@ -106,14 +92,7 @@ module.exports = class Client { const qs = `size=${size}&filename=${filename}&fieldname=${fieldname}&s3Url=${uploadUrl}` const url = `${assembly.assembly_ssl_url}/add_file?${qs}` - return fetch(url, { method: 'post', headers: this._headers }) - .catch((err) => { - if (err.name === 'AbortError') { - throw err - } else { - throw new NetworkError(err) - } - }) + return fetchWithNetworkError(url, { method: 'post', headers: this._headers }) .then((response) => response.json()) .catch((err) => this._reportError(err, { assembly, file, url, type: 'API_ERROR' })) } @@ -125,14 +104,7 @@ module.exports = class Client { */ cancelAssembly (assembly) { const url = assembly.assembly_ssl_url - return fetch(url, { method: 'delete', headers: this._headers }) - .catch((err) => { - if (err.name === 'AbortError') { - throw err - } else { - throw new NetworkError(err) - } - }) + return fetchWithNetworkError(url, { method: 'delete', headers: this._headers }) .then((response) => response.json()) .catch((err) => this._reportError(err, { url, type: 'API_ERROR' })) } @@ -143,14 +115,7 @@ module.exports = class Client { * @param {string} url The status endpoint of the assembly. */ getAssemblyStatus (url) { - return fetch(url, { headers: this._headers }) - .catch((err) => { - if (err.name === 'AbortError') { - throw err - } else { - throw new NetworkError(err) - } - }) + return fetchWithNetworkError(url, { headers: this._headers }) .then((response) => response.json()) .catch((err) => this._reportError(err, { url, type: 'STATUS_ERROR' })) } @@ -160,7 +125,7 @@ module.exports = class Client { ? `${err.message} (${err.details})` : err.message - return fetch('https://status.transloadit.com/client_error', { + return fetchWithNetworkError('https://status.transloadit.com/client_error', { method: 'post', body: JSON.stringify({ endpoint, @@ -171,13 +136,6 @@ module.exports = class Client { error: message }) }) - .catch((err) => { - if (err.name === 'AbortError') { - throw err - } else { - throw new NetworkError(err) - } - }) .then((response) => response.json()) } diff --git a/packages/@uppy/utils/src/fetchWithNetworkError.js b/packages/@uppy/utils/src/fetchWithNetworkError.js new file mode 100644 index 000000000..0c204c9d6 --- /dev/null +++ b/packages/@uppy/utils/src/fetchWithNetworkError.js @@ -0,0 +1,15 @@ +const NetworkError = require('@uppy/utils/lib/NetworkError') + +/** + * Wrapper around window.fetch that throws a NetworkError when appropriate + */ +module.exports = function fetchWithNetworkError (...options) { + return fetch(...options) + .catch((err) => { + if (err.name === 'AbortError') { + throw err + } else { + throw new NetworkError(err) + } + }) +} From bb98e5cb1e8d736a4b4a57a367fc70ae9ee036a7 Mon Sep 17 00:00:00 2001 From: Artur Paikin Date: Sat, 13 Jun 2020 01:36:54 +0100 Subject: [PATCH 3/4] update docs about error.isNetworkError --- website/src/docs/uppy.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/website/src/docs/uppy.md b/website/src/docs/uppy.md index c105a79e1..89a176b75 100644 --- a/website/src/docs/uppy.md +++ b/website/src/docs/uppy.md @@ -753,6 +753,18 @@ uppy.on('upload-error', (file, error, response) => { }) ``` +If the error is related to network conditions — endpoint unreachable due to firewall or ISP blockage, for instance — the error will have `error.isNetworkError` property set to `true`. Here’s how you can check for network errors: + +``` javascript +uppy.on('upload-error', (file, error, response) => { + if (error.isNetworkError) { + // Let your users know that file upload could have failed + // due to firewall or ISP issues + alertUserAboutPossibleFirewallOrISPIssues(error) + } +}) +``` + ### `upload-retry` Fired when an upload has been retried (after an error, for example): From 948a17d86fa7773ac14f82489b820eac2d5bf4ab Mon Sep 17 00:00:00 2001 From: Artur Paikin Date: Sat, 13 Jun 2020 02:29:19 +0100 Subject: [PATCH 4/4] add catch and error logging --- packages/@uppy/transloadit/src/Assembly.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/@uppy/transloadit/src/Assembly.js b/packages/@uppy/transloadit/src/Assembly.js index e32e59244..809564047 100644 --- a/packages/@uppy/transloadit/src/Assembly.js +++ b/packages/@uppy/transloadit/src/Assembly.js @@ -164,6 +164,7 @@ class TransloaditAssembly extends Emitter { this.status = status } }) + .catch((err) => this._onError(err)) } update () {