search

spider-gazelle / rethinkdb-orm

RethinkDB ORM for Crystal lang
MIT License
24 stars 0 forks source link

Better errors are much needed #17

Closed watzon closed 2 years ago

watzon commented 3 years ago

Idk if this applies to this repo so much as kingsleyh/crystal-rethinkdb, but I feel like better error messages are a must. Right now errors look like this:

Failed to create document! (RethinkORM::Error::DocumentNotSaved)
  from lib/rethinkdb-orm/src/rethinkdb-orm/base.cr:20:3 in 'create!:chat_id:type:title:username:first_name:last_name:description'
  from src/lucybot/helpers.cr:165:11 in 'persist_chats'
  from src/lucybot/bot.cr:15:5 in 'persistence'
  from lib/tourmaline/src/tourmaline/handlers/update_handler.cr:20:9 in '->'
  from ../../../.asdf/installs/crystal/0.35.1/src/primitives.cr:255:3 in 'call'
  from lib/tourmaline/src/tourmaline/client.cr:171:14 in 'handle_update'
  from lib/tourmaline/src/tourmaline/client/core_methods.cr:1197:15 in 'poll'
  from lib/tourmaline/src/tourmaline/client/core_methods.cr:1187:7 in 'poll'
  from src/lucybot.cr:15:3 in '->'
  from ../../../.asdf/installs/crystal/0.35.1/src/primitives.cr:255:3 in 'run_compute'
  from lib/async/src/async/future.cr:196:24 in '->'
  from ../../../.asdf/installs/crystal/0.35.1/src/primitives.cr:255:3 in 'run'
  from ../../../.asdf/installs/crystal/0.35.1/src/fiber.cr:92:34 in '->'

which gives little to no useful information. It would be nice to know why the query failed, and not just that it did.

caspiano commented 3 years ago

The rethinkdb library doesn't have very good errors, but this is a bit of a separate fail. What you have here is either a validation error or DB persistence error.

I agree, the exception in its current form is not very helpful.

An immediate improvement to this error would be raising a new error in case of an invalid document, RethinkORM::Error::InvalidDocument, and rendering the validations in the error body.

I'll attempt to improve the quality of the alternate error cases as well when I have bandwidth

caspiano commented 3 years ago

Got a PR for what I mentioned above. Unfortunately, the response back from the rethinkDB driver are really limited - essentially just an object with counts of inserted, updated, replaced, unchanged

I'll look into if it's possible to improve errors via the driver