CLI - Don't silent fail when failing to read migration directory

The `migrations_directories` from `migration_internals` silently dropped failing to read the migrations directory by calling `into_iter()` on the result of `read_dir()`.

The error is now propagated as an extra `Result` level on these functions, because keeping the same signature would require putting these errors inside the iterator, which would have required adding a dependency on `either` for the implementation to not be unreasonable.

Author: Ten0

diesel_migrations/migrations_internals/src/lib.rs

@@ -86,26 +86,20 @@ pub fn file_names(path: &Path) -> Result<Vec<String>, std::io::Error> {
 
 pub fn migrations_directories<'a>(
     path: &'a Path,
-) -> impl Iterator<Item = Result<DirEntry, std::io::Error>> + 'a {
-    path.read_dir()
-        .into_iter()
-        .flatten()
-        .filter_map(move |entry| {
-            let entry = match entry {
-                Ok(e) => e,
-                Err(e) => return Some(Err(e)),
-            };
-            let metadata = match entry.metadata() {
-                Ok(m) => m,
-                Err(e) => return Some(Err(e)),
-            };
-            if metadata.is_file() {
-                return None;
-            }
-            if entry.file_name().to_string_lossy().starts_with('.') {
-                None
-            } else {
-                Some(Ok(entry))
-            }
-        })
+) -> Result<impl Iterator<Item = Result<DirEntry, std::io::Error>> + 'a, std::io::Error> {
+    Ok(path.read_dir()?.into_iter().filter_map(|entry_res| {
+        entry_res
+            .and_then(|entry| {
+                Ok(
+                    if entry.metadata()?.is_file()
+                        || entry.file_name().to_string_lossy().starts_with('.')
+                    {
+                        None
+                    } else {
+                        Some(entry)
+                    },
+                )
+            })
+            .transpose()
+    }))
 }

diesel_migrations/migrations_macros/src/embed_migrations.rs

@@ -29,7 +29,7 @@ pub fn expand(path: String) -> proc_macro2::TokenStream {
 fn migration_literals_from_path(
     path: &Path,
 ) -> Result<Vec<proc_macro2::TokenStream>, Box<dyn Error>> {
-    let mut migrations = migrations_directories(path).collect::<Result<Vec<_>, _>>()?;
+    let mut migrations = migrations_directories(path)?.collect::<Result<Vec<_>, _>>()?;
 
     migrations.sort_by_key(DirEntry::path);
 

diesel_migrations/src/file_based_migrations.rs

@@ -86,7 +86,7 @@ impl FileBasedMigrations {
     ///
     /// This methods fails if the path passed as argument is no valid migration directory
     pub fn from_path(path: impl AsRef<Path>) -> Result<Self, MigrationError> {
-        for dir in migrations_directories(path.as_ref()) {
+        for dir in migrations_directories(path.as_ref())? {
             let path = dir?.path();
             if !migrations_internals::valid_sql_migration_directory(&path) {
                 return Err(MigrationError::UnknownMigrationFormat(path));
@@ -134,19 +134,19 @@ fn search_for_migrations_directory(path: &Path) -> Result<PathBuf, MigrationErro
 
 fn migrations_directories<'a>(
     path: &'a Path,
-) -> impl Iterator<Item = Result<DirEntry, MigrationError>> + 'a {
-    migrations_internals::migrations_directories(path).map(move |e| e.map_err(Into::into))
+) -> Result<impl Iterator<Item = Result<DirEntry, MigrationError>> + 'a, MigrationError> {
+    Ok(migrations_internals::migrations_directories(path)?.map(move |e| e.map_err(Into::into)))
 }
 
 fn migrations_in_directory<'a, DB: Backend>(
     path: &'a Path,
-) -> impl Iterator<Item = Result<SqlFileMigration, MigrationError>> + 'a {
-    migrations_directories(path).map(|entry| SqlFileMigration::from_path(&entry?.path()))
+) -> Result<impl Iterator<Item = Result<SqlFileMigration, MigrationError>> + 'a, MigrationError> {
+    Ok(migrations_directories(path)?.map(|entry| SqlFileMigration::from_path(&entry?.path())))
 }
 
 impl<DB: Backend> MigrationSource<DB> for FileBasedMigrations {
     fn migrations(&self) -> migration::Result<Vec<Box<dyn Migration<DB>>>> {
-        migrations_in_directory::<DB>(&self.base_path)
+        migrations_in_directory::<DB>(&self.base_path)?
             .map(|r| Ok(Box::new(r?) as Box<dyn Migration<DB>>))
             .collect()
     }
@@ -355,6 +355,7 @@ mod tests {
         fs::File::create(&file_path).unwrap();
 
         let migrations = migrations_in_directory::<Backend>(&migrations_path)
+            .unwrap()
             .collect::<Result<Vec<_>, _>>()
             .unwrap();
 
@@ -372,6 +373,7 @@ mod tests {
         fs::create_dir(&dot_path).unwrap();
 
         let migrations = migrations_in_directory::<Backend>(&migrations_path)
+            .unwrap()
             .collect::<Result<Vec<_>, _>>()
             .unwrap();