[RFC0017] Add invalid and missingerror values#673
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements RFC0017 by introducing invalid and missing error values to represent failed evaluations instead of returning undefined. The implementation adds new error classes (InvalidError and MissingError) and replaces undefined returns throughout the codebase with these error objects. Additionally, it includes a restructuring of PostgresLoaderExecutor with more granular error handling.
- Introduces new error types
InvalidErrorandMissingErrorwith corresponding type guards - Replaces
undefinedreturns with error objects throughout expression evaluation and property handling - Updates all value type classes to handle error representations in type checking methods
- Restructures
PostgresLoaderExecutorwith granular try/catch blocks for better error isolation
Reviewed Changes
Copilot reviewed 55 out of 55 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
libs/language-server/src/lib/ast/expressions/internal-value-representation.ts |
Defines new error classes and updates internal value representation types |
libs/language-server/src/lib/ast/expressions/typeguards.ts |
Adds type guards for error detection and removes unused helper function |
libs/language-server/src/lib/ast/expressions/evaluate-expression.ts |
Updates expression evaluation to return errors instead of undefined |
libs/language-server/src/lib/ast/wrappers/value-type/value-type.ts |
Updates ValueType interface to handle error representations |
libs/language-server/src/lib/validation/validation-util.ts |
Replaces undefined check with error type guard |
libs/extensions/rdbms/exec/src/lib/postgres-loader-executor.ts |
Restructures with granular error handling methods |
Comments suppressed due to low confidence (2)
libs/language-server/src/lib/ast/expressions/typeguards.ts:69
- [nitpick] The function name
ERROR_TYPEGUARDis inconsistent with the pattern of other type guards. Consider renaming toERROR_TYPEGUARDorisErrorfor better consistency with other type guard naming conventions.
export const ERROR_TYPEGUARD = (
libs/extensions/rdbms/exec/src/lib/postgres-loader-executor.ts:61
- The method name
createErroris ambiguous. Consider renaming tocreateExecutionErrororformatDatabaseErrorto better indicate it creates a Result with error information, not just an Error object.
private createError(
libs/language-server/src/lib/ast/expressions/internal-value-representation.ts
Show resolved
Hide resolved
libs/language-server/src/lib/ast/expressions/internal-value-representation.ts
Show resolved
Hide resolved
libs/language-server/src/lib/ast/expressions/evaluation-context.ts
Outdated
Show resolved
Hide resolved
libs/language-server/src/lib/ast/expressions/evaluators/parse-operators-evaluators.spec.ts
Outdated
Show resolved
Hide resolved
libs/language-server/src/lib/ast/expressions/evaluators/parse-operators-evaluators.ts
Show resolved
Hide resolved
libs/language-server/src/lib/ast/expressions/internal-value-representation.ts
Show resolved
Hide resolved
libs/language-server/src/lib/ast/expressions/internal-value-representation.ts
Outdated
Show resolved
Hide resolved
libs/language-server/src/lib/ast/expressions/internal-value-representation.ts
Outdated
Show resolved
Hide resolved
libs/language-server/src/lib/ast/expressions/internal-value-representation.ts
Outdated
Show resolved
Hide resolved
georg-schwarz
left a comment
There was a problem hiding this comment.
Very nice progress! Left some comments to discuss and/or address ;-)
77bac31 to
fcd4187
Compare
…lue is inserted into the table
… and `InternalErrorRepresentation` - `InvalidError` -> `InvalidValue` - `MissingError` -> `MissingValue` - `InternalValueRepresentation` -> `InternalValidValueRepresentation` - `InternalErrorRepresentation` -> `InternalErrorValueRepresentation`
18c2c2e to
553baeb
Compare
libs/language-server/src/lib/ast/expressions/evaluation-context.ts
Outdated
Show resolved
Hide resolved
libs/language-server/src/lib/ast/expressions/internal-value-representation.ts
Show resolved
Hide resolved
libs/language-server/src/lib/ast/wrappers/typed-object/block-type-wrapper.ts
Outdated
Show resolved
Hide resolved
georg-schwarz
left a comment
There was a problem hiding this comment.
Phew, what a great addition! Left some minor comments you can address if you want. Aside from that, ready to merge from my side!
Depends on #674.
This PR implements RFC0017, accepted in #670.
Also included is a restructuring of
PostgresLoaderExecutorwith a more granulartry/catch.Reason for the draft status: The RFC states that a log message must be emitted every time an error is assigned to a variable. I still need to make sure that is actually the case in the implementation
Edit: sorry for the large diff