thinky
thinky copied to clipboard
JSON format the "duplicate key" error?
A duplicate primary key message currently looks like this:
[Error: Duplicate primary key `username`:
{
"firstName": "Hank",
"lastName": "Hill",
"username": "propanerocks"
}
{
"firstName": "Hank",
"lastName": "Hill",
"username": "propanerocks"
}]
Informative, but not something I'd like to spit out to an API output, especially with sensitive fields like passwords and whatnot. I want to say "Username exists" or something, but the problem is that the error might not necessarily be a dupe key.
So with a catch-all error handler, it'd be nice if the error looked more like this:
{
"message": "Error: Duplicate primary key `username`",
"items": {
"input/submitted/etc": {
"firstName": "Hank",
"lastName": "Hill",
"username": "propanerocks"
},
"original": {
"firstName": "Hank",
"lastName": "Hill",
"username": "propanerocks"
}
}
}
That way, at the very least I can spit out the error message by itself.
Just to be sure, you basically would like to retrieve all errors instead of the first one that happens?
Not exactly. Let me try to be more specific. Let's say I have code that looks like this:
user.save(function(err, user){
if(err){ // This is what I have an issue with
res.send(409, 'Duplicate user'); // The error may not always be 'duplicate', though
return next();
} else {
res.send(200, user);
}
});
The problem I have is that err gets called regardless of whether the issue is with a duplicate key or a field that doesn't conform to a strict type, or whatever other errors there are. So when I have to deal with it, I want to spit out a specific HTTP status code, as well as tell the API consumer what just went wrong.
The way it currently works, is that the messages are a string or some weird form of an object. On top of that, the current error message displays everything. If my user items include a field in the database that stores the hashed password, that is being output along with the error message. And since it's not an array or JSON object, it's much more difficult to remove that part or isolate just the "Duplicate primary key" message.
I'm basically wondering if the error messages can be formatted as a legal JSON object for easier consumption.
+1 – a consistent error object with codes or some such would be amazing. Should this actually be a feature of RethinkDB though?
So I was going to implement that a few months ago, but then code errors got on the table at RethinkDB. Looking at the tracker, it was pushed back, so maybe thinky shouldn't wait.
For the time being, I plan to extend a bit the errors returned.
I'll add a field short_message that does not contain the old/new document, only the text message.
Then there's still a need for a regex somewhere since the call to save could fail for multiple reasons:
- master non available
- duplicate primary key
- network issues
- etc.
FYI: RethinkDB 2.1 is coming with a better error hierarchy but still no codes as far as I know. So there's still some work to do here.
What's the current practice for parsing errors? There doesn't seem to be any easy way to match "Duplicate primary key"...
The best practice is sending a whitelist of error. If you want to return something better than just a 500 error, parse the error and send back your own message (or the same if you want). Don't send back thinky's error back to the client.
This is true for thinky but also for any other software.
That's true, but it's not an error, duplicate primary key is just a bunch of text. Woops, cross-posted to #328.
From @avimar
This seems quite messy, but is the code I currently seemingly need:
if(err.name==="Document failed validation") message = "Number is invalid";
else if(err.message.indexOf("Duplicate primary key")!==-1) message = "Number already exists";
Can we get duplicate primary key as an actual error thrown by thinky, instead of being passed directly from the DB?
You are welcome to submit a pull request.
Latest entry in the "brittlest code in the universe"-contest:
try
{
yield model.save( data );
}
catch( err )
{
const errObj = JSON.parse(
err.message.substr(
err.message.indexOf( '{' )
, err.message.lastIndexOf( '}' )
)
);
const errStr = errObj.first_error.substr(
0
, errObj.first_error.indexOf( '\n') - 1
);
// use errStr
}