Indicating expression location for error

[dvd101x]

When evaluating with multiple expressions and one of them has an error:

math.evaluate(["1+1","2+"])

The error is:

SyntaxError: Unexpected end of expression (char 3)

I would like to know which is the expression with the error: SyntaxError: Unexpected end of expression (expr 2, char 3)

[josdejong]

Thanks, that is an interesting suggestion.

Anyone interested in looking into this?

[dvd101x]

I'm sorry I didn't mean to close this. I was trying to say I've been testing other alternatives. Like getting all the responses from the evaluate until the error.

math.evaluate(["1+1","2+"])

Response:

[2, "SyntaxError: Unexpected end of expression (char 3)"]

[josdejong]

You can evaluate the expressions one by one:

const expressions = ["1+1","2+"]

expressions.forEach((expression, index) => {
  try {
    const res = math.evaluate(expression)
  } catch (err) {
    // do something here... 
    console.error(index, err)
  }  
})

[dvd101x]

Thank you, it runs very well!

I made some tests to see if there was a penalty on performance for a very big list of expressions but no, it seems take about the same amount of time!

[josdejong]

Thanks, good to hear 👍

[josdejong]

Ow wait, I shouldn't yet close the issue: would be very useful to improve the errors in case of an array with expressions.

[dvd101x]

I agree, I still think it would be useful to improve errors for the case of an array of expressions.

I think my initial request is not practical as there are many different errors that might be returned.

If we evaluate the following expressions sequentially:

2+2+
1+(2+3
2 mm + 3 s
A1 = 2
A1[1]
A2 = [2,3]
A2[0]
A2[3]
concat("A",A2)
inv([0,1;2,3;4,5])
f(x,y) = x*y;
f(2)
g(2,3)
help(summ)
help(f)
[1,2;3,4]*[5,6,7]

We get many different error types:

SyntaxError: Unexpected end of expression (char 5)
SyntaxError: Parenthesis ) expected (char 7)
Error: Units do not match
2
TypeError: Cannot apply index: unsupported type of object
[2, 3]
IndexError: Index out of range (0 < 1)
IndexError: Index out of range (3 > 2)
Error: Cannot convert "A" to a number
RangeError: Matrix must be square (size: [3, 2])
[]
TypeError: Too few arguments in function f (expected: any, index: 1)
Error: Undefined function g
Error: Undefined symbol summ
Error: No documentation found on "f"
RangeError: Dimension mismatch in multiplication. Matrix columns (2) must match Vector length (3)

If we run it as an array of expressions, I think it's ok to return the first encountered error, but it would be better to somehow include which of the expressions has the error the same way for all error types.

[1]: SyntaxError: Unexpected end of expression (char 5)
[2]: SyntaxError: Parenthesis ) expected (char 7)
[3]: Error: Units do not match
2
[5]: TypeError: Cannot apply index: unsupported type of object
[2, 3]
[7]: IndexError: Index out of range (0 < 1)
[8]: IndexError: Index out of range (3 > 2)
[9]: Error: Cannot convert "A" to a number
[10]: RangeError: Matrix must be square (size: [3, 2])
[]
[12]: TypeError: Too few arguments in function f (expected: any, index: 1)
[13]: Error: Undefined function g
[14]: Error: Undefined symbol summ
[15]: Error: No documentation found on "f"
[16]: RangeError: Dimension mismatch in multiplication. Matrix columns (2) must match Vector length (3)

Or in a different fashion that can work for all error types

Expression 1 - SyntaxError: Unexpected end of expression (char 5)

SyntaxError: Unexpected end of expression (char 5) (expr 1)

Expression[1] SyntaxError: Unexpected end of expression (char 5)

[josdejong]

Yes indeed, thanks for sharing a clear example David👍

[zishiwu123]

When evaluating with multiple expressions and one of them has an error:

math.evaluate(["1+1","2+"])

The error is:

SyntaxError: Unexpected end of expression (char 3)

I would like to know which is the expression with the error: SyntaxError: Unexpected end of expression (expr 2, char 3)

Research The error message is generated by: https://github.com/josdejong/mathjs/blob/4f83b786ea420b4095073dc97f3321d1dd768147/src/expression/parse.js#L1769-L1775

Here are the contents of the state object passed into createSyntaxError() for the first example @dvd101x posted:

{
  extraNodes: {},
  expression: '2+',
  comment: '',
  index: 2,
  token: '',
  tokenType: 1,
  nestingLevel: 0,
  conditionalLevel: null
}

Proposed Solution The state object tracks the current evaluated expression "2+", which is where the error occurs. But it doesn't track the index of the current expression (let's call it expressionIndex) being evaluated from the array of expressions ["1+1","2+"] that was passed into math.evaluate. If we want to know expressionIndex, I think we need to add it as an additional property to the state object and increment it every time this line in parseObject() is executed: https://github.com/josdejong/mathjs/blob/4f83b786ea420b4095073dc97f3321d1dd768147/src/expression/parse.js#L1660

The existing index property in the state object tracks the index of the current character in the current expression. This is different from the proposed expressionIndex property.

Questions Besides occurring in parseObject(), the check for condition state.token === ',' occurs 5 other times, all in parse.js. I am not very familiar with the code base but I would like to help out with this issue. So far my questions are:

1. Does the method I proposed for parseObject() need to be applied [in the same way / in a different way /not at all] in the other 5 occurrences of the condition state.token === ','?
2. Do we want to enforce a consistent style of error messages? If so, should it be one of the formats proposed by @dvd101x?

Expression 1 - SyntaxError: Unexpected end of expression (char 5)

SyntaxError: Unexpected end of expression (char 5) (expr 1)

Expression[1] SyntaxError: Unexpected end of expression (char 5)

[gwhitney]

Thanks so much for your interest in this issue, @zishiwu123. The point in parse.js that you have identified has to do with looping over the clauses in the object syntax {x: 1, y: 2} rather than the different expressions in a call of evaluate on an array of expressions.

The loop you are looking for -- actually there are two instances -- are the deepmaps in src/expression/function/evaluate.js, near the end where it deals with Array or Matrix arguments. There is where you'd have to capture the index information and create a way that an eventual error could access it.

Finally, my personal suggestion is that the expression index annotation should only appear when the error occurs in one of these array or matrix calls, so that the errors from single-expression calls are not cluttered with index information that doesn't add anything -- when you evaluate a single expression, there really is no "index".

[dvd101x]

Is it possible to return both the correct results and the errors on the return array similar to this https://github.com/josdejong/mathjs/issues/2318#issuecomment-925861448?

Currently I use something similar that returns for each expression in the array either the result or the error as a string.

[gwhitney]

Is it possible to return both the correct results and the errors on the return array similar to this #2318 (comment)?

Well, that is an interesting question. The code internally is throwing errors. So the usual behavior would be for those errors to propagate outside of mathjs -- I mean, that's what "throwing an error" is all about. So while it's absolutely possible and desirable to improve the data in an error, imagine the client that can't afford for there to be any errors and absolutely wants to catch the first one before executing any further? So once that first exception is thrown and not caught within the mathjs package, there is no opportunity to continue evaluating the remaining expressions.

Hence, I think that all mathjs could do is offer an 'evaluateAll' convenience method that catches errors for you around every expression -- basically recreate the loop that Jos shows in https://github.com/josdejong/mathjs/issues/2318#issuecomment-925861448 but in a standardized mathjs method -- or possibly (?) add an options argument to evaluate() that would take say '{catch: true}' to tell evaluate to wrap each evaluation in a try and if it catches one, return the error object instead of the result. I think @josdejong would have to weigh in on any of these possibilities.

[josdejong]

Good discussion, thanks.

It makes sense to extend the state to pass information about the current line, thanks for investigating @zishiwu123.

About throwing an exception vs returning a list with results/errors: you can indeed only throw 1 exception. I think it is best to keep this behavior of exception throwing. On top of this, we could indeed implement an evaluateAll util method like @gwhitney propopses, but at this moment I suggest we leave this function up to the end user (and not provide it inside mathjs), the handiest implementation may differ per application, and it is really just a few lines of code:

function evaluateAll(expressions) {
  return expressions.map(expression => {
    try {
      return math.evaluate(expression)
    } catch (err) {
      return err
    }
  })
}

[dvd101x]

I think it would be useful to indicate in which expression the error was found like described by @zishiwu123.

Maybe if the expression has multiple lines the error could indicate the line and character. This might not be as important because it could also be extracted and converted by the user.

It seems reasonable to me not to implement evaluateAll but just improve the exception thrown to know where the error was found.

[gwhitney]

Yes, I think the consensus here is to extend the "state" of parsing to allow to record which expression was being evaluated in an array call to evaluate (but not burden the message in an ordinary single-expression call). A PR to that end would, I believe be welcome.