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 ToString implementation on garde::Errors #26

Closed
Altair-Bueno opened this issue Mar 31, 2023 · 6 comments
Closed

Improve ToString implementation on garde::Errors #26

Altair-Bueno opened this issue Mar 31, 2023 · 6 comments

Comments

@Altair-Bueno
Copy link
Contributor

Description

Current ToString implementation lacks a lot of details.

Current output

For example, consider the following scheme:

#[derive(Debug, Serialize, Deserialize, Validate)]
struct Person {
    #[garde(ascii, length(min = 3, max = 25))]
    username: String,
    #[garde(length(min = 15))]
    password: String,
}

When fed with the following payload (JSON format), the current implementation only shows the first error message, instead of listing all errors

// Input
{"username": "h", "password": "aaaaa"}
// ToString impl
length is lower than 3
// Debug impl
Fields({"password": Simple([Error { message: "length is lower than 15" }]), "username": Simple([Error { message: "length is lower than 3" }])})

Expected behaviour

List all errors on some human readable form, like this

`field.name`: length is lower than 15
`field.name2`: length is lower than 3

Also, provide a serde::Serialize implementation so we could provide JSON responses on REST APIs (related to #12)

@jprochazk
Copy link
Owner

It's already possible to do this via Errors::flatten(), e.g.:

#[derive(Debug, Serialize, Deserialize, Validate)]
struct Person {
    #[garde(ascii, length(min = 3, max = 25))]
    username: String,
    #[garde(length(min = 15))]
    password: String,
}

let value = Person { username: "a".into(), password: "b".into() };
match value.validate(&()) {
  Ok(()) => {}
  Err(e) => {
    println!("{:?}", e.flatten());
  }
}

As for implementing Serialize for Errors, would you expect the output to use flatten or maintain the nested structure?

@Altair-Bueno
Copy link
Contributor Author

println!("{:?}", e.flatten());

But thats the debug impl, which contains information about the underling implementation. I don't feel confident exposing that kind of information to the public

would you expect the output to use flatten or maintain the nested structure?

I think the best would be to emulate the output of other validation libraries, such as zod. Flatten is what zod uses

// Deno source code
import { z } from "https://deno.land/x/zod/mod.ts";
const scheme = const scheme = z.object({
	username: z.string(),
	age: z.number().min(18)
})
scheme.safeParse({username: 1, age: 10})
/*
{
  success: false,
  error: ZodError: [
  {
    "code": "invalid_type",
    "expected": "string",
    "received": "number",
    "path": [
      "username"
    ],
    "message": "Expected string, received number"
  },
  {
    "code": "too_small",
    "minimum": 18,
    "type": "number",
    "inclusive": true,
    "exact": false,
    "message": "Number must be greater than or equal to 18",
    "path": [
      "age"
    ]
  }
]
    at handleResult (https://deno.land/x/[email protected]/types.ts:94:19)
    at ZodObject.safeParse (https://deno.land/x/[email protected]/types.ts:231:12)
    at <anonymous>:2:8
}
*/

@jprochazk
Copy link
Owner

jprochazk commented Mar 31, 2023

But thats the debug impl, which contains information about the underling implementation. I don't feel confident exposing that kind of information to the public

That was just an example. flatten returns a Vec<(String, Error)>, and Error implements Display (it just prints the error, e.g. length is lower than 15). You can see an example of the output in snapshots. A possible one-liner to print it line-per-line is:

errors.flatten().into_iter().map(|(p, e)| format!("{p}: {e}")).collect().join('\n')

Does that not work for you?

I think the best would be to emulate the output of other validation libraries

That does look like flattened output. I think this ties into #7 and #3 a bit. Ideally each rule would have a unique (machine-readable) error code, and you'd be able to provide your own, just like you could provide a custom error message.

I don't know if it's best to serialize the constraint parameters (e.g. the minimum: 18 in zod's output) together with the error messages, or leave those separate. I'm more inclined towards the latter.

@Altair-Bueno
Copy link
Contributor Author

Does that not work for you?

It does. Maybe we could add this to the current ToString implementation?

I don't know if it's best to serialize the constraint parameters (e.g. the minimum: 18 in zod's output) together with the error messages, or leave those separate. I'm more inclined towards the latter.

Maybe an enum, like serde_json::Value, that serves as metadata?

[
  {
    // ...
    "metadata": {
       "kind": "length",
       "min": 10
	}
  },
  // ...
]

@jprochazk
Copy link
Owner

As of #28, the Display impl for Errors now uses flatten. The 2nd part (serializing errors) is split off to #3.

@Altair-Bueno
Copy link
Contributor Author

Looks great. Thanks!

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

No branches or pull requests

2 participants