From 0bac1245cd71787cd0c14754026e81008d748224 Mon Sep 17 00:00:00 2001 From: fabianlindfors Date: Mon, 3 Jan 2022 00:03:23 +0100 Subject: [PATCH] Add context to errors --- src/helpers.rs | 4 +- src/lib.rs | 109 +++++++++++++++++++++++--------- src/migrations/add_column.rs | 28 ++++---- src/migrations/add_index.rs | 7 +- src/migrations/alter_column.rs | 38 ++++++----- src/migrations/create_table.rs | 8 ++- src/migrations/remove_column.rs | 9 ++- src/migrations/remove_table.rs | 3 +- src/migrations/rename_table.rs | 3 +- 9 files changed, 140 insertions(+), 69 deletions(-) diff --git a/src/helpers.rs b/src/helpers.rs index 53ad558..3ab1480 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -2,7 +2,7 @@ use anyhow::Context; use crate::db::Conn; -pub fn setup_helpers(db: &mut dyn Conn, current_migration: &Option) -> anyhow::Result<()> { +pub fn set_up_helpers(db: &mut dyn Conn, current_migration: &Option) -> anyhow::Result<()> { let predicate = if let Some(current_migration) = current_migration { format!( "current_setting('search_path') = 'migration_{}' OR setting_bool", @@ -32,7 +32,7 @@ pub fn setup_helpers(db: &mut dyn Conn, current_migration: &Option) -> a Ok(()) } -pub fn teardown_helpers(db: &mut dyn Conn) -> anyhow::Result<()> { +pub fn tear_down_helpers(db: &mut dyn Conn) -> anyhow::Result<()> { db.query("DROP FUNCTION reshape.is_old_schema;")?; Ok(()) } diff --git a/src/lib.rs b/src/lib.rs index 7958cb4..9929008 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3,7 +3,7 @@ use crate::{ schema::Schema, }; -use anyhow::anyhow; +use anyhow::{anyhow, Context}; use colored::*; use db::{Conn, DbConn}; use postgres::Config; @@ -96,7 +96,8 @@ impl Reshape { println!(" Applying {} migrations\n", remaining_migrations.len()); - helpers::setup_helpers(&mut self.db, current_migration)?; + helpers::set_up_helpers(&mut self.db, current_migration) + .context("failed to set up helpers")?; let mut new_schema = Schema::new(); let mut processed_migrations: &[Migration] = &[]; @@ -107,10 +108,13 @@ impl Reshape { processed_migrations = &remaining_migrations[..migration_index + 1]; for (action_index, action) in migration.actions.iter().enumerate() { - print!(" + {} ", action.describe()); + let description = action.describe(); + print!(" + {} ", description); let ctx = MigrationContext::new(migration_index, action_index); - result = action.run(&ctx, &mut self.db, &new_schema); + result = action + .run(&ctx, &mut self.db, &new_schema) + .with_context(|| format!("failed to {}", description)); if result.is_ok() { action.update_schema(&ctx, &mut new_schema); @@ -133,16 +137,22 @@ impl Reshape { // Create schema and views for migration let target_migration = remaining_migrations.last().unwrap().name.to_string(); - self.create_schema_for_migration(&target_migration, &new_schema)?; + self.create_schema_for_migration(&target_migration, &new_schema) + .with_context(|| { + format!("failed to create schema for migration {}", target_migration) + })?; // Update state once migrations have been performed self.state.in_progress(remaining_migrations); - self.state.save(&mut self.db)?; + self.state + .save(&mut self.db) + .context("failed to save in-progress state")?; // If we started from a blank slate, we can finish the migration immediately if current_migration.is_none() { println!("Automatically completing migrations\n"); - self.complete_migration()?; + self.complete_migration() + .context("failed to automatically complete migrations")?; println!("Migrations complete:"); println!( @@ -176,41 +186,61 @@ impl Reshape { } }; - helpers::teardown_helpers(&mut self.db)?; + helpers::tear_down_helpers(&mut self.db).context("failed to tear down helpers")?; let mut temp_schema = Schema::new(); // Run all the completion changes as a transaction to avoid incomplete updates - let mut transaction = self.db.transaction()?; + let mut transaction = self + .db + .transaction() + .context("failed to start transaction")?; // Remove previous migration's schema if let Some(current_migration) = &self.state.current_migration { - transaction.run(&format!( - "DROP SCHEMA IF EXISTS {} CASCADE", - schema_name_for_migration(current_migration) - ))?; + transaction + .run(&format!( + "DROP SCHEMA IF EXISTS {} CASCADE", + schema_name_for_migration(current_migration) + )) + .context("failed to remove previous migration's schema")?; } for (migration_index, migration) in remaining_migrations.iter().enumerate() { println!("Completing '{}':", migration.name); for (action_index, action) in migration.actions.iter().enumerate() { - print!(" + {} ", action.describe()); + let description = action.describe(); + print!(" + {} ", description); let ctx = MigrationContext::new(migration_index, action_index); - action.complete(&ctx, &mut transaction, &temp_schema)?; - action.update_schema(&ctx, &mut temp_schema); + let result = action + .complete(&ctx, &mut transaction, &temp_schema) + .with_context(|| format!("failed to complete migration {}", migration.name)) + .with_context(|| format!("failed to complete action: {}", description)); - println!("{}", "done".green()); + if result.is_ok() { + action.update_schema(&ctx, &mut temp_schema); + println!("{}", "done".green()); + } else { + println!("{}", "failed".green()); + return result; + } } println!(""); } - self.state.complete()?; - self.state.save(&mut transaction)?; + self.state + .complete() + .context("failed to update state as completed")?; + self.state + .save(&mut transaction) + .context("failed to save state after setting as completed")?; - transaction.commit()?; + transaction + .commit() + .context("failed to apply transaction")?; Ok(()) } @@ -223,7 +253,13 @@ impl Reshape { // Create schema for migration let schema_name = schema_name_for_migration(migration_name); self.db - .run(&format!("CREATE SCHEMA IF NOT EXISTS {}", schema_name))?; + .run(&format!("CREATE SCHEMA IF NOT EXISTS {}", schema_name)) + .with_context(|| { + format!( + "failed to create schema {} for migration {}", + schema_name, migration_name + ) + })?; // Create views inside schema for table in schema.get_tables(&mut self.db)? { @@ -252,7 +288,8 @@ impl Reshape { table_name = table.real_name, view_name = table.name, columns = select_columns.join(","), - ))?; + )) + .with_context(|| format!("failed to create view for table {}", table.name))?; Ok(()) } @@ -300,16 +337,19 @@ impl Reshape { }; let target_migration = remaining_migrations.last().unwrap().name.to_string(); - helpers::teardown_helpers(&mut self.db)?; + helpers::tear_down_helpers(&mut self.db).context("failed to tear down helpers")?; // Run all the abort changes as a transaction to avoid incomplete changes - let mut transaction = self.db.transaction()?; + let mut transaction = self + .db + .transaction() + .context("failed to start transaction")?; // Remove new migration's schema - transaction.run(&format!( - "DROP SCHEMA IF EXISTS {} CASCADE", - schema_name_for_migration(&target_migration) - ))?; + let schema_name = schema_name_for_migration(&target_migration); + transaction + .run(&format!("DROP SCHEMA IF EXISTS {} CASCADE", schema_name,)) + .with_context(|| format!("failed to drop schema {}", schema_name))?; // Abort all pending migrations abort_migrations(&mut transaction, &remaining_migrations)?; @@ -317,9 +357,13 @@ impl Reshape { let keep_count = self.state.migrations.len() - remaining_migrations.len(); self.state.migrations.truncate(keep_count); self.state.status = state::Status::Idle; - self.state.save(&mut transaction)?; + self.state + .save(&mut transaction) + .context("failed to save state")?; - transaction.commit()?; + transaction + .commit() + .context("failed to commit transaction")?; Ok(()) } @@ -331,7 +375,10 @@ fn abort_migrations(db: &mut dyn Conn, migrations: &[Migration]) -> anyhow::Resu print!("Aborting '{}' ", migration.name); for (action_index, action) in migration.actions.iter().rev().enumerate() { let ctx = MigrationContext::new(migration_index, action_index); - action.abort(&ctx, db)?; + action + .abort(&ctx, db) + .with_context(|| format!("failed to abort migration {}", migration.name)) + .with_context(|| format!("failed to abort action: {}", action.describe()))?; } println!("{}", "done".green()); } diff --git a/src/migrations/add_column.rs b/src/migrations/add_column.rs index 906a058..a3f42bc 100644 --- a/src/migrations/add_column.rs +++ b/src/migrations/add_column.rs @@ -1,5 +1,6 @@ use super::{common, Action, Column, MigrationContext}; use crate::{db::Conn, schema::Schema}; +use anyhow::Context; use serde::{Deserialize, Serialize}; #[derive(Serialize, Deserialize, Debug)] @@ -80,7 +81,7 @@ impl Action for AddColumn { table = self.table, definition = definition_parts.join(" "), ); - db.run(&query)?; + db.run(&query).context("failed to add column")?; if let Some(up) = &self.up { let table = schema.get_table(db, &self.table)?; @@ -124,12 +125,13 @@ impl Action for AddColumn { table = self.table, declarations = declarations.join("\n"), ); - db.run(&query)?; + db.run(&query).context("failed to create up trigger")?; } // Backfill values in batches if self.up.is_some() { - common::batch_touch_rows(db, &table.real_name, &temp_column_name)?; + common::batch_touch_rows(db, &table.real_name, &temp_column_name) + .context("failed to batch update existing rows")?; } // Add a temporary NOT NULL constraint if the column shouldn't be nullable. @@ -147,7 +149,8 @@ impl Action for AddColumn { constraint_name = self.not_null_constraint_name(&ctx), column = temp_column_name, ); - db.run(&query)?; + db.run(&query) + .context("failed to add NOT NULL constraint")?; } Ok(()) @@ -170,7 +173,7 @@ impl Action for AddColumn { table = self.table, trigger_name = self.trigger_name(ctx), ); - db.run(&query)?; + db.run(&query).context("failed to drop up trigger")?; // Update column to be NOT NULL if necessary if !self.column.nullable { @@ -184,7 +187,8 @@ impl Action for AddColumn { table = self.table, constraint_name = self.not_null_constraint_name(ctx), ); - db.run(&query)?; + db.run(&query) + .context("failed to validate NOT NULL constraint")?; // Update the column to be NOT NULL. // This requires an exclusive lock but since PG 12 it can check @@ -198,7 +202,7 @@ impl Action for AddColumn { table = self.table, column = self.temp_column_name(ctx), ); - db.run(&query)?; + db.run(&query).context("failed to set column as NOT NULL")?; // Drop the temporary constraint let query = format!( @@ -209,7 +213,8 @@ impl Action for AddColumn { table = self.table, constraint_name = self.not_null_constraint_name(ctx), ); - db.run(&query)?; + db.run(&query) + .context("failed to drop NOT NULL constraint")?; } // Rename the temporary column to its real name @@ -221,7 +226,8 @@ impl Action for AddColumn { table = table.real_name, temp_column_name = self.temp_column_name(ctx), column_name = self.column.name, - ))?; + )) + .context("failed to rename column to final name")?; Ok(()) } @@ -244,7 +250,7 @@ impl Action for AddColumn { table = self.table, trigger_name = self.trigger_name(ctx), ); - db.run(&query)?; + db.run(&query).context("failed to drop up trigger")?; // Remove column let query = format!( @@ -255,7 +261,7 @@ impl Action for AddColumn { table = self.table, column = self.temp_column_name(ctx), ); - db.run(&query)?; + db.run(&query).context("failed to drop column")?; Ok(()) } diff --git a/src/migrations/add_index.rs b/src/migrations/add_index.rs index 9bc4c0a..235c38c 100644 --- a/src/migrations/add_index.rs +++ b/src/migrations/add_index.rs @@ -1,5 +1,6 @@ use super::{Action, MigrationContext}; use crate::{db::Conn, schema::Schema}; +use anyhow::Context; use serde::{Deserialize, Serialize}; #[derive(Serialize, Deserialize, Debug)] @@ -37,7 +38,8 @@ impl Action for AddIndex { name = self.name, table = self.table, columns = column_real_names.join(", "), - ))?; + )) + .context("failed to create index")?; Ok(()) } @@ -59,7 +61,8 @@ impl Action for AddIndex { ", name = self.name, table = self.table, - ))?; + )) + .context("failed to drop index")?; Ok(()) } } diff --git a/src/migrations/alter_column.rs b/src/migrations/alter_column.rs index a8faa9f..d61f3de 100644 --- a/src/migrations/alter_column.rs +++ b/src/migrations/alter_column.rs @@ -1,6 +1,6 @@ use super::{Action, MigrationContext}; use crate::{db::Conn, migrations::common, schema::Schema}; -use anyhow::anyhow; +use anyhow::{anyhow, Context}; use serde::{Deserialize, Serialize}; #[derive(Serialize, Deserialize, Debug)] @@ -90,7 +90,7 @@ impl Action for AlterColumn { .columns .iter() .find(|column| column.name == self.column) - .ok_or_else(|| anyhow!("no such column exists"))?; + .ok_or_else(|| anyhow!("no such column {} exists", self.column))?; let temporary_column_type = self .changes @@ -108,7 +108,7 @@ impl Action for AlterColumn { temp_column = self.temporary_column_name(ctx), temp_column_type = temporary_column_type, ); - db.run(&query)?; + db.run(&query).context("failed to add temporary column")?; let declarations: Vec = table .columns @@ -173,10 +173,12 @@ impl Action for AlterColumn { down_trigger = self.down_trigger_name(ctx), declarations = declarations.join("\n"), ); - db.run(&query)?; + db.run(&query) + .context("failed to create up and down triggers")?; // Backfill values in batches by touching the previous column - common::batch_touch_rows(db, &table.real_name, &column.real_name)?; + common::batch_touch_rows(db, &table.real_name, &column.real_name) + .context("failed to batch update existing rows")?; // Add a temporary NOT NULL constraint if the column shouldn't be nullable. // This constraint is set as NOT VALID so it doesn't apply to existing rows and @@ -193,7 +195,8 @@ impl Action for AlterColumn { constraint_name = self.not_null_constraint_name(ctx), column = self.temporary_column_name(ctx), ); - db.run(&query)?; + db.run(&query) + .context("failed to add NOT NULL constraint")?; } Ok(()) @@ -216,7 +219,7 @@ impl Action for AlterColumn { existing_name = self.column, new_name = new_name, ); - db.run(&query)?; + db.run(&query).context("failed to rename column")?; } return Ok(()); } @@ -237,7 +240,7 @@ impl Action for AlterColumn { ", self.table, column.real_name ); - db.run(&query)?; + db.run(&query).context("failed to drop old column")?; // Rename temporary column let query = format!( @@ -248,7 +251,8 @@ impl Action for AlterColumn { temp_column = self.temporary_column_name(ctx), name = column_name, ); - db.run(&query)?; + db.run(&query) + .context("failed to rename temporary column")?; // Remove triggers and procedures let query = format!( @@ -263,7 +267,8 @@ impl Action for AlterColumn { up_trigger = self.up_trigger_name(ctx), down_trigger = self.down_trigger_name(ctx), ); - db.run(&query)?; + db.run(&query) + .context("failed to drop up and down triggers")?; // Update column to be NOT NULL if necessary if !column.nullable { @@ -277,7 +282,8 @@ impl Action for AlterColumn { table = self.table, constraint_name = self.not_null_constraint_name(ctx), ); - db.run(&query)?; + db.run(&query) + .context("failed to validate NOT NULL constraint")?; // Update the column to be NOT NULL. // This requires an exclusive lock but since PG 12 it can check @@ -291,7 +297,7 @@ impl Action for AlterColumn { table = self.table, column = column_name, ); - db.run(&query)?; + db.run(&query).context("failed to set column as NOT NULL")?; // Drop the temporary constraint let query = format!( @@ -302,7 +308,8 @@ impl Action for AlterColumn { table = self.table, constraint_name = self.not_null_constraint_name(ctx), ); - db.run(&query)?; + db.run(&query) + .context("failed to drop NOT NULL constraint")?; } Ok(()) @@ -344,7 +351,8 @@ impl Action for AlterColumn { up_trigger = self.up_trigger_name(ctx), down_trigger = self.down_trigger_name(ctx), ); - db.run(&query)?; + db.run(&query) + .context("failed to drop up and down triggers")?; // Drop temporary column let query = format!( @@ -355,7 +363,7 @@ impl Action for AlterColumn { table = self.table, temp_column = self.temporary_column_name(ctx), ); - db.run(&query)?; + db.run(&query).context("failed to drop temporary column")?; Ok(()) } diff --git a/src/migrations/create_table.rs b/src/migrations/create_table.rs index 6de6264..547e28f 100644 --- a/src/migrations/create_table.rs +++ b/src/migrations/create_table.rs @@ -1,5 +1,6 @@ use super::{Action, Column, MigrationContext}; use crate::{db::Conn, schema::Schema}; +use anyhow::Context; use serde::{Deserialize, Serialize}; #[derive(Serialize, Deserialize, Debug)] @@ -71,7 +72,8 @@ impl Action for CreateTable { )", self.name, definition_rows.join(",\n"), - ))?; + )) + .context("failed to create table")?; Ok(()) } @@ -88,8 +90,8 @@ impl Action for CreateTable { fn update_schema(&self, _ctx: &MigrationContext, _schema: &mut Schema) {} fn abort(&self, _ctx: &MigrationContext, db: &mut dyn Conn) -> anyhow::Result<()> { - let query = format!("DROP TABLE IF EXISTS {table}", table = self.name,); - db.run(&query)?; + db.run(&format!("DROP TABLE IF EXISTS {}", self.name,)) + .context("failed to drop table")?; Ok(()) } diff --git a/src/migrations/remove_column.rs b/src/migrations/remove_column.rs index 7b351fa..08e4b71 100644 --- a/src/migrations/remove_column.rs +++ b/src/migrations/remove_column.rs @@ -1,5 +1,6 @@ use super::{Action, MigrationContext}; use crate::{db::Conn, schema::Schema}; +use anyhow::Context; use serde::{Deserialize, Serialize}; #[derive(Serialize, Deserialize, Debug)] @@ -77,7 +78,7 @@ impl Action for RemoveColumn { table = self.table, declarations = declarations.join("\n"), ); - db.run(&query)?; + db.run(&query).context("failed to create down trigger")?; } Ok(()) @@ -102,7 +103,8 @@ impl Action for RemoveColumn { column = self.column, trigger_name = self.trigger_name(&ctx), ); - db.run(&query)?; + db.run(&query) + .context("failed to drop column and down trigger")?; Ok(()) } @@ -124,7 +126,8 @@ impl Action for RemoveColumn { ", table = self.table, trigger_name = self.trigger_name(&ctx), - ))?; + )) + .context("failed to drop down trigger")?; Ok(()) } diff --git a/src/migrations/remove_table.rs b/src/migrations/remove_table.rs index d9203f9..3896b1b 100644 --- a/src/migrations/remove_table.rs +++ b/src/migrations/remove_table.rs @@ -1,5 +1,6 @@ use super::{Action, MigrationContext}; use crate::{db::Conn, schema::Schema}; +use anyhow::Context; use serde::{Deserialize, Serialize}; #[derive(Serialize, Deserialize, Debug)] @@ -35,7 +36,7 @@ impl Action for RemoveTable { ", table = self.table, ); - db.run(&query)?; + db.run(&query).context("failed to drop table")?; Ok(()) } diff --git a/src/migrations/rename_table.rs b/src/migrations/rename_table.rs index 549ba70..06a8c94 100644 --- a/src/migrations/rename_table.rs +++ b/src/migrations/rename_table.rs @@ -1,5 +1,6 @@ use super::{Action, MigrationContext}; use crate::{db::Conn, schema::Schema}; +use anyhow::Context; use serde::{Deserialize, Serialize}; #[derive(Serialize, Deserialize, Debug)] @@ -38,7 +39,7 @@ impl Action for RenameTable { table = self.table, new_name = self.new_name, ); - db.run(&query)?; + db.run(&query).context("failed to rename table")?; Ok(()) }