Base exception interface does not extend Throwable

[alcaeus]

Description

The base exception interface used in the driver (MongoDB\Driver\Exception\Exception) does not extend the Throwable or Exception base interfaces. This can be problematic; in our case, Doctrine MongoDB ODM documented a @throws \MongoDB\Driver\Exception\Exception on a class, which is then caught by static analysis with PHPStan:

PHPDoc tag @throws with type MongoDB\Driver\Exception\Exception is not subtype of Throwable

Environment

ext-mongodb version: 1.5.2

[jmikola]

Based on https://3v4l.org/NiH5O, it looks like it's kosher for an interface to extend Throwable so long as a class does not attempt to implement it directly. Nevertheless, this would require special logic so that Throwable is only extended for PHP 7.

I'm pretty sure I borrowed the idea of a base library exception interface for other projects (I don't recall offhand). Assuming you've also seen this design pattern before, do you know how they handle this? It may be trivial for a PHP 7+ library to extend Throwable directly, but conditionally doing so based on the PHP version (or polyfilling Throwable if undefined for PHP 5.x) seems a bit clunky.

[alcaeus]

To be honest, I had to check the PHP docs to realise that \Exception is not an interface - that complicates matters greatly. I'm not sure there is a way to solve this problem while maintaining compatibility with PHP 5.x 😐

[jmikola]

We may be able to do something like following in Exception.c:

 void php_phongo_exception_init_ce(INIT_FUNC_ARGS) /* {{{ */
 {
 	zend_class_entry ce;

 	INIT_NS_CLASS_ENTRY(ce, "MongoDB\\Driver\\Exception", "Exception", php_phongo_exception_me);
 	php_phongo_exception_ce = zend_register_internal_interface(&ce TSRMLS_CC);
+#if PHP_VERSION_ID >= 70000
+       zend_class_implements(php_phongo_exception_ce TSRMLS_CC, 1, zend_ce_throwable);
+#endif
 } /* }}} */

If you want to try this out locally and see if it works for your use case, I think we can consider the PR with @derickr's input. My only concern is that a class in PHP 7.x could implement this interface (since it really has no API) without also extending (directly or indirectly) the Exception or Error base classes. I'd want to be sure that PHP still catches that so we don't introduce some loophole for userland classes to circumvent that restriction.

[derickr]

I will need to try that out, but can you clarify what exactly "that restriction" refers to?

[jmikola]

"that restriction" refers to PHP 7 not allowing classes to implement Throwable without also extending Exception or Error (either directly or indirectly through an inheritance chain). See: https://3v4l.org/cSkq0

AFAICT, this is on par with existing restrictions in 5.x for implementing Traversable without also extending Iterator or IteratorAggregate.

[jmikola]

@alcaeus I created PHPC-1307 to track this, so I'll close this issue. Feel free to follow JIRA for updates.