8000 [PropertyAccessor] remove `try/catch` or provide context into re-thrown exception · Issue #34899 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[PropertyAccessor] remove try/catch or provide context into re-thrown exception #34899

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.

Sign up for GitHub

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

Jump to bottom
Closed
Pictor13 opened this issue Dec 9, 2019 · 1 comment
Closed

[PropertyAccessor] remove try/catch or provide context into re-thrown exception #34899

Pictor13 opened this issue Dec 9, 2019 · 1 comment
Labels
DX DX = Developer eXperience (anything that improves the experience of using Symfony) PropertyAccess

Comments

@Pictor13
Copy link
Pictor13 commented Dec 9, 2019

I just had some debugging problem, when the PropertyAccessor is called from the Form component.

Specifically, the call to PropertyAccessor::writeProperty() is wrapped in a try/catch block and, in case of exception, it is re-thrown manually by the static method throwInvalidArgumentException().

Except for providing a namespaced custom exception, I am not sure why the try/catch is there, since I suppose that anybody would want the original error to be reported, when trying to set a property on target object/array.

In fact, re-throwing hinders the discovery of the actual problem, breaking the call-stack at the setValue () call instead of reporting what went wrong. Not really debuggable.
This is a screenshot from my specific case:

Screenshot 2019-12-09 at 19 10 40

"Expected argument of type "%s", "%s" given at property path "%s".

...after which I am left with guess work.
The re-thrown error message was too specific and not providing the context (File + Line where the real exception happened).

I am not sure about the benefits of re-throwing here.
So I am asking if it is possible to remove the try/catch.
Or eventually to change the re-throwing to at least report all the information provided from the original exception, rather than hiding them (atm the message doesn't help if we don't get the info of where this is happening).

What do you think about that?
Does it make sense?
@xabbuh xabbuh added DX DX = Developer eXperience (anything that improves the experience of using Symfony) PropertyAccess labels Dec 9, 2019
@xabbuh xabbuh mentioned this issue Dec 9, 2019
Merged
nicolas-grekas added a commit that referenced this issue Dec 10, 2019
@nicolas-grekas
bug #34902 [PropertyAccess] forward caught exception (xabbuh)
56bfbd8 
This PR was merged into the 3.4 branch.

Discussion
----------

[PropertyAccess] forward caught exception

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #34899
| License       | MIT
| Doc PR        |

Commits
-------

98e18d3 forward caught exception
@Pictor13
Copy link
Author

Glad it made sense; am sure it will help others.
Thank you for fixing it so quickly! 💚

@thelem thelem mentioned this issue Oct 18, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
DX DX = Developer eXperience (anything that improves the experience of using Symfony) PropertyAccess
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@nicolas-grekas @xabbuh @Pictor13
0