Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Serializer] fix support for lazy/unset properties#44295
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| try { | ||
| $attributeValue =$this->getAttributeValue($object,$attribute,$format,$context); | ||
| }catch (AccessException$e) { | ||
| if (sprintf('The property "%s::$%s" is not initialized.',\get_class($object),$attribute) ===$e->getMessage()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
rather than detecting the error message (which could easily be broken if we reword the message, especially as there is no tests covering this), shouldn't we use a subclass of the AccessException instead ?
The rule until now is that the exceptionmessages are not covered by any BC rules and should not be relied upon to catch exceptions.
And this is a cross-component dependency, which would require actually changing the BC rules to allow the current pattern
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yes I agree, that's what we've done on 5.3 already, but introducing a new class in 4.4 doesn't look like a good idea to me...
Uh oh!
There was an error while loading.Please reload this page.
This basically backports#43469 into 4.4, which is the way to go tofix#44273.
The code that exists to handle uninitialized properties is broken anyway (it was before the recent changes.)