Improve errors

[sanders41]

Closes #267

add new PydanticErrorKind(kind: str, **context: int | str): which uses an existing error kind

For this part I wasn't clear on what is needed. Does this literally mean create a new error type called PydanticErrorKind? And for "use an existing error kind" I assume you have a specific one to use in mind?

[samuelcolvin]

See https://github.com/pydantic/pydantic-core/blob/main/src/errors/kinds.rs

PydanticErrorKind should be mapped to this enum, both for performance and so the message is reused.

[samuelcolvin]

Thanks so much for this, hope that helps. If you have more questions, LMK.

I'll be back at a computer on Monday so can explain more fully then if they're still not clear.

[sanders41]

I had a go at adding it. I have a feeling it is either in pretty good shape or couldn't be further from correct, I'm just not sure which it is 😄

Either way I'm happy to make any updates.

[codecov-commenter]

Codecov Report

Merging #270 (e74d21d) into main (837a6ce) will decrease coverage by 0.80%.
The diff coverage is 13.72%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #270      +/-   ##
==========================================
- Coverage   98.61%   97.80%   -0.81%     
==========================================
  Files          51       51              
  Lines        5342     5388      +46     
  Branches       39       39              
==========================================
+ Hits         5268     5270       +2     
- Misses         74      118      +44     

Impacted Files	Coverage Δ
src/errors/mod.rs	92.30% <ø> (ø)
src/errors/value_exception.rs	100.00% <ø> (ø)
src/errors/kinds.rs	72.56% <2.22%> (-26.60%)	⬇️
src/lib.rs	100.00% <100.00%> (ø)
src/validators/float.rs	98.90% <100.00%> (ø)
src/validators/function.rs	99.42% <100.00%> (ø)
src/validators/union.rs	98.80% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 837a6ce...e74d21d. Read the comment docs.

[samuelcolvin]

Tests need adding too.

Let me know if you need help with the remaining bits?

[samuelcolvin]

Suggested change

	contect: 'dict[str, str | int] | None'
	context: 'dict[str, str | int] | None'

also needs an __init__ method.

[samuelcolvin]

This should be moved to value_exception.rs.

[samuelcolvin]

Suggested change

	message_template: String,
	message_template: String,

This isn't required.

The whole idea of PydanticErrorKind is that message_template is taken from ErrorKind.

[samuelcolvin]

This is what I meant, a bit of macro fu required, but otherwise I think quite clear.

@sanders41 are you okay finishing this off, or would you like me to?
Needs:

• conflicts to be resolved
• tests for PydanticErrorKind
• other comments above

[sanders41]

Thanks for getting me unstuck here! Seeing what you did it makes sense now. I can work on finishing off the rest of it.

[sanders41]

Sorry for the delay with getting back to this.

One thing I noticed in testing is when an ErrorKind has multiple kinds that all serialize to the same thing, too_small for example, PydanticErrorKind always takes the last one and never uses any others. This seems like a bug with ErrorKind, but maybe there is a different way to handle the kinds?

Also when trying to merge main to resolve the conflicts it looks like PydanticCustomError has been renamed to PydanticValueError? In my branch on import shows up as a conflict, but the rest it keeps PydanticCustomError so I want to make sure I resolve it correctly.

[samuelcolvin]

I think that's fine, we should give every error itsy own kind anyway which would solve the problem. I'll create an issue.

[samuelcolvin]

@sanders41 any idea when this might be completed? I'll wait to merge #282 until this is merged to avoid more conflicts.

[sanders41]

I was waiting to hear back about resolving the merge conflict. I have one conflict on an import of PydanticCustomError where it looks like it changed to PydanticValueError, but everywhere else in the code on my branch it stayed PydanticCustomError. I think it should change to PydanticValueError everywhere?

[samuelcolvin]

It's generally up to the PR author to resolve conflicts. In this case PydanticValueError should be renamed to PydanticCustomError everywhere.

[sanders41]

Yes, I planned to resolve it. I just wanted to make sure I resolved it in the correct direction because looking at main it looked like it could be possibly the other way around 😄 . I can resolve this today.

[samuelcolvin]

great, thanks so much.

[sanders41]

Ha, I'm an idiot...of course that is the way to resolve it, I renamed PydanticValueError as part of the PR!!! Sorry about that.

[samuelcolvin]

looking good, we need some tests of using PydanticErrorKind inside validation functions and confirming the correct ValidationError is raised.

[samuelcolvin]

thanks so much for this.