Skip to content

Stop DebugHandler silencing errors #25234

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Conversation

ostrolucky
Copy link
Contributor

@ostrolucky ostrolucky commented Nov 30, 2017

Q A
Branch? 2.7
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #8703
License MIT
Doc PR

I don't know if this is best fix for the issue, but I want to get things moving here. ErrorHandler silencing errors has been rampant in Symfony for years. Can be reproduced easily, dunno why there isn't more traction in fixing this.

The line I am removing has been introduced in #10921, I didn't find explanation why though.
Will add test case if this approach is approved.

@nicolas-grekas
Copy link
Member

Errors are silenced by default so that no bad info can leak from broken apps.
We have plenty of mechanism that deal with reporting errors to users when appropriate.
Instead, we should fix these.

@ostrolucky
Copy link
Contributor Author

ostrolucky commented Dec 9, 2017

Errors aren't silenced by default, only when Debug component is enabled. Since it's enabled by default in app_dev only, this effectively means it silences errors for developers only. In production, it doesn't do silencing, because it's not enabled. Docs seem to have my back in this:

The Debug component provides tools to ease debugging PHP code.

You should never enable the debug tools in a production environment as they might disclose sensitive information to the user.

So I don't understand what's the issue with this patch, nor why was that line put there in the first place. It's really contraproductive. I want all errors shown when enabling this component. I don't want to be pushed to comment out Debug::enable() line in my index.php each time I get blank page.

We have plenty of mechanism that deal with reporting errors to users when appropriate.
Instead, we should fix these.

Well I thought this is one of them and I am fixing it? You have better fix in mind for cases like I have shown in linked issue?

@nicolas-grekas
Copy link
Member

Here is the fix #25408
Thanks for the reproducer in #8703 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants