Error handling improvements #125
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Remove Error.Delegate - Skip JVM stack trace handling - Consistent formatting of raw delegate errors and regular positioned errors
Instead of sprinkling error propagation with or without stack frames all over the Evaluator and other parts of the codebase, we now do it in one place (visitExpr) with additional logic in Error.withStackFrame. Duplicate frames are skipped, an error without any stack frame always gets the current position, and all other frames are added or skipped depending on the Expr at the position.
This makes stack traces much more descriptive. We also change some of the reported positions / expr types to better match the semantics of the errors.
They are reported as sjsonnet.ParseError with normal stack traces instead of the previous one-liners which required customized error messages for parse errors in imported files. Errors are now stored as Error objects instead of Strings in the parse cache. This paves the way for error reporting during static evaluation and it is already required now to translate the paths in parse error stack traces to a different base directory (when reusing a cached parse error from a previous run).
Unknown variables and illegal self/super/$ references are now checked statically as they are supposed to be. Error messages are consistent with Google Jsonnet's. Fixes databricks#122
iuliand-db
reviewed
Aug 5, 2021
|
|
||
| def asSeenFrom(ev: EvalErrorScope): Error = |
iuliand-db
approved these changes
Aug 5, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Instead of sprinkling error propagation with or without stack frames all over the Evaluator and other parts of the codebase, we now do it in one place (visitExpr) with additional logic in Error.withStackFrame. Duplicate frames are skipped, an error without any stack frame always gets the current position, and all other frames are added or skipped depending on the Expr at the position.
This makes stack traces much more descriptive. We also change some of the reported positions / expr types to better match the semantics of the errors.
They are reported as sjsonnet.ParseError with normal stack traces instead of the previous one-liners which required customized error messages for parse errors in imported files. Errors are now stored as Error objects instead of Strings in the parse cache. This paves the way for error reporting during static evaluation and it is already required now to translate the paths in parse error stack traces to a different base directory (when reusing a cached parse error from a previous run).
Unknown variables and illegal self/super/$ references are now checked statically as they are supposed to be. Error messages are consistent with Google Jsonnet's.
Fixes #122