Custom Errors

[tgriesser]

Implement custom Error that can be used for easier debugging, maintaining the query, bindings, and other information.

[koskimas]

This would be great! (If this means what I think it means)

My problem is this: In the 0.44 version an error like this:

{ [error: duplicate key value violates unique constraint "user_username_unique"]
name: 'error',
length: 166,
severity: 'ERROR',
code: '23505',
detail: 'Key (username)=(user1) already exists.',
hint: undefined,
position: undefined,
internalPosition: undefined,
internalQuery: undefined,
where: undefined,
file: 'nbtinsert.c',
line: '396',
routine: '_bt_check_unique'
}

was thrown for unique constraint violation. From this I was able to hack out the column that failed and I was able to use the error code. Now in the 0.45 version the error contains just a message string.

It would be great if the errors thrown by the database driver could be passed with the error somehow.

[tgriesser]

Ah, thanks for pointing this out @koskimas!... So the change I made in 0.4.5 was to do this:

return chain.then(builder.handleResponse).otherwise(function(e) {
  var err = new Error(e.toString() + ' - ' + '{sql: ' + sql + ', bindings: ' + bindings + '}');
      err.originalStack = e.stack;
      throw err;
});

which is sort of a mess... first, because it's using object notation for the sql / bindings, but it's impossible to parse because it is after the e.toString() + ' - ' +... and then the original stack isn't really useful either, is it?

What do you think about something like this

return chain.then(builder.handleResponse).otherwise(function(error) {
  var newError = new Error('{sql: ' + sql + ', bindings: ' + bindings + ', message: ' + error.toString() + '}');
      newError.clientError = error;
      throw newError;
});

That way, you'd have the new error which contains the sql and bindings sent to the builder, which I find is often what I need to debug, as well as the toString of the original error... then if you want it you can access clientError on the newError object to get the original error and all of the associated info.

Does that sound like a good compromise, or do you have any suggestions?

[koskimas]

The clientError property would work perfectly in my case. I'm not sure how wise it is for me to count on the pg error to remain the same in the future since it says "need to document how to handle errors..." in the documentation of node-postgres :D

By the way, I just recently discovered knex and I think it's awesome! Keep up the great work.

[tgriesser]

Thanks, will do! Also, for reference, here's the new implementation of the error in 0.4.6:

// Since we usually only need the `sql` and `bindings` to help us debug the query, output them
// into a new error... this way, it `console.log`'s nicely for debugging, but you can also
// parse them out with a `JSON.parse(error.message)`. Also, use the original `clientError` from the
// database client is retained as a property on the `newError`, for any additional info.
return chain.then(builder.handleResponse).otherwise(function(error) {
  var newError = new Error(JSON.stringify({sql: sql, bindings: bindings}));
      newError.clientError = error;
  throw newError;
});

[tgriesser]

@koskimas - also, if you like Knex, you might be interested in checking out Bookshelf if you haven't already.