From 1dea186a6ee888dcaed12e00a14b5c0f66e60376 Mon Sep 17 00:00:00 2001 From: Felix Rieseberg Date: Sat, 18 Nov 2017 00:50:53 -0800 Subject: [PATCH 1/5] :thinking_face: What if you could return an Error? --- lib/renderer/init.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lib/renderer/init.js b/lib/renderer/init.js index b7fc3877ab..ae694423d3 100644 --- a/lib/renderer/init.js +++ b/lib/renderer/init.js @@ -45,6 +45,17 @@ electron.ipcRenderer.on('ELECTRON_INTERNAL_RENDERER_ASYNC_WEB_FRAME_METHOD', (ev event.sender.send(`ELECTRON_INTERNAL_BROWSER_ASYNC_WEB_FRAME_RESPONSE_${requestId}`, null, resolvedResult) }) .catch((resolvedError) => { + if (resolvedError instanceof Error) { + // Errors get lost, because: JSON.stringify(new Error('Message')) === {} + // Take the serializable properties and construct a generic object + resolvedError = { + message: resolvedError.message, + stack: resolvedError.stack, + name: resolvedError.name, + __ELECTRON_SERIALIZED_ERROR__: true + } + } + event.sender.send(`ELECTRON_INTERNAL_BROWSER_ASYNC_WEB_FRAME_RESPONSE_${requestId}`, resolvedError) }) } From 9a2d103e7f30c8bcb9a23069965a0667cfd4eb49 Mon Sep 17 00:00:00 2001 From: Felix Rieseberg Date: Sat, 18 Nov 2017 00:51:14 -0800 Subject: [PATCH 2/5] :thinking_face: What if we made it an Error again? --- lib/browser/api/web-contents.js | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/browser/api/web-contents.js b/lib/browser/api/web-contents.js index 4e3ec332ae..7485a71958 100644 --- a/lib/browser/api/web-contents.js +++ b/lib/browser/api/web-contents.js @@ -112,6 +112,16 @@ const webFrameMethods = [ ] const webFrameMethodsWithResult = [] +const errorConstructors = { + Error, + EvalError, + RangeError, + ReferenceError, + SyntaxError, + TypeError, + URIError +} + const asyncWebFrameMethods = function (requestId, method, callback, ...args) { return new Promise((resolve, reject) => { this.send('ELECTRON_INTERNAL_RENDERER_ASYNC_WEB_FRAME_METHOD', requestId, method, args) @@ -120,7 +130,18 @@ const asyncWebFrameMethods = function (requestId, method, callback, ...args) { if (typeof callback === 'function') callback(result) resolve(result) } else { - reject(error) + if (error && error.__ELECTRON_SERIALIZED_ERROR__) { + let rehydratedError = error + + if (errorConstructors[error.name]) { + rehydratedError = new errorConstructors[error.name](error.message) + rehydratedError.stack = error.stack + } + + reject(rehydratedError); + } else { + reject(error) + } } }) }) From 3311e0bd67e0bc971b5ab52072c4b27427172d9e Mon Sep 17 00:00:00 2001 From: Felix Rieseberg Date: Sat, 18 Nov 2017 00:51:27 -0800 Subject: [PATCH 3/5] :construction_worker: This calls for tests --- spec/api-browser-window-spec.js | 20 ++++++++++++++++++++ spec/static/main.js | 4 ++++ 2 files changed, 24 insertions(+) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 185b8fba82..c1e2f6cbb7 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -2514,6 +2514,15 @@ describe('BrowserWindow module', () => { const code = `(() => "${expected}")()` const asyncCode = `(() => new Promise(r => setTimeout(() => r("${expected}"), 500)))()` const badAsyncCode = `(() => new Promise((r, e) => setTimeout(() => e("${expectedErrorMsg}"), 500)))()` + const errorCodes = { + Error: 'Promise.reject(new Error("Wamp-wamp"))', + ReferenceError: 'Promise.reject(new ReferenceError("Wamp-wamp"))', + EvalError: 'Promise.reject(new EvalError("Wamp-wamp"))', + RangeError: 'Promise.reject(new RangeError("Wamp-wamp"))', + SyntaxError: 'Promise.reject(new SyntaxError("Wamp-wamp"))', + TypeError: 'Promise.reject(new TypeError("Wamp-wamp"))', + URIError: 'Promise.reject(new URIError("Wamp-wamp"))' + } it('doesnt throw when no calback is provided', () => { const result = ipcRenderer.sendSync('executeJavaScript', code, false) @@ -2561,6 +2570,17 @@ describe('BrowserWindow module', () => { done() }) }) + it('rejects the returned promise with an error if an Error.prototype is thrown', async () => { + for (const errorKey in errorCodes) { + await new Promise((resolve) => { + ipcRenderer.send('executeJavaScript', errorCodes[errorKey], true) + ipcRenderer.once('executeJavaScript-promise-error-name', (event, name) => { + assert.equal(name, errorKey) + resolve() + }) + }) + } + }) it('works after page load and during subframe load', (done) => { w.webContents.once('did-finish-load', () => { // initiate a sub-frame load, then try and execute script during it diff --git a/spec/static/main.js b/spec/static/main.js index faede549a1..4a4dfd109d 100644 --- a/spec/static/main.js +++ b/spec/static/main.js @@ -217,6 +217,10 @@ app.on('ready', function () { window.webContents.send('executeJavaScript-promise-response', result) }).catch((error) => { window.webContents.send('executeJavaScript-promise-error', error) + + if (error && error.name) { + window.webContents.send('executeJavaScript-promise-error-name', error.name) + } }) if (!hasCallback) { From cf7e9df3a0944adb0f3b94270fb717c91f203fd4 Mon Sep 17 00:00:00 2001 From: Felix Rieseberg Date: Sat, 18 Nov 2017 01:02:09 -0800 Subject: [PATCH 4/5] :wrench: Appease the linter --- lib/browser/api/web-contents.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/browser/api/web-contents.js b/lib/browser/api/web-contents.js index 7485a71958..36f43aafc4 100644 --- a/lib/browser/api/web-contents.js +++ b/lib/browser/api/web-contents.js @@ -138,7 +138,7 @@ const asyncWebFrameMethods = function (requestId, method, callback, ...args) { rehydratedError.stack = error.stack } - reject(rehydratedError); + reject(rehydratedError) } else { reject(error) } From 5b18bea7e3742084d1de0500365a5cfe499b2905 Mon Sep 17 00:00:00 2001 From: Felix Rieseberg Date: Sat, 18 Nov 2017 10:59:09 -0800 Subject: [PATCH 5/5] :wrench: Alex has good ideas --- lib/browser/api/web-contents.js | 10 +++------- spec/api-browser-window-spec.js | 24 ++++++++++++------------ 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/lib/browser/api/web-contents.js b/lib/browser/api/web-contents.js index 36f43aafc4..61b37f7733 100644 --- a/lib/browser/api/web-contents.js +++ b/lib/browser/api/web-contents.js @@ -130,13 +130,9 @@ const asyncWebFrameMethods = function (requestId, method, callback, ...args) { if (typeof callback === 'function') callback(result) resolve(result) } else { - if (error && error.__ELECTRON_SERIALIZED_ERROR__) { - let rehydratedError = error - - if (errorConstructors[error.name]) { - rehydratedError = new errorConstructors[error.name](error.message) - rehydratedError.stack = error.stack - } + if (error.__ELECTRON_SERIALIZED_ERROR__ && errorConstructors[error.name]) { + const rehydratedError = new errorConstructors[error.name](error.message) + rehydratedError.stack = error.stack reject(rehydratedError) } else { diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index c1e2f6cbb7..fea4c3a739 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -2514,15 +2514,15 @@ describe('BrowserWindow module', () => { const code = `(() => "${expected}")()` const asyncCode = `(() => new Promise(r => setTimeout(() => r("${expected}"), 500)))()` const badAsyncCode = `(() => new Promise((r, e) => setTimeout(() => e("${expectedErrorMsg}"), 500)))()` - const errorCodes = { - Error: 'Promise.reject(new Error("Wamp-wamp"))', - ReferenceError: 'Promise.reject(new ReferenceError("Wamp-wamp"))', - EvalError: 'Promise.reject(new EvalError("Wamp-wamp"))', - RangeError: 'Promise.reject(new RangeError("Wamp-wamp"))', - SyntaxError: 'Promise.reject(new SyntaxError("Wamp-wamp"))', - TypeError: 'Promise.reject(new TypeError("Wamp-wamp"))', - URIError: 'Promise.reject(new URIError("Wamp-wamp"))' - } + const errorTypes = new Set([ + Error, + ReferenceError, + EvalError, + RangeError, + SyntaxError, + TypeError, + URIError + ]) it('doesnt throw when no calback is provided', () => { const result = ipcRenderer.sendSync('executeJavaScript', code, false) @@ -2571,11 +2571,11 @@ describe('BrowserWindow module', () => { }) }) it('rejects the returned promise with an error if an Error.prototype is thrown', async () => { - for (const errorKey in errorCodes) { + for (const error in errorTypes) { await new Promise((resolve) => { - ipcRenderer.send('executeJavaScript', errorCodes[errorKey], true) + ipcRenderer.send('executeJavaScript', `Promise.reject(new ${error.name}("Wamp-wamp")`, true) ipcRenderer.once('executeJavaScript-promise-error-name', (event, name) => { - assert.equal(name, errorKey) + assert.equal(name, error.name) resolve() }) })