Skip to content

[Debug] Always decorate existing exception handlers to deal with fatal errors #25829

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
Merged
merged 1 commit into from
Jan 18, 2018
Merged

[Debug] Always decorate existing exception handlers to deal with fatal errors #25829

merged 1 commit into from
Jan 18, 2018

Conversation

nicolas-grekas
Copy link
Member

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

Decorating the exception is seamless, let's always do it and fix handling of fatal errors.
Related to #25408 also.

@nicolas-grekas nicolas-grekas added this to the 2.7 milestone Jan 18, 2018
@linaori
Copy link
Contributor

linaori commented Jan 18, 2018

Verified the fix locally by doing set_error_handler('var_dump'); in app.php, seems to work now 👍

@linaori
Copy link
Contributor

linaori commented Jan 18, 2018

Small update: set_exception_handler('var_dump'); gives me

/home/ivanderberg/projects/mijn/www/vendor/symfony/debug/ErrorHandler.php:541:
object(Symfony\Component\Debug\Exception\FatalErrorException)[3806]
  protected 'message' => string 'Error: Uncaught ArgumentCountError: Too few arguments to function Symfony\Component\Debug\ErrorHandler::handleError(), 1 passed and exactly 4 expected in /home/ivanderberg/projects/mijn/www/vendor/symfony/debug/ErrorHandler.php:369
Stack trace:
#0 [internal function]: Symfony\Component\Debug\ErrorHandler->handleError(Object(TypeError))
#1 {main}
  thrown' (length=356)
  private 'string' (Exception) => string '' (length=0)
  protected 'code' => int 0
  protected 'file' => string '/home/ivanderberg/projects/mijn/www/vendor/symfony/debug/ErrorHandler.php' (length=73)
  protected 'line' => int 369
  private 'trace' (Exception) => 
    array (size=0)
      empty
  private 'previous' (Exception) => null
  protected 'severity' => int 1

@nicolas-grekas
Copy link
Member Author

Fixed thanks!

@linaori
Copy link
Contributor

linaori commented Jan 18, 2018

@fabpot note that the current state does not work with set_exception_handler('var_dump');.

@fabpot
Copy link
Member

fabpot commented Jan 18, 2018

Thank you @nicolas-grekas.

@fabpot fabpot merged commit 205d7ae into symfony:2.7 Jan 18, 2018
fabpot added a commit that referenced this pull request Jan 18, 2018
@fabpot
bug #25829 [Debug] Always decorate existing exception handlers to dea…
78a8a63 
…l with fatal errors (nicolas-grekas)

This PR was merged into the 2.7 branch.

Discussion
----------

[Debug] Always decorate existing exception handlers to deal with fatal errors

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

Decorating the exception is seamless, let's always do it and fix handling of fatal errors.
Related to #25408 also.

Commits
-------

205d7ae [Debug] Always decorate existing exception handlers to deal with fatal errors
nicolas-grekas added a commit to nicolas-grekas/symfony that referenced this pull request Jan 18, 2018
@nicolas-grekas
Revert "bug symfony#25829 [Debug] Always decorate existing exception …
b6fc69e 
…handlers to deal with fatal errors (nicolas-grekas)"

This reverts commit 78a8a63, reversing
changes made to 5d39415.
This was referenced Jan 29, 2018
Merged
Merged
Merged
Merged
Merged
@nanasess nanasess mentioned this pull request Jan 30, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

Assignees
No one assigned
Labels
Bug Debug Status: Reviewed
Projects
None yet
Milestone
2.7
Development

Successfully merging this pull request may close these issues.
4 participants
@nicolas-grekas @linaori @fabpot @carsonbot