Catch Throwable in order to support both PHP 5 and PHP 7+ #82
Conversation
|
👋 Hi there! I was just wondering if there was anything blocking this, other than limited resources, I guess...? Being able to use this plugin on PHP 8+ without issues (related to the things addressed in this PR) sounds like a good thing, no? 🙂 |
| } catch ( \Throwable $e ) { | ||
| /* translators: %s = error message */ | ||
| return new \WP_Error( 'invalid-saml', sprintf( esc_html__( 'Error: Could not parse the authentication response, please forward this error to your administrator: "%s"', 'wp-simple-saml' ), esc_html( $e->getMessage() ) ) ); | ||
| } catch ( \Exception $e ) { | ||
| /* translators: %s = error message */ | ||
| return new \WP_Error( 'invalid-saml', sprintf( esc_html__( 'Error: Could not parse the authentication response, please forward this error to your administrator: "%s"', 'wp-simple-saml' ), esc_html( $e->getMessage() ) ) ); |
There was a problem hiding this comment.
@tfrommen My understanding of this is that in order to be caught the exception must be of type throwable or it would not work, in which case this catch will be triggered for all exceptions 🤔
There was a problem hiding this comment.
Thanks for merging this, Tom!
Just to follow up on your question: The idea of this code is to catch any and all things PHP may throw, in any old or current version. Overall, we may be looking at \Exception instances, which is what everyone knows, but also \Error instances, or custom implementations of \Throwable. Before this PR, the plugin only caught \Exception, but that does not cover \Error instances, that newer version of OneLogin throw. On newer versions of PHP, both is covered by \Throwable. But older versions do not know about that, so we need to still have the \Exception catch in place.
Irrelevant of the PHP version, only one catch block will execute, both they are both doing the same, of course.
Hope that helps.
This PR updates the three existing
try—catchblocks to try and catch aThrowablefirst (which will work in PHP 7+) before the current code targeting anExceptionis executed.This works on PHP 5-8 (see here), and allows all the
Errorinstances thrown by OneLogin to be caught on PHP 7 or 8 (e.g., #78).