Emit an error when the electron process crashes or prints to stderr #43
Conversation
|
Thanks, this will make the module a lot less frustrating to get working on new machines! I looked into doing something similar at one point, but I found that there was sometimes stderr output from Electron that shouldn't trigger errors (warnings about the graphics, etc.). Are these warnings gone now? (We did upgrade to Electron v1, maybe it doesn't have warnings). Also, I think there is an issue with the tests now, it seems to fail on Travis. |
paulkernfeld
force-pushed
the
master
branch
2 times, most recently
from
cb4805f
to
e355600
Compare
|
Good point. I don't personally get those warnings, but that's important. I redid this so that it only throws the error if the exit code is nonzero. |
Add a unit test Allow the user to specify a path to the electron executable
|
All right, looks like the tests are still failing. I think that the latest version of this PR may actually have revealed an existing problem, rather than causing a new problem. Have you ever seen something like this? According to StackOverflow, it seems like xvfb needs to be run differently on Travis. |
Use a fork of headless that supports sending arbitrary arguments
|
Hmmm... not sure how to get this working. Have you ever seen this RANDR thing before? |
| @@ -79,7 +79,8 @@ class Daemon extends EventEmitter { | |||
| if (headless == null) { | |||
| return cb(new Error('Could not load "headless" module')) | |||
| } | |||
| headless((err, child, display) => { | |||
| var opts = {args: ['+extension', 'RANDR']} | |||
There was a problem hiding this comment.
This thread shows that we should be doing -extension to remove the RANDR extension: http://stackoverflow.com/questions/17944234/xlib-extension-randr-missing-on-display-21-trying-to-run-headless-googl
There was a problem hiding this comment.
I tried that as well, but I got the same error.
|
Maybe we can safely ignore the RANDR error? Is it just a warning or is it actually preventing electron from running? |
|
These changes were merged. 👍 |
|
Sorry I disappeared, and thanks for merging and fixing this! |
Add a unit test
Allow the user to specify a path to the electron executable