[Cache][Lock] Fix usages of error_get_last() #27232
Conversation
| if (@!$redis->isConnected()) { | ||
| set_error_handler('var_dump', 0); | ||
| $isConnected = @$redis->isConnected(); | ||
| restore_error_handler(); |
There was a problem hiding this comment.
Symfony often has something similar to this. Would it make sense at add a small abstraction for this?
$isConnected = ErrorFree::handle(function () use ($redis) {
return @$redis->isConnected();
});Advantage is that user-land can benefit from this abstraction as well for libraries or application implementations that need to do something similar. Primarily referring to something similar to: https://github.com/symfony/symfony/pull/27232/files#diff-e88ad8ede40599bea0592b1113c4452cR744
There was a problem hiding this comment.
The issue is where would we put this? There is no global "utils" component, and I'm not sure we want one...
There was a problem hiding this comment.
I would pick the same location (for now) as where the error handlers are located. If I'm not mistaken, this is the debug component. Downside, it becomes a centralized dependency...
There was a problem hiding this comment.
it becomes a centralized dependency
that's the issue, it's not justified imho, the added lines are just fine
There was a problem hiding this comment.
Note that symfony/abstractions might be a place for global and very general functions doing this, but I'm not sure at all right now, it should be discussed separately.
|
Why is it on 3.4 and not 2.7? |
97c7924 to
ff4bb8f
Compare
|
See #27236 for 2.7 ( |
ff4bb8f to
ac4ceb8
Compare
ac4ceb8 to
7904784
Compare
This PR was merged into the 2.7 branch. Discussion ---------- [Filesystem] Fix usages of error_get_last() | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Same as #27232 for 2.7. When a userland error handler doesn't return `false`, `error_get_last()` is not updated, so we cannot see the real last error, but the previous one. See https://3v4l.org/Smmt7 Commits ------- 9d015c7 [Filesystem] Fix usages of error_get_last()
|
Thank you @nicolas-grekas. |
This PR was merged into the 3.4 branch. Discussion ---------- [Cache][Lock] Fix usages of error_get_last() | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - When a userland error handler doesn't return `false`, `error_get_last()` is not updated, so we cannot see the real last error, but the previous one. See https://3v4l.org/Smmt7 Commits ------- 7904784 [Cache][Lock] Fix usages of error_get_last()
When a userland error handler doesn't return
false,error_get_last()is not updated, so we cannot see the real last error, but the previous one.See https://3v4l.org/Smmt7