search

Crell / Serde

Robust Serde (serialization/deserialization) library for PHP 8.
Other
299 stars 14 forks source link

Field path on deserialization error #13

Open repli2dev opened 1 year ago

repli2dev commented 1 year ago

When error occures during deserialization the field path could be present to use in API error response.

Detailed description and context

For programmer centric experience when using deserialization as a part of API endpoint validation the resulting error should contain a field path and basic error description and error type to be passed back to the caller, so that he can react and solve the issue.

(Or to be unified with like symfony validator and on UI level shown to fields of the form.)

Possible implementation

N/A

Your environment

Crell commented 1 year ago

I don't disagree, but given the way Serde is built, I'm not really sure how to even build that path.

Crell commented 12 months ago

Random thought for the future: Could the exception backtrace be introspected to dig out the chain of Field objects? Maybe, not sure.

Kingdutch commented 5 months ago

We ran across this need as well. If I look at how Serde works then it seems that this chain has been added as $parentField to populateObject in https://github.com/Crell/Serde/pull/48?

So that would now make it possible to provide that to the InvalidArrayKeyType that might be thrown and to $this->handleDefault for its use in MissingRequiredValueWhenDeserializing :D

I'd be happy to provide a pull request to make that work, but I think I'd have 2 questions:

  1. Can I simply add the $parentField as field to the exceptions that are thrown?
  2. Is it okay to add this field to handleDefault just for error handling? Any problems in terms of BC with that being a protected field?
Crell commented 4 months ago

$parentField doesn't include a full ancestry, though. Only the immediate parent. I suppose that might be enough in some cases, (show "field Foo in object Bar" or something), so if you want to have a go at it, I will review.

To your questions:

  1. I think it would need to be optional, but yes. Follow the pattern of the existing properties. It may also make sense to update the message strings accordingly.
  2. I... think that would be OK? No one should be extending that method themselves so I don't think it's a BC break.