Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[PropertyAccess] Do not silence TypeErrors from client code.#23100
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
[PropertyAccess] Do not silence TypeErrors from client code.#23100
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| self::throwInvalidArgumentException($e->getMessage(),$e->getTrace(),0); | ||
| // It wasn't thrown in this class so rethrow it | ||
| throw$e; |
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.
shouldn't be this done inthrowInvalidArgumentException instead?
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.
no, because it is used elsewhere too
nicolas-grekasJun 9, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
shouldn't the other place also throw?
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.
the other place is error handler callback for PHP 5, there is no exception to rethrow, and errors are passed correctly to the original handler (here)
fabpot commentedJun 14, 2017
Thank you@tsufeki. |
…e. (tsufeki)This PR was merged into the 3.2 branch.Discussion----------[PropertyAccess] Do not silence TypeErrors from client code.| Q | A| ------------- | ---| Branch? | 3.2| Bug fix? | yes| New feature? | no <!-- don't forget updating src/**/CHANGELOG.md files -->| BC breaks? | no| Deprecations? | no <!-- don't forget updating UPGRADE-*.md files -->| Tests pass? | yes| Fixed tickets || License | MIT| Doc PR |Fixes TypeError silencing in `setValue()` when said error is thrown inside setter/adder/etc.An example is given in the included test, but more real-life story is botched accessors for a many-to-one association on a Doctrine entity:```phpclass B { function setA(A $a) { ... } // forgotten "= null" here}class A { function removeB(B $b) { if ($this->bs->contains($b)) { $this->bs->removeElement($b); $b->setA(null); // TypeError thrown } return $this; }}```No error is shown to the user, even though removing doesn't work.This bug is not present in 2.7 & 2.8.Commits-------45b961d [PropertyAccess] Do not silence TypeErrors from client code.tsufeki commentedJun 14, 2017
@fabpot I think |
Fixes TypeError silencing in
setValue()when said error is thrown inside setter/adder/etc.An example is given in the included test, but more real-life story is botched accessors for a many-to-one association on a Doctrine entity:
No error is shown to the user, even though removing doesn't work.
This bug is not present in 2.7 & 2.8.