PHPLIB-636: ListIndexes should use CommandException for catching missing namespace and database errors #837
Conversation
…ing namespace and database errors
src/Operation/ListIndexes.php
Outdated
| @@ -123,7 +123,7 @@ private function createOptions() | |||
| * | |||
| * @param Server $server | |||
| * @return IndexInfoIteratorIterator | |||
| * @throws DriverRuntimeException for other driver errors (e.g. connection errors) | |||
| * @throws CommandException for other driver errors (e.g. connection errors) | |||
There was a problem hiding this comment.
These two instances of @throws should not have been modified. Did you think to do this because of a phpcs warning that DriverRuntimeException was undefined? IIRC, it does attempt to check class references in doc block comments.
Remember that ServerException and its child classes are defined as originating from a server response. Something like a connection error (mentioned on this very line) can still be thrown by Server::executeReadCommand() and ListIndexes::executeCommand() will not catch it at all and allow it to propagate up the chain.
See Errors/Exceptions in ServerException::executeReadCommand() for a list of possible exceptions that the method might throw. Note that it doesn't mention ServerException directly, but that's covered by RuntimeException.
On a separate note, if these lines were actually supposed to be modified, we would also want to update the documentation for Errors/Exceptions in Collection::listIndexes(), the source of which is in docs/reference/method/MongoDBCollection-listIndexes.txt.
There was a problem hiding this comment.
Ah I got confused and changed it while I was changing the DriverRuntimeException to CommandException, but now I see I shouldn't have modified it in the @throws. Sorry, it makes sense to me now!
| @@ -19,7 +19,7 @@ | |||
|
|
|||
| use EmptyIterator; | |||
| use MongoDB\Driver\Command; | |||
| use MongoDB\Driver\Exception\RuntimeException as DriverRuntimeException; | |||
There was a problem hiding this comment.
Related to my comment below, I think we may actually need to leave this line in place to satisfy the reference in @throws (phpcs might complain otherwise, even though it's just in a comment). So CommandException should probably be added as a new line.
https://jira.mongodb.org/browse/PHPLIB-636