neoxygen / neo4j-neoclient

Simple PHP HttpClient for the Neo4j ReST API with Multi DB Support
MIT License
121 stars 138 forks source link

Make specific exceptions for error conditions #70

Closed flip111 closed 8 years ago

flip111 commented 8 years ago

This is an example of an exception i get

[Neoxygen\NeoClient\Exception\Neo4jException]
Neo4j Exception with code "Neo.ClientError.Schema.ConstraintViolation" and message "Node 59 already exists with label user and property "username"=[flip111]"

It would be nice to have a specific exception for this. Maybe the class can be called ConstraintViolationException. Then what i can do in the code is:

function createUser() {
  try {
    $client->sendCypherQuery('CREATE (n:user {username:"flip111"})');
  } catch (ConstraintViolationException $e) {
    // notify the user somehow the user already exist
  }
  // Other exceptions are not caught and can display a 500 Internal Server error page
}

It would require creating exception based on the exception code. (That would also require knowing all the possible exception codes, which i don't know where to find a list for)

flip111 commented 8 years ago

Alternatively an easy improvement without have to create tons of exception classes would be to make the code available on the exception as property. Right now this information is pushed inside a string and not accesible anymore

https://github.com/neoxygen/neo4j-neoclient/blob/6028690c950c47e43bf66fac8b184d927860deb2/src/Extension/AbstractExtension.php#L94-L98

This is how the code would look like in that case:

function createUser() {
  try {
    $client->sendCypherQuery('CREATE (n:user {username:"flip111"})');
  } catch (\Neo4jException $e) {
    if ($e->getCode() === 'Neo.ClientError.Schema.ConstraintViolation') {
      // notify the user somehow the user already exist
    } else {
      throw $e;
    }
  }
  // Other exceptions are not caught and can display a 500 Internal Server error page
}

(not really sure if that getter is really needed, perhaps a simple property will do)

ikwattro commented 8 years ago

This is a planned stuff.

flip111 commented 8 years ago

:+1: