-
Notifications
You must be signed in to change notification settings - Fork 266
PHPLIB-1689 Make the Atlas Search exception more specific #1794
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v2.x
Are you sure you want to change the base?
Changes from all commits
9589a94
2373027
f14f4a8
b51d375
7968831
6c73f9d
caf4013
1348e94
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| <?php | ||
|
|
||
| namespace MongoDB\Exception; | ||
|
|
||
| use MongoDB\Driver\Exception\ServerException; | ||
| use Throwable; | ||
|
|
||
| final class SearchNotSupportedException extends ServerException | ||
| { | ||
| /** @internal */ | ||
| public static function create(ServerException $e): self | ||
| { | ||
| $message = $e->getCode() === 31082 | ||
| ? $e->getMessage() | ||
| : 'Using Atlas Search Database Commands and the $listSearchIndexes aggregation stage requires additional configuration. ' | ||
| . 'Please connect to Atlas or an AtlasCLI local deployment to enable. ' | ||
| . 'For more information on how to connect, see https://dochub.mongodb.org/core/atlas-cli-deploy-local-reqs'; | ||
|
|
||
| return new self($message, $e->getCode(), $e); | ||
| } | ||
|
|
||
| /** @internal */ | ||
| public static function isSearchNotSupportedError(Throwable $exception): bool | ||
| { | ||
| if (! $exception instanceof ServerException) { | ||
| return false; | ||
| } | ||
|
|
||
| return match ($exception->getCode()) { | ||
| // MongoDB 8: Using Atlas Search Database Commands and the $listSearchIndexes aggregation stage requires additional configuration. | ||
| 31082 => true, | ||
| // MongoDB 7: $listSearchIndexes stage is only allowed on MongoDB Atlas | ||
| 6047401 => true, | ||
| // MongoDB 7-ent: Search index commands are only supported with Atlas. | ||
| 115 => true, | ||
| // MongoDB 4 to 6, 7-community | ||
| 59 => match ($exception->getMessage()) { | ||
| 'no such command: \'createSearchIndexes\'' => true, | ||
| 'no such command: \'updateSearchIndex\'' => true, | ||
| 'no such command: \'dropSearchIndex\'' => true, | ||
| default => false, | ||
| }, | ||
| // MongoDB 4 to 6 | ||
| 40324 => match ($exception->getMessage()) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you trawl through all the old versions' source code to find all these error codes? 😭
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried many server versions using mongo-orchestration, m and mongo-launch; Atlas M0 and Atlas Local Docker. |
||
| 'Unrecognized pipeline stage name: \'$listSearchIndexes\'' => true, | ||
| 'Unrecognized pipeline stage name: \'$search\'' => true, | ||
| 'Unrecognized pipeline stage name: \'$searchMeta\'' => true, | ||
| 'Unrecognized pipeline stage name: \'$vectorSearch\'' => true, | ||
| default => false, | ||
| }, | ||
| // Not a Search error | ||
| default => false, | ||
| }; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,12 +21,14 @@ | |
| use MongoDB\Driver\Command; | ||
| use MongoDB\Driver\CursorInterface; | ||
| use MongoDB\Driver\Exception\RuntimeException as DriverRuntimeException; | ||
| use MongoDB\Driver\Exception\ServerException; | ||
| use MongoDB\Driver\ReadConcern; | ||
| use MongoDB\Driver\ReadPreference; | ||
| use MongoDB\Driver\Server; | ||
| use MongoDB\Driver\Session; | ||
| use MongoDB\Driver\WriteConcern; | ||
| use MongoDB\Exception\InvalidArgumentException; | ||
| use MongoDB\Exception\SearchNotSupportedException; | ||
| use MongoDB\Exception\UnexpectedValueException; | ||
| use MongoDB\Exception\UnsupportedException; | ||
| use MongoDB\Model\CodecCursor; | ||
|
|
@@ -233,7 +235,15 @@ public function execute(Server $server): CursorInterface | |
| $this->createCommandOptions(), | ||
| ); | ||
|
|
||
| $cursor = $this->executeCommand($server, $command); | ||
| try { | ||
| $cursor = $this->executeCommand($server, $command); | ||
| } catch (ServerException $exception) { | ||
| if (SearchNotSupportedException::isSearchNotSupportedError($exception)) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice solution |
||
| throw SearchNotSupportedException::create($exception); | ||
| } | ||
GromNaN marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| throw $exception; | ||
| } | ||
|
|
||
| if (isset($this->options['codec'])) { | ||
| return CodecCursor::fromCursor($cursor, $this->options['codec']); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| <?php | ||
|
|
||
| namespace MongoDB\Tests\Exception; | ||
|
|
||
| use MongoDB\Driver\Command; | ||
| use MongoDB\Driver\Exception\ServerException; | ||
| use MongoDB\Exception\SearchNotSupportedException; | ||
| use MongoDB\Tests\Collection\FunctionalTestCase; | ||
| use PHPUnit\Framework\Attributes\DoesNotPerformAssertions; | ||
|
|
||
| class SearchNotSupportedExceptionTest extends FunctionalTestCase | ||
| { | ||
| #[DoesNotPerformAssertions] | ||
| public function testListSearchIndexesNotSupportedException(): void | ||
| { | ||
| $collection = $this->createCollection($this->getDatabaseName(), $this->getCollectionName()); | ||
|
|
||
| try { | ||
| $collection->listSearchIndexes(); | ||
| } catch (SearchNotSupportedException) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Basically are you doing it like these because it's too complex to skip if it is supported? Can't we just do the inverse of the check you do in
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a way to test even if I noticed that not all server versions are consistent. As an example in mongo |
||
| // If an exception is thrown because Atlas Search is not supported, | ||
| // then the test is successful because it has the correct exception class. | ||
| } | ||
| } | ||
|
|
||
| #[DoesNotPerformAssertions] | ||
| public function testSearchStageNotSupportedException(): void | ||
| { | ||
| // The server returns an empty result if the search index does not exist. | ||
| // We don't need to create a search index for this test. | ||
| $collection = $this->createCollection($this->getDatabaseName(), $this->getCollectionName()); | ||
|
|
||
| try { | ||
| $collection->aggregate([ | ||
| ['$search' => ['index' => 'default', 'text' => ['query' => 'test', 'path' => 'field']]], | ||
| ]); | ||
| } catch (SearchNotSupportedException) { | ||
| // If an exception is thrown because Atlas Search is not supported, | ||
| // then the test is successful because it has the correct exception class. | ||
| } | ||
|
|
||
| try { | ||
| $collection->aggregate([ | ||
| ['$vectorSearch' => ['index' => 'default', 'queryVector' => [0.1, 0.2, 0.3], 'path' => 'embedding', 'numCandidates' => 5, 'limit' => 5]], | ||
| ]); | ||
| } catch (SearchNotSupportedException) { | ||
| // If an exception is thrown because Atlas Search is not supported, | ||
| // then the test is successful because it has the correct exception class. | ||
| } | ||
| } | ||
|
|
||
| #[DoesNotPerformAssertions] | ||
| public function testSearchIndexManagementNotSupportedException(): void | ||
| { | ||
| $collection = $this->createCollection($this->getDatabaseName(), $this->getCollectionName()); | ||
|
|
||
| try { | ||
| $collection->createSearchIndex(['mappings' => ['dynamic' => false]], ['name' => 'test-search-index']); | ||
| } catch (SearchNotSupportedException) { | ||
| // If an exception is thrown because Atlas Search is not supported, | ||
| // then the test is successful because it has the correct exception class. | ||
| } | ||
|
|
||
| try { | ||
| $collection->updateSearchIndex('test-search-index', ['mappings' => ['dynamic' => true]]); | ||
| } catch (SearchNotSupportedException) { | ||
| // If an exception is thrown because Atlas Search is not supported, | ||
| // then the test is successful because it has the correct exception class. | ||
| } | ||
|
|
||
| try { | ||
| $collection->dropSearchIndex('test-search-index'); | ||
| } catch (SearchNotSupportedException) { | ||
| // If an exception is thrown because Atlas Search is not supported, | ||
| // then the test is successful because it has the correct exception class. | ||
| } | ||
| } | ||
|
|
||
| public function testOtherStageNotFound(): void | ||
| { | ||
| $collection = $this->createCollection($this->getDatabaseName(), $this->getCollectionName()); | ||
|
|
||
| try { | ||
| $collection->aggregate([ | ||
| ['$searchStageNotExisting' => ['text' => ['query' => 'test', 'path' => 'field']]], | ||
| ]); | ||
| self::fail('Expected ServerException was not thrown'); | ||
| } catch (ServerException $exception) { | ||
| self::assertNotInstanceOf(SearchNotSupportedException::class, $exception, $exception); | ||
| } | ||
| } | ||
|
|
||
| public function testOtherCommandNotFound(): void | ||
| { | ||
| try { | ||
| $this->manager->executeCommand($this->getDatabaseName(), new Command(['nonExistingCommand' => 1])); | ||
| self::fail('Expected ServerException was not thrown'); | ||
| } catch (ServerException $exception) { | ||
| self::assertFalse(SearchNotSupportedException::isSearchNotSupportedError($exception)); | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
match inception 🤯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A decision tree 🎄