fix(@embark/core): don't exit in Engine consumer API #2087
Conversation
DeepCode Report (#df671f)DeepCode analyzed this pull request. |
| return callback(err); | ||
| } else { | ||
| callback(); | ||
| } |
There was a problem hiding this comment.
Some thoughts to consider for the future and potentially #2028 :
While startEngine() takes a callback to inform its caller when it's "ready" it actually also triggers actions (e.g. spinning up cockpit etc). In fact, the "only" thing startEngine() does is running actions for plugins if they are registered. If there's no plugins, it'll just call its given callback.
Then, OTOH, compilation and deployment is done only as part of this particular callback (see line 209-210).
Might be worth considering renaming startEngine() to something more appropriate, if it really only triggers plugin actions.
Food for thought.
There was a problem hiding this comment.
To add to that: it even so happens that (right now) all embark:engine:started actions are being registered inside the cmd_controller which should probably rather happen in the dedicated modules.
| engine.logger.info(err.stack); | ||
| } else { | ||
| engine.logger.error(err); | ||
| } |
There was a problem hiding this comment.
This check here is needed because sometimes we're getting an actual Error sometimes we're getting just a string (error message).
Hopefully in the future we have this streamlined and always emit a proper error type (see #911)
| engine.startEngine(async (err) => { | ||
| if (err) { | ||
| return callback(err); | ||
| } else { |
There was a problem hiding this comment.
You can remove the else since you return in the if
`Engine.startEngine(cb)` potentially exits the current process if any registered plugins emits an error in the bootstrap phase. This limits consumers of this API to react accordingly. Embark's APIs should be considered library APIs that propagate errors, but let callers decide what to do with them. This commit ensures we keep propagating the error of `startEngine()` without exiting the current process.
0x-r4bbit
force-pushed
the
fix/engine-error-handling
branch
from
630788a
to
df671f7
Compare
Engine.startEngine(cb)potentially exits the current process ifany registered plugins emits an error in the bootstrap phase.
This limits consumers of this API to react accordingly. Embark's APIs
should be considered library APIs that propagate errors, but let callers
decide what to do with them.
This commit ensures we keep propagating the error of
startEngine()withoutexiting the current process.