I'm going to vote against exception wrapping.
If I'm reading the implementation correctly, the original exception is
thrown away, there's no way to get the original message and backtrace.
That will make debugging more difficult.
I reviewed about 150 callers of unserialize() in the MediaWiki core
and extensions. There were no instances of the kind of error guarding
shown in the RFC. Occasionally, we have:
$result = @unserialize( $data );
if ( !$result ) {
throw new Exception( ... );
}
The assumption is that if there is a failure, false will be returned,
as is documented in the manual. I don't understand the assertion in
the RFC that set_error_handler() is required to detect unserialization
failure.
Normally, exceptions in MediaWiki are allowed to propagate back to the
global exception handler. There are a couple of categories of
exceptions which should never be caught as a matter of policy:
* Database errors. These are permanent errors. Attempting to persist
with executing the request after a database error will mostly likely
lead to another error.
* Request timeout exceptions. We're using our Excimer extension to
implement request timeouts by throwing exceptions from a timer
callback. This has been an interesting can of worms which has brought
some nice features with a number of unexpected consequences. It means
that it's always incorrect to catch and discard an arbitrary Throwable.
Probably nothing in our ecosystem is doing database access in an
__unserialize() magic method. But if someone tried it, I would want
the resulting errors to be properly logged with correct backtraces,
not silently discarded.
I would vote in favour of such a feature being added as a non-default
option, by analogy with JSON_THROW_ON_ERROR.
--
Tim Starling
Wikimedia Foundation
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
