[Debug] Allow throwing from __toString() with `return trigger_error($e, E_USER_ERROR);` #15076

nicolas-grekas · 2015-06-23T11:33:17Z

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

nicolas-grekas · 2015-06-23T12:45:35Z

When one does return trigger_error($exception, E_USER_ERROR); from a __toString() method,
it is possible to convert the call to make it do the same as throw $exception;, which is otherwise forbidden from happening in __toString in PHP.
This convention plays nice with any error handler (even the native one). And when a compliant error handler is used (like ours if this PR gets merged) it can be used to throw from __toString().

Tobion · 2015-06-23T12:50:01Z

src/Symfony/Component/Debug/Tests/Fixtures/ToStringThrower.php

+        try {
+            throw $this->exception;
+        } catch (\Exception $e) {
+            return user_error($e, E_USER_ERROR);


we use trigger_error everywhere else

This is why I use user_error here: to make sure we do not forget about this alias.

Comment added about that

But then there is no test for trigger_error is there?

That's true...

nicolas-grekas · 2015-06-23T13:14:33Z

The full workaround is the following:

return trigger_error($e, E_USER_ERROR); triggers the error handler
$e is casted to string automatically and given to the error handler as $message
the error handler looks up for the original exception in its $context variable, searching for a matching index whose value is an exception whose string representation is exactly $message
the matched $e exception is temporarily stored in a private $toStringException static property
the return value of trigger_error is always a boolean, so we exit __toString() by returning this boolean
which triggers an E_RECOVERABLE_ERROR, because this is the behavior of __toString() when a not-string value is returned
which calls the error handler again
that checks the $toStringException property and throws it (yes, it is allowed at this point).

On HHVM, things are much simpler, because throwing from __toString is allowed.
The behavior ends up being the same (and that's the reason why the provided test case passes on HHVM also).

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.

Tobion · 2015-06-23T13:47:43Z

the return value of trigger_error is always null

It actually is a boolean

nicolas-grekas · 2015-06-23T13:49:40Z

It actually is a boolean

thanks, updated

Tobion · 2015-06-23T13:49:40Z

src/Symfony/Component/Debug/ErrorHandler.php

@@ -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)) {


normally we do null !== self::$toStringException

Tobion · 2015-06-23T14:09:54Z

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.

nicolas-grekas · 2015-06-23T14:23:56Z

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)

Tobion · 2015-06-23T14:59:19Z

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.

class Uri {
    public function __toString() {
        return trigger_error(new \LogicException('Given URI components cannot be represented as URI reference'), E_USER_ERROR);
    }
}

Or do you get a stack trace with a trigger_error anyway?

    public function __toString() {
        return trigger_error('Given URI components cannot be represented as URI reference', E_USER_ERROR);
    }

nicolas-grekas · 2015-06-23T15:09:02Z

nicolas-grekas commented

Jun 23, 2015

If you do not follow the convention (which is the case in both your examples), then you will end up in the if after the foreach:
https://github.com/symfony/symfony/pull/15076/files#diff-e7d086f2c2086fdb8f108e74f11dac5bR429

which will handle it as uncaught exception, then kill the process (by returning false).
We have no better choice here since throwing would also kill the process with a fatal error.

…e, E_USER_ERROR);`

fabpot · 2015-06-25T08:49:37Z

👍

stof · 2015-06-25T09:01:00Z

👍

…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()

nicolas-grekas mentioned this pull request Jun 23, 2015

[HttpFoundation] Use convention to allow throwing from __toString() #15077

Merged

nicolas-grekas force-pushed the debug-tostring branch from a5dfc97 to 3b9ab5d Compare June 23, 2015 12:11

Tobion reviewed Jun 23, 2015
View reviewed changes

nicolas-grekas force-pushed the debug-tostring branch from 3b9ab5d to 15d932c Compare June 23, 2015 12:57

nicolas-grekas added the Ready label Jun 23, 2015

Tobion reviewed Jun 23, 2015
View reviewed changes

nicolas-grekas force-pushed the debug-tostring branch from 15d932c to 8dfd416 Compare June 23, 2015 13:50

[Debug] Allow throwing from __toString() with `return trigger_error($…

f360758

…e, E_USER_ERROR);`

nicolas-grekas force-pushed the debug-tostring branch from 8dfd416 to f360758 Compare June 23, 2015 18:38

nicolas-grekas merged commit f360758 into symfony:2.8 Jun 25, 2015

nicolas-grekas deleted the debug-tostring branch June 25, 2015 09:03

fabpot mentioned this pull request Nov 16, 2015

Release v2.8.0-BETA1 #16550

Merged

Tobion mentioned this pull request Dec 9, 2015

[PSR-7] Adding a Errata section to PSR-7 to clarify UriInterface modification mechanism php-fig/fig-standards#619

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[Debug] Allow throwing from __toString() with `return trigger_error($e, E_USER_ERROR);` #15076

[Debug] Allow throwing from __toString() with `return trigger_error($e, E_USER_ERROR);` #15076

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

[Debug] Allow throwing from __toString() with return trigger_error($e, E_USER_ERROR); #15076

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

[Debug] Allow throwing from __toString() with `return trigger_error($e, E_USER_ERROR);` #15076