chore(build): properly handle worker errors #2434
Conversation
| targets.forEach((config) => { | ||
| workers(config, (err) => { | ||
| if (err) { | ||
| return reject(err); | ||
| console.error(err); |
There was a problem hiding this comment.
would this error be displayed 2 times? (here and in in the catch )
What happens when there are multiple errors? e.g. multiple targets generate errors.
There was a problem hiding this comment.
Yes I think it would be displayed two times. I figured it was easiest to just log all the exceptions, since as you say there could be multiple.
There was a problem hiding this comment.
In the case of multiple exceptions we print them all but then reject with the last one.
There was a problem hiding this comment.
I figured. wouldn't be better to reject with an array of exceptions and print them all, only once in the catch?
There was a problem hiding this comment.
I thought it was bad practice to reject with anything but an Error? But I guess if we're the only ones handling it, then it's fine.
There was a problem hiding this comment.
OK, I came up with a much simpler solution using Promise.all and promisify. No need to do any fancy error handling ourselves; just let Promise.all handle it.
nolanlawson
force-pushed
the
nolan/worker-errors
branch
from
8caca57
to
cba1810
Compare
Details
In the
lwcpackage, in thebuild.jsscript, if any of the worker processes throws an error, currently it will just hang forever with no output. This can be confusing. 🙂This PR fixes it so that if any worker throws an error, the process exits with a non-zero exit code and logs the error. I confirmed that this worked by throwing a deliberate
Errorin one of the build workers and confirming that it gave useful error output.Does this PR introduce breaking changes?
No, it does not introduce breaking changes.