Add Base Exception class#218
Conversation
christyjacob4
left a comment
There was a problem hiding this comment.
Looks good 👌
Can you also scan through the code base and check if there are usages of exceptions other than Utopia\Database\Exception or Utopia\Database\Exception\Authorization etc ?
Basically we want to ensure that every custom error thrown by the database library is an instance of the Database library exception or its sub classes.
I have updated every instance where we throw an exception to be a type of ie. in cases where we don't throw an exception explicitly. |
…atabase into add-exception-class
Improve exceptions
| use Swoole\Coroutine\Client as CoroutineClient; | ||
| use Utopia\Database\Adapter\Mongo\Auth; | ||
| use Utopia\Database\Adapter\Mongo\MongoClientOptions; | ||
| use Utopia\Database\Document; |
There was a problem hiding this comment.
Can you sync your branch with the base branch ? I don't think this file is expected to be here.
There was a problem hiding this comment.
I am on the latest version of main
There was a problem hiding this comment.
removed this file
| try { | ||
| $role = Role::parse($role); | ||
| } catch (\Exception $e) { | ||
| } catch (Exception $e) { |
There was a problem hiding this comment.
Hmm.. is this necessary ? Do we need to explicitly import the native exception class ?
| // Check for deprecated query syntax | ||
| if (\str_contains($method, '.')) { | ||
| throw new \Exception("Invalid query method"); | ||
| throw new DatabaseException("Invalid query method"); |
There was a problem hiding this comment.
Okay after reviewing Role, Query and Permission I don't think we need to expose any of those errors in the UI either. Let's revert these files back too
| break; | ||
| default: | ||
| throw new Exception('Unknown Type'); | ||
| throw new DatabaseException('Unknown Type. Must be one of ${Database::VAR_INTEGER}, ${Database::VAR_FLOAT}, ${Database::VAR_BOOLEAN}, ${Database::VAR_DOCUMENT}, ${Database::VAR_DATETIME}'); |
There was a problem hiding this comment.
| throw new DatabaseException('Unknown Type. Must be one of ${Database::VAR_INTEGER}, ${Database::VAR_FLOAT}, ${Database::VAR_BOOLEAN}, ${Database::VAR_DOCUMENT}, ${Database::VAR_DATETIME}'); | |
| throw new DatabaseException("Unknown Type. Must be one of ${Database::VAR_INTEGER}, ${Database::VAR_FLOAT}, ${Database::VAR_BOOLEAN}, ${Database::VAR_DOCUMENT}, ${Database::VAR_DATETIME}"); |
String interpolation only works when the string is enclosed in double quotes
|
|
||
| default: | ||
| throw new Exception('Unknown Operator:' . $operator); | ||
| throw new DatabaseException('Unknown Operator:' . $operator .'. Must be one of ${Query::TYPE_EQUAL}, ${Query::NOTEQUAL, ${Query::TYPE_LESSER}, ${Query::TYPE_LESSEREQUAL}, ${Query::TYPE_GREATER}, ${Query::TYPE_GREATEREQUAL}, ${Query::TYPE_CONTAINS}, {Query::TYPE_SEARCH}'); |
There was a problem hiding this comment.
| throw new DatabaseException('Unknown Operator:' . $operator .'. Must be one of ${Query::TYPE_EQUAL}, ${Query::NOTEQUAL, ${Query::TYPE_LESSER}, ${Query::TYPE_LESSEREQUAL}, ${Query::TYPE_GREATER}, ${Query::TYPE_GREATEREQUAL}, ${Query::TYPE_CONTAINS}, {Query::TYPE_SEARCH}'); | |
| throw new DatabaseException('Unknown Operator:' . $operator .". Must be one of ${Query::TYPE_EQUAL}, ${Query::NOTEQUAL}, ${Query::TYPE_LESSER}, ${Query::TYPE_LESSEREQUAL}, ${Query::TYPE_GREATER}, ${Query::TYPE_GREATEREQUAL}, ${Query::TYPE_CONTAINS}, {Query::TYPE_SEARCH}"); |
Following the changes in this PR, we need to add some base Database Exception that we could use to distinguish between our errors and native errors. This PR implements a base exception class.