Skip to content

Improve exceptions#1

Merged
everly-gif merged 9 commits intoeverly-gif:add-exception-classfrom
Meldiron:add-exception-class
Dec 6, 2022
Merged

Improve exceptions#1
everly-gif merged 9 commits intoeverly-gif:add-exception-classfrom
Meldiron:add-exception-class

Conversation

@Meldiron
Copy link

No description provided.

if(isset(self::$formats[$name])) {
if(self::$formats[$name]['type'] !== $type) {
throw new Exception('Format ("'.$name.'") not available for this attribute type ("'.$type.'")');
throw new DatabaseException('Format ("'.$name.'") not available for this attribute type ("'.$type.'")');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We dont need to expose these errors since a format is an internal database concept and the end-user can't really do anything with that this information. These exceptions are better just logged.

}

throw new Exception('Unknown format validator: "'.$name.'"');
throw new DatabaseException('Unknown format validator: "'.$name.'"');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

$parsed[] = Query::parse($query);
} catch (\Throwable $th) {
throw new Exception("Invalid query: ${query}", previous: $th);
throw new DatabaseException("Invalid query: ${query}");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to expose it. An end user cannot control the query ( or fix any syntax issues on their query )

$collection = $this->silent(fn() => $this->getCollection($id));
if (!$collection->isEmpty() && $id !== self::METADATA){
throw new Duplicate('Collection ' . $id . ' Exists!');
throw new DuplicateException('Collection ' . $id . ' Exists!');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is okay 👍

Apart from this, the other changes in the file are not required. filters are not something an end user can fix from the dashboard

{
if (empty($this->namespace)) {
throw new Exception('Missing namespace');
throw new DatabaseException('Missing namespace');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets revert the changes in this file. Namespace, & missing database are not under the end user's control

{
if (empty($namespace)) {
throw new Exception('Missing namespace');
throw new DatabaseException('Missing namespace');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets revert the changes in this file. Missing namespace, missing database etc are not under the control of the end-user

@everly-gif everly-gif merged commit 81a9716 into everly-gif:add-exception-class Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants