Don't obscure errors during migrations

spec/migrator/migration_spec.cr

@@ -66,7 +66,7 @@ end
 
 describe Avram::Migrator::Migration::V1 do
   it "executes statements in a transaction" do
-    expect_raises Exception, %(relation "table_does_not_exist" does not exist) do
+    expect_raises Avram::FailedMigration do
       MigrationThatPartiallyWorks::V999.new.up
     end
 

spec/spec_helper.cr

@@ -15,9 +15,9 @@ backend = Log::IOBackend.new(STDERR)
 backend.formatter = Dexter::JSONLogFormatter.proc
 Log.builder.bind("avram.*", :error, backend)
 
-Db::Create.new(quiet: true).call
-Db::Migrate.new(quiet: true).call
-Db::VerifyConnection.new(quiet: true).call
+Db::Create.new(quiet: true).run_task
+Db::Migrate.new(quiet: true).run_task
+Db::VerifyConnection.new(quiet: true).run_task
 
 Spec.before_each do
   TestDatabase.truncate

spec/tasks/db_create_spec.cr

@@ -4,7 +4,7 @@ describe Db::Create do
   it "raises a connection error when unable to connect" do
     Avram.temp_config(database_to_migrate: DatabaseWithIncorrectSettings) do
       expect_raises(Exception, /It looks like Postgres is not running/) do
-        Db::Create.new(quiet: true).call
+        Db::Create.new(quiet: true).run_task
       end
     end
   end

spec/tasks/db_schema_dump_spec.cr

@@ -4,7 +4,7 @@ include CleanupHelper
 describe Db::Schema::Dump do
   it "generates a new sql dump file" do
     with_cleanup do
-      Db::Schema::Dump.new("structure.sql").call
+      Db::Schema::Dump.new("structure.sql").run_task
 
       filename = "structure.sql"
       File.exists?(filename).should eq true

spec/tasks/db_schema_restore_spec.cr

@@ -5,22 +5,22 @@ SQL_DUMP_FILE = "spec/support/files/sample_backup.sql"
 describe Db::Schema::Restore do
   it "raises an error when no import file is supplied" do
     expect_raises(Exception, "A path to the import SQL file must be provided") do
-      Db::Schema::Restore.new.call
+      Db::Schema::Restore.new.run_task
     end
   end
 
   it "raises an error when unable to find the import file" do
     expect_raises(Exception, "Unable to locate the restore file: missing_file.sql") do
-      Db::Schema::Restore.new("missing_file.sql").call
+      Db::Schema::Restore.new("missing_file.sql").run_task
     end
   end
 
   it "restores from the sample_backup file" do
     Avram.temp_config(database_to_migrate: SampleBackupDatabase) do
-      Db::Drop.new.call
-      Db::Create.new(quiet: true).call
+      Db::Drop.new.run_task
+      Db::Create.new(quiet: true).run_task
 
-      Db::Schema::Restore.new(SQL_DUMP_FILE).call
+      Db::Schema::Restore.new(SQL_DUMP_FILE).run_task
 
       SampleBackupDatabase.run do |db|
         value = db.scalar("SELECT COUNT(*) FROM sample_records").as(Int64)

spec/tasks/db_verify_connection_spec.cr

@@ -5,7 +5,7 @@ describe Db::VerifyConnection do
     creds = Avram::Credentials.parse("postgres://eat@joes/crab_shack")
     TestDatabase.temp_config(credentials: creds) do
       expect_raises Exception, /Unable to connect to Postgres for database 'TestDatabase'/ do
-        Db::VerifyConnection.new.call
+        Db::VerifyConnection.new.run_task
       end
     end
   end

src/avram/errors.cr

@@ -146,4 +146,21 @@ module Avram
   # Used when `Avram::Operation` fails.
   class FailedOperation < AvramError
   end
+
+  class FailedMigration < AvramError
+    def initialize(@migration : String, statements : Array(String), cause : Exception)
+      super(message(statements), cause)
+    end
+
+    private def message(statements : Array(String))
+      <<-ERROR
+      Error occurred while performing a migration.
+
+      Migration: #{@migration}
+
+      Statements:
+        #{statements.join("\n")}
+      ERROR
+    end
+  end
 end

src/avram/migrator/migration.cr

@@ -97,15 +97,7 @@ abstract class Avram::Migrator::Migration::V1
       end
     end
   rescue e : PQ::PQError
-    raise <<-ERROR
-    There was a problem running this statement:
-
-      #{statements.join("\n")}
-
-    Problem:
-
-      #{e.message}
-    ERROR
+    raise FailedMigration.new(migration: self.class.name, statements: statements, cause: e)
   end
 
   def reset_prepared_statements

src/avram/migrator/migrator.cr

@@ -1,9 +1,8 @@
 module Avram::Migrator
   def self.run
     yield
-  rescue e : PQ::PQError
-    raise e.message.colorize(:red).to_s
-  rescue e : Exception
-    raise e.message.to_s
+  rescue e
+    puts e.inspect_with_backtrace
+    exit 1
   end
 end

src/avram/migrator/runner.cr

@@ -94,7 +94,7 @@ class Avram::Migrator::Runner
   end
 
   def self.dump_db(dump_to : String = "db/structure.sql", quiet : Bool = false)
-    Db::VerifyConnection.new(quiet: true).call
+    Db::VerifyConnection.new(quiet: true).run_task
     run "pg_dump -s #{cmd_args} > #{dump_to}"
     unless quiet
       puts "Done dumping #{db_name.colorize(:green)}"
@@ -189,7 +189,5 @@ class Avram::Migrator::Runner
     end
   rescue e : DB::ConnectionRefused
     raise "Unable to connect to the database. Please check your configuration.".colorize(:red).to_s
-  rescue e : Exception
-    raise "Unexpected error while running migrations: #{e.message}".colorize(:red).to_s
   end
 end

src/avram/tasks/db/base_task.cr

@@ -0,0 +1,9 @@
+abstract class BaseTask < LuckyCli::Task
+  abstract def run_task
+
+  def call
+    Avram::Migrator.run do
+      run_task
+    end
+  end
+end

src/avram/tasks/db/create.cr

@@ -1,4 +1,4 @@
-class Db::Create < LuckyCli::Task
+class Db::Create < BaseTask
   alias Migrator = Avram::Migrator
   summary "Create the database"
 
@@ -19,9 +19,7 @@ class Db::Create < LuckyCli::Task
     TEXT
   end
 
-  def call
-    Migrator.run do
-      Migrator::Runner.create_db(@quiet)
-    end
+  def run_task
+    Migrator::Runner.create_db(@quiet)
   end
 end

src/avram/tasks/db/drop.cr

@@ -1,13 +1,11 @@
 require "colorize"
 
-class Db::Drop < LuckyCli::Task
+class Db::Drop < BaseTask
   summary "Drop the database"
 
-  def call
-    Avram::Migrator.run do
-      Avram::Migrator::Runner.drop_db
-      puts "Done dropping #{Avram::Migrator::Runner.db_name.colorize(:green)}"
-    end
+  def run_task
+    Avram::Migrator::Runner.drop_db
+    puts "Done dropping #{Avram::Migrator::Runner.db_name.colorize(:green)}"
   end
 
   def help_message

src/avram/tasks/db/migrate.cr

@@ -1,6 +1,6 @@
 require "colorize"
 
-class Db::Migrate < LuckyCli::Task
+class Db::Migrate < BaseTask
   summary "Run any pending migrations"
 
   def initialize(@quiet : Bool = false)
@@ -18,9 +18,7 @@ class Db::Migrate < LuckyCli::Task
     TEXT
   end
 
-  def call
-    Avram::Migrator.run do
-      Avram::Migrator::Runner.new(@quiet).run_pending_migrations
-    end
+  def run_task
+    Avram::Migrator::Runner.new(@quiet).run_pending_migrations
   end
 end

src/avram/tasks/db/migrate_one.cr

@@ -1,6 +1,6 @@
 require "colorize"
 
-class Db::Migrate::One < LuckyCli::Task
+class Db::Migrate::One < BaseTask
   summary "Run just the next pending migration"
 
   def help_message
@@ -14,9 +14,7 @@ class Db::Migrate::One < LuckyCli::Task
     TEXT
   end
 
-  def call
-    Avram::Migrator.run do
-      Avram::Migrator::Runner.new.run_next_migration
-    end
+  def run_task
+    Avram::Migrator::Runner.new.run_next_migration
   end
 end

src/avram/tasks/db/migrations_status.cr

@@ -1,9 +1,9 @@
 require "shell-table"
 
-class Db::Migrations::Status < LuckyCli::Task
+class Db::Migrations::Status < BaseTask
   summary "Print the current status of migrations"
 
-  def call
+  def run_task
     if migrations.none?
       puts "There are no migrations.".colorize(:green)
     else

src/avram/tasks/db/redo.cr

@@ -1,6 +1,6 @@
 require "colorize"
 
-class Db::Redo < LuckyCli::Task
+class Db::Redo < BaseTask
   summary "Rollback and run just the last migration"
 
   def help_message
@@ -14,8 +14,8 @@ class Db::Redo < LuckyCli::Task
     TEXT
   end
 
-  def call
-    Db::Rollback.new.call
-    Db::Migrate.new.call
+  def run_task
+    Db::Rollback.new.run_task
+    Db::Migrate.new.run_task
   end
 end

src/avram/tasks/db/reset.cr

@@ -1,6 +1,6 @@
 require "colorize"
 
-class Db::Reset < LuckyCli::Task
+class Db::Reset < BaseTask
   summary "Drop, recreate, and run migrations."
 
   def help_message
@@ -18,9 +18,9 @@ class Db::Reset < LuckyCli::Task
     TEXT
   end
 
-  def call
-    Db::Drop.new.call
-    Db::Create.new.call
-    Db::Migrate.new.call
+  def run_task
+    Db::Drop.new.run_task
+    Db::Create.new.run_task
+    Db::Migrate.new.run_task
   end
 end

src/avram/tasks/db/rollback.cr

@@ -1,6 +1,6 @@
 require "colorize"
 
-class Db::Rollback < LuckyCli::Task
+class Db::Rollback < BaseTask
   summary "Rollback the last migration"
 
   def help_message
@@ -14,9 +14,7 @@ class Db::Rollback < LuckyCli::Task
     TEXT
   end
 
-  def call
-    Avram::Migrator.run do
-      Avram::Migrator::Runner.new.rollback_one
-    end
+  def run_task
+    Avram::Migrator::Runner.new.rollback_one
   end
 end

src/avram/tasks/db/rollback_all.cr

@@ -1,6 +1,6 @@
 require "colorize"
 
-class Db::RollbackAll < LuckyCli::Task
+class Db::RollbackAll < BaseTask
   summary "Rollback all migrations"
 
   def help_message
@@ -16,10 +16,8 @@ class Db::RollbackAll < LuckyCli::Task
     TEXT
   end
 
-  def call
-    Avram::Migrator.run do
-      Avram::Migrator::Runner.new.rollback_all
-      puts "Done rolling back all migrations".colorize(:green)
-    end
+  def run_task
+    Avram::Migrator::Runner.new.rollback_all
+    puts "Done rolling back all migrations".colorize(:green)
   end
 end

src/avram/tasks/db/rollback_to.cr

@@ -1,6 +1,6 @@
 require "colorize"
 
-class Db::RollbackTo < LuckyCli::Task
+class Db::RollbackTo < BaseTask
   summary "Rollback to a specific migration"
 
   def help_message
@@ -16,14 +16,12 @@ class Db::RollbackTo < LuckyCli::Task
     TEXT
   end
 
-  def call
-    Avram::Migrator.run do
-      version = ARGV.first?
-      if version && version.to_i64?
-        Avram::Migrator::Runner.new.rollback_to(version.to_i64)
-      else
-        raise "Migration version is required. Example: lucky db.rollback_to 20180802180356".colorize(:red).to_s
-      end
+  def run_task
+    version = ARGV.first?
+    if version && version.to_i64?
+      Avram::Migrator::Runner.new.rollback_to(version.to_i64)
+    else
+      raise "Migration version is required. Example: lucky db.rollback_to 20180802180356".colorize(:red).to_s
     end
   end
 end

src/avram/tasks/db/schema_dump.cr

@@ -1,4 +1,4 @@
-class Db::Schema::Dump < LuckyCli::Task
+class Db::Schema::Dump < BaseTask
   summary "Export database schema to a sql file"
 
   def initialize(@dump_to : String? = nil, @quiet : Bool = false)
@@ -18,10 +18,8 @@ class Db::Schema::Dump < LuckyCli::Task
     TEXT
   end
 
-  def call
+  def run_task
     dump_to = @dump_to || ARGV.first? || raise "Must pass a file path to dump the db structure to"
-    Avram::Migrator.run do
-      Avram::Migrator::Runner.dump_db(dump_to, @quiet)
-    end
+    Avram::Migrator::Runner.dump_db(dump_to, @quiet)
   end
 end

src/avram/tasks/db/schema_restore.cr

@@ -1,4 +1,4 @@
-class Db::Schema::Restore < LuckyCli::Task
+class Db::Schema::Restore < BaseTask
   summary "Restore database from a sql dump file"
 
   def initialize(@import_file_path : String? = nil, @quiet : Bool = false)
@@ -17,10 +17,8 @@ class Db::Schema::Restore < LuckyCli::Task
     TEXT
   end
 
-  def call
+  def run_task
     import_file_path = @import_file_path || ARGV.first? || raise "A path to the import SQL file must be provided"
-    Avram::Migrator.run do
-      Avram::Migrator::Runner.restore_db(import_file_path.as(String), @quiet)
-    end
+    Avram::Migrator::Runner.restore_db(import_file_path.as(String), @quiet)
   end
 end