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

Improve error representation #64

Closed
jprochazk opened this issue Aug 12, 2023 · 0 comments · Fixed by #67
Closed

Improve error representation #64

jprochazk opened this issue Aug 12, 2023 · 0 comments · Fixed by #67
Milestone

Comments

@jprochazk
Copy link
Owner

Background

Currently, errors are represented as an enum. Containers use a variant of this enum that may hold other errors and associate them with keys (Errors::Fields) or indices (Errors::List). For leaf nodes of the error "tree", Errors::Simple is used, which is just a list of individual garde::Error produced by the validation rules when a check fails. There's a special Errors::Nested variant to handle the case where both the container and its items are validated. Combining dive or inner with a top-level rule like length(min=1) will emit this kind of error.

I believe that this representation is maximally flexible in terms of what you can build with it, because it is a direct reflection of the "shape" of the validation implementation. But it doesn't seem to be the right representation.

At least in my own usage of garde, I tend to just call flatten and use that when reporting errors, and I really dislike how much the validate implementations have to allocate to accomodate both the "tree" and "flat" representations. There has also been at least one request for accessing errors on specific fields in a more ergonomic way (#46).

To summarize, the issues are:

  • The derived Validate implementation perform excessive allocation
  • There is no convenient way to report the errors without accepting more allocations
  • There is no convenient way to inspect the errors

Proposed solution

Changing errors to be represented as a flat list like Vec<(Path, Error)> could improve the ergonomics and significantly alleviate the need for allocation. Because most field names, list indices, and often also errors are completely static, both Path and Error could use Cow under the hood to allow storing 'static values directly.

The actual error builder mechanism would then be drastically simplified: push all errors into a single top-level list. For dive, we'd add a way to supply an existing error list (again to reduce allocation) as a #[doc(hidden)] and default-implemented method on the Validate trait. Something similar would be done for inner.

Here's what the new error representation could look like: playground

@jprochazk jprochazk changed the title Improve error reporting Improve error representation Aug 12, 2023
@jprochazk jprochazk mentioned this issue Sep 1, 2023
4 tasks
@jprochazk jprochazk added this to the v0.15 milestone Sep 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant