Better Errors?

[astanciu]

I've noticed the errors that come out of Thinking are very unstructured, or maybe I'm doing something wrong.

For example, I'm testing inserting a doc, for which there already exists a primary key and want to capture this error to nicely let the user know. Here's what console.dir(error) shows:

[Error: An error occurred during the batch insert. Original results:
{
  "changes": [
    {
      "new_val": {
        "displayname": "Acme, Co",
        "name": "acme",
        "website": "http://acme.com"
      },
      "old_val": {
        "displayname": "Acme, Co",
        "name": "acme",
        "website": "http://acme.com"
      }
    }
  ],
  "deleted": 0,
  "errors": 1,
  "first_error": "Duplicate primary key `name`:\n{\n\t\"displayname\":\t\"Acme, Co\",\n\t\"name\":\t\"acme\",\n\t\"website\":\t\"http://acme.com\"\n}\n{\n\t\"displayname\":\t\"Acme, Co\",\n\t\"name\":\t\"acme\",\n\t\"website\":\t\"http://acme.com\"\n}",
  "inserted": 0,
  "replaced": 0,
  "skipped": 0,
  "unchanged": 0
}]

This isn't JSON, nor an array, nor anything easily digest-able. Am I missing something or can we improve the errors by making them actual objects with properties that can be read?

I noticed the same issue with the validation errors... No easy way to parse them. Seems to be similar to #328

[neumino]

Thinky just proxies errors returned by RethinkDB. It currently doesn't parse errors. I've been waiting RethinkDB to start rolling out specific errors, and they did in 2.1, but they are not as precise as I wanted.

I guess thinky should start creating specific errors and work later to sync errors when RethinkDB provide more specific errors.

[astanciu]

Yeah, maybe even just wrapping that into a proper object with some kind of code would be useful.

{
error: 'exists',
message: 'Already exists',
original: {original obj}
}

[astanciu]

Actually, I think there is something that Thinky is doing on top of RethinkDB's driver which isn't helping.

When I try the same operation (insert a doc whose primary key already exists) this is what happens: With Native Driver: Does not throw an error, the Promise is resolved with an object that looks like this:

{ deleted: 0,
  errors: 1,
  first_error: 'Duplicate primary key `name`:\n{\n\t"displayname":\t"Acme, Co",\n\t"name":\t"acme",\n\t"website":\t"http://acme.com"\n}\n{\n\t"displayname":\t"Acme, Co",\n\t"name":\t"acme",\n\t"website":\t"http://acme.com"\n}',
  inserted: 0,
  replaced: 0,
  skipped: 0,
  unchanged: 0 }

With Thinky: Throws an error like in my first commet which is un-parseable.

With the native driver at least I can check for .errors > 0 and the error is stored in .first_error.

I agree that It's still not super clean.

Maybe Thinky can return that same error but formatted in proper JSON:

{
  error: "An error occurred during the batch insert",
  original: {
    "changes": [{
      "new_val": {
        "displayname": "Acme, Co",
        "name": "acme",
        "website": "http://acme.com"
      },
      "old_val": {
        "displayname": "Acme, Co",
        "name": "acme",
        "website": "http://acme.com"
      }
    }],
    "deleted": 0,
    "errors": 1,
    "first_error": "Duplicate primary key `name`:\n{\n\t\"displayname\":\t\"Acme, Co\",\n\t\"name\":\t\"acme\",\n\t\"website\":\t\"http://acme.com\"\n}\n{\n\t\"displayname\":\t\"Acme, Co\",\n\t\"name\":\t\"acme\",\n\t\"website\":\t\"http://acme.com\"\n}",
    "inserted": 0,
    "replaced": 0,
    "skipped": 0,
    "unchanged": 0
  }
}

[primitive-type]

This, or alternatively, a PrimaryKeyError type (similar to ValidationError) would be helpful in order to properly check for duplicate primary keys.