Repository: diesel-rs/diesel
File: diesel_cli/src/migrations/mod.rs (Rust)

Code before:
use fd_lock::RwLock;
use std::any::Any;
use std::collections::{HashMap, HashSet};
use std::error::Error;
use std::fmt::Display;
use std::fs::{self, File};
use std::path::{Path, PathBuf};
use std::{env, io};

use crate::database::InferConnection;
use crate::{config::Config, regenerate_schema_if_file_specified};

mod diff_schema;

#[tracing::instrument]
pub(super) fn run_migration_command(matches: &ArgMatches) -> Result<(), crate::errors::Error> {
    match matches
        .subcommand()
        .expect("Clap ensures a subcommand is set")
    {
        ("run", _) => {
            let mut conn = InferConnection::from_matches(matches)?;
            let dir = migrations_dir(matches)?;
            let dir = FileBasedMigrations::from_path(dir.clone()).map_err(|err| match err {
                MigrationError::IoError(err) => crate::errors::Error::IoError(err, Some(dir)),
                err => crate::errors::Error::MigrationError(Box::new(err)),
            })?;
            run_migrations_with_output(&mut conn, dir)?;
            regenerate_schema_if_file_specified(matches)?;
        }
        ("revert", args) => {
            let mut conn = InferConnection::from_matches(matches)?;
            let dir = migrations_dir(matches)?;
            let dir = FileBasedMigrations::from_path(dir.clone()).map_err(|err| match err {
                MigrationError::IoError(err) => crate::errors::Error::IoError(err, Some(dir)),
                err => crate::errors::Error::MigrationError(Box::new(err)),
            })?;
            if args.get_flag("REVERT_ALL") {
                revert_all_migrations_with_output(&mut conn, dir)?;
            } else {
                let number = args
                    .get_one::<u64>("REVERT_NUMBER")
                    .expect("Clap ensure this argument is set");
                for _ in 0..*number {
                    match revert_migration_with_output(&mut conn, dir.clone()) {
                        Ok(_) => {}
                        Err(e) if e.is::<MigrationError>() => {
                            match e.downcast_ref::<MigrationError>() {
                                // If n is larger then the actual number of migrations,
                                // just stop reverting them
                                Some(MigrationError::NoMigrationRun) => break,

Review comment (suggestion):
Is it not possible to improve the `Display` implementation of crate::errors::Error so that it also tries to unpack IO errors from Migration error? (via downcast)

It looks like it would be:
- Cleaner in that there wouldn't be some particular cases where it wouldn't be wrapped
- Simpler in that there wouldn't be so many places where we have this extra logic of wrapping that's not super clear why it's there

Code after:
use fd_lock::RwLock;
use std::any::Any;
use std::collections::{HashMap, HashSet};
use std::error::Error;
use std::fmt::Display;
use std::fs::{self, File};
use std::path::{Path, PathBuf};
use std::{env, io};

use crate::database::InferConnection;
use crate::{config::Config, regenerate_schema_if_file_specified};

mod diff_schema;

#[tracing::instrument]
pub(super) fn run_migration_command(matches: &ArgMatches) -> Result<(), crate::errors::Error> {
    match matches
        .subcommand()
        .expect("Clap ensures a subcommand is set")
    {
        ("run", _) => {
            let (mut conn, dir) = conn_and_migration_dir(matches)?;

            run_migrations_with_output(&mut conn, dir)?;
            regenerate_schema_if_file_specified(matches)?;
        }
        ("revert", args) => {
            let (mut conn, dir) = conn_and_migration_dir(matches)?;

            if args.get_flag("REVERT_ALL") {
                revert_all_migrations_with_output(&mut conn, dir)?;
            } else {
                let number = args
                    .get_one::<u64>("REVERT_NUMBER")
                    .expect("Clap ensure this argument is set");
                for _ in 0..*number {
                    match revert_migration_with_output(&mut conn, dir.clone()) {
                        Ok(_) => {}
                        Err(e) if e.is::<MigrationError>() => {
                            match e.downcast_ref::<MigrationError>() {
                                // If n is larger then the actual number of migrations,
                                // just stop reverting them
                                Some(MigrationError::NoMigrationRun) => break,
                                _ => return Err(crate::errors::Error::MigrationError(e)),
                            }
                        }
                        Err(e) => return Err(crate::errors::Error::MigrationError(e)),
                    }
                }
            }