From 2b345c1f774fe36b7209d3c6f74cdcdc120ae580 Mon Sep 17 00:00:00 2001 From: Stefan Zermatten Date: Fri, 12 Feb 2021 11:00:44 +0200 Subject: [PATCH] Improved error handling for most calculations --- .../embedInlineCalculations.js | 2 +- .../afterComputation/evaluateString.js | 5 - .../engine/computeInlineCalculations.js | 6 +- .../computation/engine/evaluateCalculation.js | 42 +++--- app/imports/api/creature/log/CreatureLogs.js | 20 ++- app/imports/api/properties/Constants.js | 34 +++-- app/imports/parser/parseTree/ErrorNode.js | 2 +- app/imports/parser/parser.js | 15 ++- .../ui/creature/slots/SlotFillDialog.vue | 9 +- app/imports/ui/log/CharacterLog.vue | 16 +-- app/imports/ui/pages/Parser.vue | 121 ------------------ app/imports/ui/router.js | 9 -- 12 files changed, 80 insertions(+), 201 deletions(-) delete mode 100644 app/imports/ui/pages/Parser.vue diff --git a/app/imports/api/creature/computation/afterComputation/embedInlineCalculations.js b/app/imports/api/creature/computation/afterComputation/embedInlineCalculations.js index 3821edff..8c565f3b 100644 --- a/app/imports/api/creature/computation/afterComputation/embedInlineCalculations.js +++ b/app/imports/api/creature/computation/afterComputation/embedInlineCalculations.js @@ -4,6 +4,6 @@ export default function embedInlineCalculations(string, calculations){ let index = 0; return string.replace(/\{([^{}]*)\}/g, () => { let comp = calculations && calculations[index++]; - return comp && comp.result; + return comp && comp.result ? comp.result : string; }); } diff --git a/app/imports/api/creature/computation/afterComputation/evaluateString.js b/app/imports/api/creature/computation/afterComputation/evaluateString.js index 5bd780a5..0dfe4115 100644 --- a/app/imports/api/creature/computation/afterComputation/evaluateString.js +++ b/app/imports/api/creature/computation/afterComputation/evaluateString.js @@ -18,11 +18,6 @@ export default function evaluateString(string, scope, fn = 'compile'){ errors.push(e); return {result: string, errors}; } - // Parsing failed - if (node === null){ - errors.push('...'); - return {result: string, errors}; - } let context = new CompilationContext(); let result = node[fn](scope, context); if (result instanceof ConstantNode){ diff --git a/app/imports/api/creature/computation/engine/computeInlineCalculations.js b/app/imports/api/creature/computation/engine/computeInlineCalculations.js index c5979af5..1f02854e 100644 --- a/app/imports/api/creature/computation/engine/computeInlineCalculations.js +++ b/app/imports/api/creature/computation/engine/computeInlineCalculations.js @@ -1,4 +1,5 @@ import evaluateCalculation from '/imports/api/creature/computation/engine/evaluateCalculation.js'; +import ErrorNode from '/imports/parser/parseTree/ErrorNode.js'; import { union } from 'lodash'; export default function computeInlineCalculations(prop, memo){ @@ -21,9 +22,12 @@ function computeInlineCalcsForField(prop, memo, field){ context, dependencies, } = evaluateCalculation({string: calculation, prop, memo, fn: 'compile'}); + if (result instanceof ErrorNode){ + result = `${result.toString()}`; + } let computation = { calculation, - result: result.toString(), + result: result && result.toString(), }; if (context.errors.length){ computation.errors = context.errors; diff --git a/app/imports/api/creature/computation/engine/evaluateCalculation.js b/app/imports/api/creature/computation/engine/evaluateCalculation.js index 2a92c40a..bf071622 100644 --- a/app/imports/api/creature/computation/engine/evaluateCalculation.js +++ b/app/imports/api/creature/computation/engine/evaluateCalculation.js @@ -1,8 +1,9 @@ import computeStat from '/imports/api/creature/computation/engine/computeStat.js'; -import { parse, CompilationContext } from '/imports/parser/parser.js'; +import { prettifyParseError, parse, CompilationContext } from '/imports/parser/parser.js'; import SymbolNode from '/imports/parser/parseTree/SymbolNode.js'; import AccessorNode from '/imports/parser/parseTree/AccessorNode.js'; import ConstantNode from '/imports/parser/parseTree/ConstantNode.js'; +import ErrorNode from '/imports/parser/parseTree/ErrorNode.js'; import findAncestorByType from '/imports/api/creature/computation/engine/findAncestorByType.js'; import { union } from 'lodash'; @@ -14,10 +15,10 @@ export default function evaluateCalculation({ fn = 'reduce', }){ let dependencies = []; - let errors = []; + let context = new CompilationContext(); if (!string) return { - context: {errors}, result: new ConstantNode({value: string, type: 'string'}), + context, dependencies, }; // Parse the string @@ -25,34 +26,24 @@ export default function evaluateCalculation({ try { calc = parse(string); } catch (e) { - errors.push({ - type: 'parsing', - message: e.message || e - }); + let error = prettifyParseError(e); return { - context: {errors}, - result: new ConstantNode({value: string, type: 'string'}), - dependencies, - }; - } - if (!calc){ - return { - context: {errors}, - result: new ConstantNode({value: calc, type: 'string'}), + result: new ErrorNode({context, error}), + context, dependencies, }; } // Replace constants with their parsed constant let replaceResults = replaceConstants({ - calc, memo, prop, dependencies, errors + calc, memo, prop, dependencies, context }); dependencies = replaceResults.dependencies; calc = replaceResults.calc; if (replaceResults.failed){ return { - context: {errors}, result: new ConstantNode({value: string, type: 'string'}), + context, dependencies, }; } @@ -61,13 +52,12 @@ export default function evaluateCalculation({ dependencies = computeSymbols({calc, memo, prop, dependencies}) // Evaluate - let context = new CompilationContext(); let result = calc[fn](memo.statsByVariableName, context); return {result, context, dependencies}; } // Replace constants in the calc with the right ParseNodes -function replaceConstants({calc, memo, prop, dependencies, errors}){ +function replaceConstants({calc, memo, prop, dependencies, context}){ let constFailed = []; calc = calc.replaceNodes(node => { if (!(node instanceof SymbolNode)) return; @@ -101,16 +91,16 @@ function replaceConstants({calc, memo, prop, dependencies, errors}){ } }); constFailed.forEach(name => { - errors.push({ + context.storeError({ type: 'error', message: `${name} is a constant property with parsing errors` }); }); - return { - failed: !!constFailed.length, - dependencies, - calc, - }; + let failed = !!constFailed.length; + if (failed){ + calc = new ErrorNode({error: 'Failed to replace constants'}); + } + return { failed, dependencies, calc }; } // Ensure all symbol nodes are defined and computed diff --git a/app/imports/api/creature/log/CreatureLogs.js b/app/imports/api/creature/log/CreatureLogs.js index d0add822..a6f027f6 100644 --- a/app/imports/api/creature/log/CreatureLogs.js +++ b/app/imports/api/creature/log/CreatureLogs.js @@ -4,7 +4,11 @@ import LogContentSchema from '/imports/api/creature/log/LogContentSchema.js'; import { ValidatedMethod } from 'meteor/mdg:validated-method'; import { RateLimiterMixin } from 'ddp-rate-limiter-mixin'; import {assertEditPermission} from '/imports/api/creature/creaturePermissions.js'; -import { parse, CompilationContext } from '/imports/parser/parser.js'; +import { + parse, + CompilationContext, + prettifyParseError +} from '/imports/parser/parser.js'; const PER_CREATURE_LOG_LIMIT = 100; if (Meteor.isServer){ @@ -149,13 +153,15 @@ const logRoll = new ValidatedMethod({ avatarPicture: 1, }}); assertEditPermission(creature, this.userId); - let parsedResult = parse(roll); - let logContent; - if (parsedResult === null) { - logContent = [{error: 'Unexpected end of input'}]; + let logContent = [] + let parsedResult = undefined; + try { + parsedResult = parse(roll); + } catch (e){ + let error = prettifyParseError(e); + logContent.push({error}); } - else try { - logContent = []; + if (parsedResult) try { let rollContext = new CompilationContext(); let compiled = parsedResult.compile(creature.variables, rollContext); let compiledString = compiled.toString(); diff --git a/app/imports/api/properties/Constants.js b/app/imports/api/properties/Constants.js index 830ee526..41c0f14c 100644 --- a/app/imports/api/properties/Constants.js +++ b/app/imports/api/properties/Constants.js @@ -1,7 +1,11 @@ import SimpleSchema from 'simpl-schema'; import VARIABLE_NAME_REGEX from '/imports/constants/VARIABLE_NAME_REGEX.js'; import ErrorSchema from '/imports/api/properties/subSchemas/ErrorSchema.js'; -import { parse, CompilationContext } from '/imports/parser/parser.js'; +import { + parse, + CompilationContext, + prettifyParseError, +} from '/imports/parser/parser.js'; import AccessorNode from '/imports/parser/parseTree/AccessorNode.js'; import SymbolNode from '/imports/parser/parseTree/SymbolNode.js'; /* @@ -35,19 +39,19 @@ let ConstantSchema = new SimpleSchema({ } let string = calc.value; // Evaluate the calculation with no scope - let {result, errors} = parseString(string); - // Any errors will result in a failure - if (errors.length) return errors; + let {result, context} = parseString(string); + // Any existing errors will result in an early failure + if (context.errors.length) return context.errors; // Ban variables in constants if necessary result && result.traverse(node => { if (node instanceof SymbolNode || node instanceof AccessorNode){ - errors.push({ + context.storeError()({ type: 'error', message: 'Variables can\'t be used to define a constant' }); } }); - return errors; + return context.errors; } }, 'errors.$':{ @@ -56,9 +60,9 @@ let ConstantSchema = new SimpleSchema({ }); function parseString(string, fn = 'compile'){ - let errors = []; + let context = new CompilationContext(); if (!string){ - return {result: string, errors}; + return {result: string, errors: []}; } // Parse the string using mathjs @@ -66,18 +70,12 @@ function parseString(string, fn = 'compile'){ try { node = parse(string); } catch (e) { - let message = e.toString().split('.')[0]; - errors.push({type: 'error', message}); - return {result: string, errors}; + let message = prettifyParseError(e); + context.storeError({type: 'error', message}); + return {result: string, errors: context.errors}; } - // Parsing incomplete - if (node === null){ - errors.push({type: 'warning', message: 'Unexpected end of input'}); - return {result: string, errors}; - } - let context = new CompilationContext(); let result = node[fn]({/*empty scope*/}, context); - return {result, errors: context.errors} + return {result, context} } export { ConstantSchema }; diff --git a/app/imports/parser/parseTree/ErrorNode.js b/app/imports/parser/parseTree/ErrorNode.js index fd23b666..ff9565ee 100644 --- a/app/imports/parser/parseTree/ErrorNode.js +++ b/app/imports/parser/parseTree/ErrorNode.js @@ -16,6 +16,6 @@ export default class ErrorNode extends ParseNode { return this; } toString(){ - return '###'; + return this.error.toString(); } } diff --git a/app/imports/parser/parser.js b/app/imports/parser/parser.js index 3fc568e1..13f24579 100644 --- a/app/imports/parser/parser.js +++ b/app/imports/parser/parser.js @@ -28,10 +28,21 @@ export function parse(string){ if (results.length === 1){ return results[0]; } else if (results.length === 0){ - // Valid parsing up until now, but need more. Unexpected end of input. - return null; + // Valid parsing up until now, but need more + throw new EndOfInputError('Unexpected end of input'); } else { console.warn('Grammar is ambiguous!', {string, results}); return results[0]; } } + +export function prettifyParseError(e){ + if (e.message) e = e.message + return e.toString().split('.')[0]; +} + +class EndOfInputError extends Error { + constructor(message = '', ...args) { + super(message, ...args); + } +} diff --git a/app/imports/ui/creature/slots/SlotFillDialog.vue b/app/imports/ui/creature/slots/SlotFillDialog.vue index d1a032dc..75177a29 100644 --- a/app/imports/ui/creature/slots/SlotFillDialog.vue +++ b/app/imports/ui/creature/slots/SlotFillDialog.vue @@ -244,8 +244,13 @@ export default { nodes = nodes.filter(node => { if (node.slotFillerCondition){ let context = new CompilationContext(); - let conditionResult = parse(node.slotFillerCondition) - .reduce(this.creature.variables, context); + let conditionResult; + try { + conditionResult = parse(node.slotFillerCondition) + .reduce(this.creature.variables, context); + } catch (e){ + console.warn(e); + } if (conditionResult && !conditionResult.value) return false; } if ( diff --git a/app/imports/ui/log/CharacterLog.vue b/app/imports/ui/log/CharacterLog.vue index 0e39dd42..b29d65a3 100644 --- a/app/imports/ui/log/CharacterLog.vue +++ b/app/imports/ui/log/CharacterLog.vue @@ -34,7 +34,7 @@ import CreatureLogs, { logRoll } from '/imports/api/creature/log/CreatureLogs.js'; import Creatures from '/imports/api/creature/Creatures.js'; import { assertEditPermission } from '/imports/api/creature/creaturePermissions.js'; -import { parse } from '/imports/parser/parser.js'; +import { parse, prettifyParseError } from '/imports/parser/parser.js'; import LogEntry from '/imports/ui/log/LogEntry.vue'; export default { @@ -61,12 +61,12 @@ export default { try { result = parse(value); } catch (e){ - console.error(e); - this.inputError = 'Invalid syntax'; - return; - } - if (result === null){ - this.inputError = '...'; + if (e.constructor.name === 'EndOfInputError'){ + this.inputError = '...'; + } else { + let error = prettifyParseError(e); + this.inputError = error; + } return; } try { @@ -74,7 +74,7 @@ export default { this.inputHint = compiled.toString(); return; } catch (e){ - console.error(e); + console.warn(e); this.inputError = 'Compilation error'; return; } diff --git a/app/imports/ui/pages/Parser.vue b/app/imports/ui/pages/Parser.vue deleted file mode 100644 index 2ff7c6ef..00000000 --- a/app/imports/ui/pages/Parser.vue +++ /dev/null @@ -1,121 +0,0 @@ - - - - - diff --git a/app/imports/ui/router.js b/app/imports/ui/router.js index 2eac1cf3..c0175bcb 100644 --- a/app/imports/ui/router.js +++ b/app/imports/ui/router.js @@ -24,7 +24,6 @@ import PatreonLevelTooLow from '/imports/ui/pages/PatreonLevelTooLow.vue'; import Tabletops from '/imports/ui/pages/Tabletops.vue'; import Tabletop from '/imports/ui/pages/Tabletop.vue'; import TabletopToolbar from '/imports/ui/tabletop/TabletopToolbar.vue'; -import Parser from '/imports/ui/pages/Parser.vue'; let userSubscription = Meteor.subscribe('user'); @@ -111,14 +110,6 @@ RouterFactory.configure(factory => { title: 'Character List', }, beforeEnter: ensureLoggedIn, - },{ - path: '/parser', - components: { - default: Parser, - }, - meta: { - title: 'Parser', - }, },{ path: '/library', components: {