search

mountetna / timur

Data browser for biologists
GNU General Public License v2.0
3 stars 6 forks source link

Intercept and fix raw Magma errors for the UI. #181

Closed jasondcater closed 5 years ago

jasondcater commented 6 years ago

When there is a Magma error on the getDocuments API cycle, Timur will send back the raw error message from Magma. We need to intercept it and send back a user friendly message (and one that doesn't expose the Magma stack).

I got the error when using a mismatch between timur view data and the magma database. Timur was requesting a column/record that didn't exist on my version of magma. Magma complained. The following was sent back to the client.

#<Sequel::DatabaseError: PG::UndefinedColumn: ERROR:  column mduuvjjglw.rbc_lysis does not exist
LINE 1: ...ll_count" AS "mduuvjjglw_post_digest_cell_count", "mduuvjjgl...
                                                             ^
>
/home/developer/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/sequel-4.49.0/lib/sequel/adapters/postgres.rb:166:in `async_exec'
/home/developer/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/sequel-4.49.0/lib/sequel/adapters/postgres.rb:166:in `block in execute_query'
/home/developer/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/sequel-4.49.0/lib/sequel/database/logging.rb:49:in `log_connection_yield'
/home/developer/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/sequel-4.49.0/lib/sequel/adapters/postgres.rb:166:in `execute_query'
/home/developer/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/sequel-4.49.0/lib/sequel/adapters/postgres.rb:153:in `block in execute'
/home/developer/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/sequel-4.49.0/lib/sequel/adapters/postgres.rb:129:in `check_disconnect_errors'
/home/developer/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/sequel-4.49.0/lib/sequel/adapters/postgres.rb:153:in `execute'
graft commented 6 years ago

This should probably be fixed at the magma level - why is a raw database error being returned at all?

Does the error return differently in production vs. development mode? Perhaps a development server is exposing an error deliberately?

Do you know what the request is that is being posted to magma?

jasondcater commented 6 years ago

The query is just a request for data from the database. There is a mismatch between requested attributes and columns in the DB. Magma should definitely be fixed to not emit those error to Timur. Timur should also catch any unexpected output from Magma and filter it to the client. It doens't matter if it's due to the dev/prod mode. One cannot predict 100% of all output from interacting systems. It's best to build in safe guards wherever it's appropriate. In this case is is also appropriate for Timur to be checking the returns from it's various services to make sure they are well formed.