Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create standard error models for all thema operations #164

Open
sdboyer opened this issue May 23, 2023 · 2 comments
Open

Create standard error models for all thema operations #164

sdboyer opened this issue May 23, 2023 · 2 comments
Labels
enhancement New feature or request invariants Involves the definition or enforcement of a key system invariant prio/mid Medium priority readiness/minimum Requisite for achieving a minimum readiness

Comments

@sdboyer
Copy link
Contributor

sdboyer commented May 23, 2023

As a system that deals with validation, error design is an essential part of Thema's UX. We need a clear, consistent model for the errors that Thema will produce, including clear and easily introspected Go error types.

Each major thema operation (validation, translation) needs its own error model. Lineage binding also needs an error model - probably more urgently than either validate or translate, as the lineage binding error model is basically the "lineage typechecker" that enforces all invariants.

Properties for thema's error models should include:

  • A well-defined tree of error types, identifiable via errors.Is
  • Line-level information about errors, with graceful fallback when no line-level information is available
  • A clear aggregation mechanism for situations where multiple errors may occur (e.g. validation fails on multiple fields)
  • Network portability, so that e.g. a standalone thema http server reliably produces the same information as would have arisen from something in-process

I got started on the mechanics of this in #82 by pulling in github.com/cockroachdb/errors, but ended up deferring it in the interest of actually the overall PR done.

This supersedes #44, as the scope of that issue was really just focused on validate errors - only one of the models we need. Still relevant is the discussion about noisy errors in disjunctions, though.

@sdboyer sdboyer added enhancement New feature or request invariants Involves the definition or enforcement of a key system invariant labels May 23, 2023
@sdboyer
Copy link
Contributor Author

sdboyer commented May 23, 2023

This is gonna take some discussion before starting - probably needs a design doc.

@sdboyer
Copy link
Contributor Author

sdboyer commented May 24, 2023

Note that this:

Line-level information about errors

is deceptively tricky. On the one hand, CUE very awesomely carries line-level traceability for validation errors by construction. But there are caveats.

Line-level error info ultimately ties back to associations between the vertices that underlie a cue.Value and source file, specifically a token.Pos. There are many things that can cause a cue.Value to lack such information - e.g., if it was created by cue.Value.Unify(), if it is the result of a comprehension.

It's not as simple as "if cue.Value.Source() returns nil, there will be no line info in errors." That wouldn't simplify things much, anyway. But i do not know of a clean, general rule that clearly tells us in what circumstance we can expect errors coming from to have line information (a non-empty return from this). (cc @myitcv)

In addition to the pure CUE reasons that line numbers may be lacking, we'll also need to account for things the user may have done that would cause the cue.Value contained in a Thema type to have its line numbers disappear. For example, on Instance, Translate() Hydrate and Dehydrate all manipulate a cue.Value in a way that may make it impossible to meaningfully include line numbers in errors, as the source has undergone a transformation and the "input file" no longer exists. Similarly, composition (#8) will almost certainly include calls to cue.Value.Unify() that...could? cause similar issues.

When i decided Thema should accept cue.Value as the input type for various funcs like BindLineage() or Schema.Validate() (vs. say having them accept []byte), it significantly simplified Thema's API, but it came with the burden of dealing with some of these error ambiguities. As a result, we have to generally expect that every error Thema emits will have both a "with line number" and "without line numbers" form. A complete error model will need to account for this difference, perhaps including explicit differentiation between cases where there are no line numbers because an original input cue.Value lacked source context, and cases where Thema operations themselves have caused them to be unavailable. Ambiguity around error line number availability has been quite frustrating for me when working with the CUE API, and i suspect it will be just as frustrating for Thema's users.

That said, not all of this needs to be dealt with in a first pass. I'm just outlining the scope of the problem.

@joanlopez joanlopez added prio/mid Medium priority readiness/minimum Requisite for achieving a minimum readiness labels Jun 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request invariants Involves the definition or enforcement of a key system invariant prio/mid Medium priority readiness/minimum Requisite for achieving a minimum readiness
Projects
None yet
Development

No branches or pull requests

2 participants