Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FR: Better error reporting for webContents->executeJavaScript #9102

Closed
aight8 opened this issue Apr 3, 2017 · 9 comments · Fixed by #18635
Closed

FR: Better error reporting for webContents->executeJavaScript #9102

aight8 opened this issue Apr 3, 2017 · 9 comments · Fixed by #18635

Comments

@aight8
Copy link

aight8 commented Apr 3, 2017

  • Electron version: 1.6.5
  • Operating system: OSX

Expected behavior

Possible solutions

  • Set a more useful filename in the throwed error on the window object for identify where it happened (usefull)

  • The result/promise is never resolved now. Maybe it should get a callback with error argument and reject the promise exactly in this case?

Documentation:

The documentation requires a better overview where/how to catch errors. Additionally some other information to understand the working of the different contexts.

Something like:

Location Unhandled errors Available modules*** Exposed variables****
preload.js ? ? ?
executeJavaScript()* ? ? ?
page scripts 5) window error handler** ? ?

*) webContents.executeJavaScript function
**) window.addEventListener('error', function(errorEvent) {}) or window.onerror = function(...) {}
***) which modules can be imported by using the require global function
****) In the global context (can be the window object)
5) all scripts which is executed on the page or it resources

Web preferences which alter things:

Web preference default How it alter things
sandbox false describe here...
contextIsolation false describe here...
nodeIntegration true describe here...
javascript false if false then: all javascript execution is prevented
webSecurity false if false then: describe here...

Actual behavior

If you use executeJavaScript it's difficult to catch the error which occurs in the passed javascript.

In renderer

window.addEventListener('error', ...) is catching the error but the it's limited to the following:

column:   0
error:    null
filename: 
line:     0
message:  Script error

This changes if I set it to webSecurity to false then I get:

column:   7
error: 
filename: 
line:     1
message:  Uncaught Error: this is my exception

So the problem is now, webSecurity is disabled, I think this is the best thing to do. And the filename is even now empty, I even now have no clue where it happened.

Trough the debugger

I attached and registered the debugger with the domains: Debugger.enable and Log.enable

Log.entryAdded is never received for errors in executeJavaScript code.

How to reproduce

use the options above then:

webContents.executeJavaScript('throw new Error("this is my exception")')

Same for syntax error or others...

In all examples the start options are (+ changes are described):
Start options: webSecurity: true, nodeIntegration: false, sandbox: false
Yes, this is almost the default config except nodeIntegration.

This is not related to the new sandbox function, it's false - in sandbox mode it's broken entirely: #9073

@MarshallOfSound
Copy link
Member

Lot's to read here and not much time, but a quick solution to your error handling issue is to simply make the return type of your executeJavaScript call a Promise.

webContents.executeJavaScript('new Promise(() => { throw new Error("this is my exception") })').catch(err => console.warn(err))

@aight8
Copy link
Author

aight8 commented Apr 5, 2017

I will this. However if the code snippet contains syntax errors it's the same problem. I try to catch all kind of errors can happen

@aight8
Copy link
Author

aight8 commented Apr 13, 2017

more
I write another use case here, I don't create a new issue for this - I summarise it:
If the promise which is returned in executeJavaScript throw any error or call reject (intended or not) something very bad thing happens:
The promise reject value is serialised to send it back to the main process via IPC. However as we know, an error error cannot be serialised without extra work. So currently the returned promise from executeJavaScript is rejected with an empty object.

Error: Promise rejected with value: {} // <- totally useless. windows like behaviour.

That's really annoying - because:

  • Normally reject values should always be an Error object
  • Any not intended error are mostly Error objects (I hope so) - so they automatically land into this problem.

How to solve?
The IPC serialiser could serialise error objects specially. (name, message, stack + other values)

Temporary solution
A temporary solution were to wrap the whole promise function body in a try catch, and if an error is thrown, catch it and re-throw a simple Object with the interface:

interface ErrorShape {
   name?: string
   message?: string
   stack?: string
   // ...
}

snippet:

try {
const result await = webContents.executeJavaScript(`
new Promise((resolve, reject) => {
   try {
      // do stuff...
   } catch(err) {
      throw { name: err.name, message: err.message, stack: err.stack }
   }
})
`)
} catch(errObj) {
    // errObj is not an error object but it has the same fields like the basic error object
}

@aight8
Copy link
Author

aight8 commented Apr 19, 2017

bump

@MarshallOfSound
Copy link
Member

@aight8 Please don't "bump" issues, it does nothing other than send annoying emails to people watching the repo 👍

@bughit
Copy link
Contributor

bughit commented Nov 17, 2017

a promise based API should either resolve or reject, which corresponds to either moving to the next statement or throwing an exception in sync code.

try {
  const result = await promiseApi();
  // either you end up here
} catch (e) {
 // or here, there is no valid third option
}

So shouldn't failing without rejecting be considered a bug?

@felixrieseberg
Copy link
Member

I feel like I fixed this with the error serialization (#11158). Is that good enough to close this issue?

@bughit
Copy link
Contributor

bughit commented Jan 10, 2018

#11158 takes care of errors in promise wrapped code, right?

If the syntax error is not in wrapped code executeJavaScript('invalid js') it will never reject (or resolve), which is not proper behavior of a promise based api.

@FibreFoX
Copy link

FibreFoX commented Dec 12, 2018

I'm hitting the same issuue as @bughit describes (with latest 3.0.x electron) ....why is there no refused Promise being thrown?! Took several days to debug this promise not working as it should.

ruleechen added a commit to ruleechen/electron-connector that referenced this issue Mar 12, 2019
MarshallOfSound added a commit that referenced this issue Jun 5, 2019
…e script (#18635)

* fix: reject the executeJavaScript promise when it fails to execute the script

Closes #9102

* Update atom/renderer/api/atom_api_web_frame.cc

Co-Authored-By: Jeremy Apthorp <nornagon@nornagon.net>

* Update atom/renderer/api/atom_api_web_frame.cc

Co-Authored-By: Jeremy Apthorp <nornagon@nornagon.net>

* fix: missing semicolon
trop bot pushed a commit that referenced this issue Jun 10, 2019
MarshallOfSound pushed a commit that referenced this issue Jun 13, 2019
…e script (#18714)

* fix: reject the executeJavaScript promise when it fails to execute the script

Closes #9102

* Update atom/renderer/api/atom_api_web_frame.cc

Co-Authored-By: Jeremy Apthorp <nornagon@nornagon.net>

* Update atom/renderer/api/atom_api_web_frame.cc

Co-Authored-By: Jeremy Apthorp <nornagon@nornagon.net>

* fix: missing semicolon
jmercouris added a commit to atlas-engineer/cl-electron that referenced this issue Mar 5, 2024
Unfortunately we cannot get more information from Electron with
regards to the nature of the error. In fact, the error that it reports
instructs the user to check the JavaScript console within the renderer
for more details.

You can see more information here:

electron/electron#18635
electron/electron#9102
jmercouris added a commit to atlas-engineer/cl-electron that referenced this issue Mar 6, 2024
Unfortunately we cannot get more information from Electron with
regards to the nature of the error. In fact, the error that it reports
instructs the user to check the JavaScript console within the renderer
for more details.

You can see more information here:

electron/electron#18635
electron/electron#9102
aadcg pushed a commit to atlas-engineer/cl-electron that referenced this issue Mar 7, 2024
Unfortunately we cannot get more information from Electron with
regards to the nature of the error. In fact, the error that it reports
instructs the user to check the JavaScript console within the renderer
for more details.

You can see more information here:

electron/electron#18635
electron/electron#9102
aadcg added a commit to atlas-engineer/cl-electron that referenced this issue Mar 7, 2024
Unfortunately we cannot get more information from Electron with regards to the
nature of the error. In fact, the error that it reports instructs the user to
check the JavaScript console within the renderer for more details.

For more information, see:

electron/electron#18635
electron/electron#9102

Fixes #30.
aadcg pushed a commit to atlas-engineer/cl-electron that referenced this issue Mar 7, 2024
Unfortunately we cannot get more information from Electron with regards to the
nature of the error. In fact, the error that it reports instructs the user to
check the JavaScript console within the renderer for more details.

For more information, see:

electron/electron#18635
electron/electron#9102

Fixes #30.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants