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

Log4js error logging when child process exits #1038

Closed
simondel opened this issue Jul 20, 2018 · 1 comment
Closed

Log4js error logging when child process exits #1038

simondel opened this issue Jul 20, 2018 · 1 comment
Assignees

Comments

@simondel
Copy link
Member

When a child process exits unexpectedly, Log4js logs the following error:

13:35:52 (31080) ERROR log4js A worker log process hung up unexpectedly { Error: read ECONNRESET
    at TCP.onread (net.js:656:25) errno: 'ECONNRESET', code: 'ECONNRESET', syscall: 'read' }
@simondel
Copy link
Member Author

Note: this is on Windows

//cc @nicojs

@ghost ghost assigned nicojs Jul 22, 2018
@ghost ghost added the 🔎 Needs review label Jul 22, 2018
simondel pushed a commit that referenced this issue Aug 2, 2018
Make all test runner and transpiler child processes silent. The standard out and standard error (stdout and stderr) are now only visible when `loglevel: 'trace'`. If a child process crashes, the last 10 messages received are logged as warning. 

This is also a refactoring of the way we spawn child processes. Instead of having 2 similar implementations (one for transpiler and one for test runners), they are both consolidated in one coherent `ChildProcessProxy` abstraction. 

Also clean up the test runner decorator pattern. Timeouts and retries are now implemented only once. Recognizing that the child process crashed is done by validating that the error is an instance of `ChildProcessCrashedError`. No process specifics other than the name of the error is known from the outside.

The `Task` class is also refactored. Instead of relying on a custom implementation, it uses the `Promise.race` method for timeout functionality.

Fixes #1038 #976
@ghost ghost removed the 🔎 Needs review label Aug 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants