Better context for script errors
gerzytet opened this issue · 9 comments
In #192, we introduced a warning system that not only tells you the problem, but also prints out the line it occurs on.
We should do the same with interpreter error messages, as it's much easier to debug a line of code than a toString of the token.
In addition, the exception handler should include the exception type in the error report, to reduce the
cases where the only thing we have to look at in the error is null
.
Can you explaine your plan more in detail?
I'm also planning to rework on the current warning system, and it might can he handled together.
The current error handler explains errors like this:
couldn't execute the trigger
caused by
couldn't finish interpreting mytrigger
caused by
error at row 1 column 3
caused by
no executor named #derp found
If you are an administrator, see console for details"
and those errors frequently include direct token toStrings which don't help very much.
My end goal is this format:
could not execute [triggertype] [triggername] at row [row], column [column]:
[optional error type]: [bottom error message]
[script line]
[^ pointing to the column]
If you are an administrator, see console for details
Using the first example, the error would look like this:
could not execute namedTtrigger mytrigger at row 1 column 3
no executor named #derp found
#derp
^
If you are an administrator, see console for details
More specifically:
We change InterpreterException to include include constructors that have line numbers or token context. TriggerReactor.handleException() will use it to display the line that the error is on.
If one of the causes in the cause chain is not an interpreter error or js error, the hhandler should say what type it is.
I have some more ideas about how to handle the new formatting, to make it more user-friendly, which is why I want to do this one.
The warning system works a lot differently then the interpreter errors that rely on cause chains, it can be done separately.
@wysohn
I see what you are doing.
Any chance where I can see the brief design? (UML is great, but not necessary)
@gerzytet
@wysohn
My plan is to do the following:
- Add a constructor to Interpreter with the signature Interpreter(Parser, Trigger). Add new fields to interpreter to hold these objects
- Change lexer.getScriptLines() to lexer.getScriptLine(int). There’s no reason why the lexer should make the script openly mutable.
- Add Parser.getScriptLine(int) as a delegate to the above method
- Add ScriptException, a common superclass for the lexer, parser, and interpreter exceptions. Will contain fields for row numbers, column numbers, and other context.
- Add new constructors to the lexer and parser exceptions that accept line context, and either a Token, Node, or row+column depending on the class.
- Add constructors for interpreter exceptions that accept parser and lexer exceptions, and trigger name/type context.
- Add factory methods to Lexer, Parser, and Interpreter for creating exceptions with the above information. Exceptions should no longer be created directly within
- Change exception handling within the parser to do nothing with lexer exceptions. The interpreter will construct interpreter exceptions from lexer and parser exceptions. no more long cause chains
- Write scriptException.toString() to use the above format
- Overload TriggerReactor.handleException for ScriptExceptions
Looks good
I would really like to see something to visualize the architecture of how the classes will be structured. Can you do that for me? UML or hand drawing or whatever shows how each class is structured together. (Visual Paradigm is online and free)
on second thought, I decided to keep the cause chains for debugging purposes. They won't be visible anymore though
As I began to try to implement this one, I started to realize how complicated it would be.
I don’t think it’s worth the effort anymore.
I've deleted the interpreter_errors branch remotely.
I still have it locally, but I haven't made a whole lot of progress.
The problem is that it would require replacing every error constructor in the Interpreter, all for a different error message format.
If we want to try this again in the future, a better solution would be a regex that scrapes the error message for row/column info and then reformats.
It isn't really the right way, but the right way requires refactoring large amounts of code.