fix: improve error messaging in evalBlueprint() #1512
Conversation
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| try { | ||
| entry = script.runInContext(context) | ||
| } catch (e) { | ||
| logger.error(`Error evaluating Blueprint .runInContext "${blueprint._id}": "${stringifyError(e)}"`) |
There was a problem hiding this comment.
This needs to throw something still, otherwise it will throw an error on line 25 as it cant read property 'default' on undefined
I wonder if the handling is corect to have here, or if it should be the existing try/catch around the one usage of this that we care about this error from where it needs some better handling of the error, the one where it throws throw new Meteor.Error(400, `Blueprint ${blueprintId} failed to parse`)
While there may be value in having the try/catch at this level, we currently assume that once blueprints are stored in the db they will be sane and always parsable, which this wont change but will make noisier in the logs if it happens. So I am unsure if it adds value
About the Contributor
This PR is made on behalf of BBC
Type of Contribution
This is a code improvement
Current Behavior
Currently blueprints that are uploaded but fails to run (e.g. if it reference to node process) will show this error-message in core:
[error] Blueprint restore failed: Error: Blueprint main-showstyle failed to parse [400]New Behavior
Adding a try-catch around the .runInContext() ensures a detailed error message, informing about the issue in the blueprint
Testing
Affected areas
This PR affects errorhandling in upload of Blueprints
Time Frame
Other Information
Status