Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[DX] Show the ParseException message in all YAML file loaders#36501
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
[DX] Show the ParseException message in all YAML file loaders#36501
Uh oh!
There was an error while loading.Please reload this page.
Conversation
nicolas-grekas commentedApr 21, 2020
as@stof spotted, all these should be moved outside sprintf() |
nicolas-grekas commentedApr 21, 2020
(but this is for master) |
javiereguiluz commentedApr 22, 2020
This is great! Thanks a lot@fancyweb! I tested it in the Symfony Demo app and the difference is huge: Before After |
fabpot commentedApr 22, 2020
Idea: instead of selectively add the previous exception message to the current exception message, what about doing something similar to some Go error handling libraries where the end message of an error is always the concatenation of all error messages. That's something we could do only in the exception page/profiler. |
fancyweb commentedApr 22, 2020
You mean concatenating the exception message after the sprintf? |
nicolas-grekas commentedApr 22, 2020 • 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.
@fancyweb yes - we should also fix existing messages. |
d57b602 tofc6cf3dComparefancyweb commentedApr 23, 2020
I fixed the existing message in the DI YamlFileLoader. Should I apply fabbot exception message formatting here, ie get rid of the |
nicolas-grekas commentedApr 26, 2020 • 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.
Showing all stacked messages in a more prominent way is absolutely required I agree. The error page currently is not clear enough in this regard (I recently stayed stuck on an exception because I missed the 2nd one which provided the real issue.) Doing both this change and improving the error page looks like what we should do IMHO. The reason is that we don't have full control over all situations where error messages are used; e.g. exceptions in logs, etc. Still for master on my side - at least that's the policy we've had in the past: that's improving something. |
javiereguiluz commentedMay 3, 2020
I agree with@nicolas-grekas about the possible improvements in the error page. We'll discuss about this in the coming weeks to have it ready for Symfony 5.2 👍 |
nicolas-grekas commentedMay 4, 2020
Thank you@fancyweb. |


Uh oh!
There was an error while loading.Please reload this page.
This PR synchronizes the exception message in the Routing, Validator and Translation YAML file loaders with the DependencyInjection YAML file loader behavior. Adding the ParseException message is a big DX gain because it highlights the problem directly instead of having to scroll down 7 previous exceptions.
I'm targetting 3.4 because DX can be considered as a bug fix AFAIK.