-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Debug] Allow throwing from __toString() with return trigger_error($e, E_USER_ERROR);
#15076
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
Conversation
nicolas-grekas
commented
Jun 23, 2015
Q | A |
---|---|
Bug fix? | no |
New feature? | yes |
BC breaks? | no |
Deprecations? | no |
Tests pass? | yes |
Fixed tickets | - |
License | MIT |
Doc PR | - |
a5dfc97
to
3b9ab5d
Compare
When one does |
try { | ||
throw $this->exception; | ||
} catch (\Exception $e) { | ||
return user_error($e, E_USER_ERROR); |
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.
we use trigger_error everywhere else
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.
This is why I use user_error
here: to make sure we do not forget about this alias.
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.
Comment added about that
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.
But then there is no test for trigger_error is there?
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.
That's true...
3b9ab5d
to
15d932c
Compare
The full workaround is the following:
On HHVM, things are much simpler, because throwing from __toString is allowed. When no error handler is plugged, PHP dies with a fatal error (as usual in this situation), and with a message full of details: the exception string contains its type+message+stack trace excerpt. |
It actually is a boolean |
thanks, updated |
@@ -377,7 +378,10 @@ public function handleError($type, $message, $file, $line, array $context, array | |||
} | |||
|
|||
if ($throw) { | |||
if (($this->scopedErrors & $type) && class_exists('Symfony\Component\Debug\Exception\ContextErrorException')) { | |||
if (isset(self::$toStringException)) { |
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.
normally we do null !== self::$toStringException
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.
updated
15d932c
to
8dfd416
Compare
How did you find out about this hack? Never seen it before. We also had to add stupid workarounds in PSR-7 StreamInterface and UriInterface since throwing an exception in toString is not allowed in PHP. |
I don't remember exactly if I discovered this hack by myself or not, but I used it a few years ago in my own (now history) framework (nicolas-grekas/Patchwork@0f1f1fd) |
What if I just want to create an exception without catching it first. Something like the following which I guess cannot work. This is why I thought a helper method might be useful, which throws the exception, catches it again and then calls trigger error.
Or do you get a stack trace with a trigger_error anyway?
|
If you do not follow the convention (which is the case in both your examples), then you will end up in the which will handle it as uncaught exception, then kill the process (by returning false). |
…e, E_USER_ERROR);`
8dfd416
to
f360758
Compare
👍 |
1 similar comment
👍 |
…trigger_error($e, E_USER_ERROR);` (nicolas-grekas) This PR was merged into the 2.8 branch. Discussion ---------- [Debug] Allow throwing from __toString() with `return trigger_error($e, E_USER_ERROR);` | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Commits ------- f360758 [Debug] Allow throwing from __toString() with `return trigger_error($e, E_USER_ERROR);`
…_toString() (nicolas-grekas) This PR was merged into the 2.3 branch. Discussion ---------- [HttpFoundation] Use convention to allow throwing from __toString() | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Corollary to #15076, works without it. Commits ------- 8982c32 [HttpFoundation] Use convention to allow throwing from __toString()