Structured errors continued

src/app/config.rs

@@ -6,18 +6,14 @@ use crate::app::{ddns, settings, user, vfs};
 
 #[derive(thiserror::Error, Debug)]
 pub enum Error {
+	#[error(transparent)]
+	Ddns(#[from] ddns::Error),
 	#[error(transparent)]
 	Settings(#[from] settings::Error),
 	#[error(transparent)]
 	User(#[from] user::Error),
-	#[error("Unspecified")]
-	Unspecified,
-}
-
-impl From<anyhow::Error> for Error {
-	fn from(_: anyhow::Error) -> Self {
-		Error::Unspecified
-	}
+	#[error(transparent)]
+	Vfs(#[from] vfs::Error),
 }
 
 #[derive(Default, Deserialize)]
@@ -82,9 +78,7 @@ impl Manager {
 				.iter()
 				.filter(|old_user| !users.iter().any(|u| u.name == old_user.name))
 			{
-				self.user_manager
-					.delete(&old_user.name)
-					.map_err(|_| Error::Unspecified)?;
+				self.user_manager.delete(&old_user.name)?;
 			}
 
 			// Insert new users

src/app/ddns.rs

@@ -1,14 +1,31 @@
-use anyhow::bail;
 use diesel::prelude::*;
 use log::{error, info};
 use serde::{Deserialize, Serialize};
 use std::thread;
 use std::time;
 
-use crate::db::{ddns_config, DB};
+use crate::db::{self, ddns_config, DB};
 
 const DDNS_UPDATE_URL: &str = "https://ydns.io/api/v1/update/";
 
+#[derive(thiserror::Error, Debug)]
+pub enum Error {
+	#[error("DDNS update query failed with HTTP status code `{0}`")]
+	UpdateQueryFailed(u16),
+	#[error(transparent)]
+	DatabaseConnection(#[from] db::Error),
+	#[error(transparent)]
+	Database(#[from] diesel::result::Error),
+	#[error("Unspecified")]
+	Unspecified,
+}
+
+impl From<anyhow::Error> for Error {
+	fn from(_: anyhow::Error) -> Self {
+		Error::Unspecified
+	}
+}
+
 #[derive(Clone, Debug, Deserialize, Insertable, PartialEq, Eq, Queryable, Serialize)]
 #[diesel(table_name = ddns_config)]
 pub struct Config {
@@ -27,7 +44,7 @@ impl Manager {
 		Self { db }
 	}
 
-	fn update_my_ip(&self) -> anyhow::Result<()> {
+	fn update_my_ip(&self) -> Result<(), Error> {
 		let config = self.config()?;
 		if config.host.is_empty() || config.username.is_empty() {
 			info!("Skipping DDNS update because credentials are missing");
@@ -39,17 +56,14 @@ impl Manager {
 			.auth(&config.username, &config.password)
 			.call();
 
-		if !response.ok() {
-			bail!(
-				"DDNS update query failed with status code: {}",
-				response.status()
-			);
+		if response.ok() {
+			Ok(())
+		} else {
+			Err(Error::UpdateQueryFailed(response.status()))
 		}
-
-		Ok(())
 	}
 
-	pub fn config(&self) -> anyhow::Result<Config> {
+	pub fn config(&self) -> Result<Config, Error> {
 		use crate::db::ddns_config::dsl::*;
 		let mut connection = self.db.connect()?;
 		Ok(ddns_config
@@ -57,7 +71,7 @@ impl Manager {
 			.get_result(&mut connection)?)
 	}
 
-	pub fn set_config(&self, new_config: &Config) -> anyhow::Result<()> {
+	pub fn set_config(&self, new_config: &Config) -> Result<(), Error> {
 		use crate::db::ddns_config::dsl::*;
 		let mut connection = self.db.connect()?;
 		diesel::update(ddns_config)

src/app/index/query.rs

@@ -5,12 +5,14 @@ use diesel::sql_types;
 use std::path::Path;
 
 use super::*;
-use crate::db::{directories, songs};
+use crate::db::{self, directories, songs};
 
 #[derive(thiserror::Error, Debug)]
 pub enum QueryError {
-	#[error("VFS path not found")]
-	VFSPathNotFound,
+	#[error(transparent)]
+	DatabaseConnection(#[from] db::Error),
+	#[error(transparent)]
+	Vfs(#[from] vfs::Error),
 	#[error("Unspecified")]
 	Unspecified,
 }
@@ -47,9 +49,7 @@ impl Index {
 			output.extend(virtual_directories.map(CollectionFile::Directory));
 		} else {
 			// Browse sub-directory
-			let real_path = vfs
-				.virtual_to_real(virtual_path)
-				.map_err(|_| QueryError::VFSPathNotFound)?;
+			let real_path = vfs.virtual_to_real(virtual_path)?;
 			let real_path_string = real_path.as_path().to_string_lossy().into_owned();
 
 			let real_directories: Vec<Directory> = directories::table
@@ -83,9 +83,7 @@ impl Index {
 		let mut connection = self.db.connect()?;
 
 		let real_songs: Vec<Song> = if virtual_path.as_ref().parent().is_some() {
-			let real_path = vfs
-				.virtual_to_real(virtual_path)
-				.map_err(|_| QueryError::VFSPathNotFound)?;
+			let real_path = vfs.virtual_to_real(virtual_path)?;
 			let song_path_filter = {
 				let mut path_buf = real_path;
 				path_buf.push("%");

src/app/index/update/inserter.rs

@@ -1,4 +1,3 @@
-use anyhow::Error;
 use crossbeam_channel::Receiver;
 use diesel::prelude::*;
 use log::error;
@@ -87,26 +86,26 @@ impl Inserter {
 	}
 
 	fn flush_directories(&mut self) {
-		let res = self.db.connect().and_then(|mut connection| {
+		let res = self.db.connect().ok().and_then(|mut connection| {
 			diesel::insert_into(directories::table)
 				.values(&self.new_directories)
 				.execute(&mut *connection) // TODO https://github.com/diesel-rs/diesel/issues/1822
-				.map_err(Error::new)
+				.ok()
 		});
-		if res.is_err() {
+		if res.is_none() {
 			error!("Could not insert new directories in database");
 		}
 		self.new_directories.clear();
 	}
 
 	fn flush_songs(&mut self) {
-		let res = self.db.connect().and_then(|mut connection| {
+		let res = self.db.connect().ok().and_then(|mut connection| {
 			diesel::insert_into(songs::table)
 				.values(&self.new_songs)
 				.execute(&mut *connection) // TODO https://github.com/diesel-rs/diesel/issues/1822
-				.map_err(Error::new)
+				.ok()
 		});
-		if res.is_err() {
+		if res.is_none() {
 			error!("Could not insert new songs in database");
 		}
 		self.new_songs.clear();

src/app/playlist.rs

@@ -7,14 +7,18 @@ use std::path::Path;
 
 use crate::app::index::Song;
 use crate::app::vfs;
-use crate::db::{playlist_songs, playlists, users, DB};
+use crate::db::{self, playlist_songs, playlists, users, DB};
 
 #[derive(thiserror::Error, Debug)]
 pub enum Error {
+	#[error(transparent)]
+	DatabaseConnection(#[from] db::Error),
 	#[error("User not found")]
 	UserNotFound,
 	#[error("Playlist not found")]
 	PlaylistNotFound,
+	#[error(transparent)]
+	Vfs(#[from] vfs::Error),
 	#[error("Unspecified")]
 	Unspecified,
 }

src/app/settings.rs

@@ -4,12 +4,14 @@ use serde::Deserialize;
 use std::convert::TryInto;
 use std::time::Duration;
 
-use crate::db::{misc_settings, DB};
+use crate::db::{self, misc_settings, DB};
 
 #[derive(thiserror::Error, Debug)]
 pub enum Error {
 	#[error("Missing auth secret")]
 	AuthSecretNotFound,
+	#[error(transparent)]
+	DatabaseConnection(#[from] db::Error),
 	#[error("Auth secret does not have the expected format")]
 	InvalidAuthSecret,
 	#[error("Missing settings")]

src/app/user.rs

@@ -7,10 +7,12 @@ use serde::{Deserialize, Serialize};
 use std::time::{SystemTime, UNIX_EPOCH};
 
 use crate::app::settings::AuthSecret;
-use crate::db::{users, DB};
+use crate::db::{self, users, DB};
 
-#[derive(thiserror::Error, Debug, PartialEq, Eq)]
+#[derive(thiserror::Error, Debug)]
 pub enum Error {
+	#[error(transparent)]
+	DatabaseConnection(#[from] db::Error),
 	#[error("Cannot use empty username")]
 	EmptyUsername,
 	#[error("Cannot use empty password")]
@@ -387,10 +389,10 @@ mod test {
 			password: TEST_PASSWORD.to_owned(),
 			admin: false,
 		};
-		assert_eq!(
+		assert!(matches!(
 			ctx.user_manager.create(&new_user).unwrap_err(),
 			Error::EmptyUsername
-		);
+		));
 	}
 
 	#[test]
@@ -401,10 +403,10 @@ mod test {
 			password: "".to_owned(),
 			admin: false,
 		};
-		assert_eq!(
+		assert!(matches!(
 			ctx.user_manager.create(&new_user).unwrap_err(),
 			Error::EmptyPassword
-		);
+		));
 	}
 
 	#[test]
@@ -455,12 +457,12 @@ mod test {
 		};
 
 		ctx.user_manager.create(&new_user).unwrap();
-		assert_eq!(
+		assert!(matches!(
 			ctx.user_manager
 				.login(TEST_USERNAME, "not the password")
 				.unwrap_err(),
 			Error::IncorrectPassword
-		)
+		));
 	}
 
 	#[test]
@@ -539,9 +541,9 @@ mod test {
 		let authorization = ctx
 			.user_manager
 			.authenticate(&token, AuthorizationScope::PolarisAuth);
-		assert_eq!(
+		assert!(matches!(
 			authorization.unwrap_err(),
 			Error::IncorrectAuthorizationScope
-		)
+		));
 	}
 }

src/app/vfs.rs

@@ -1,11 +1,30 @@
-use anyhow::{bail, Result};
 use core::ops::Deref;
 use diesel::prelude::*;
 use regex::Regex;
 use serde::{Deserialize, Serialize};
 use std::path::{self, Path, PathBuf};
 
-use crate::db::{mount_points, DB};
+use crate::db::{self, mount_points, DB};
+
+#[derive(thiserror::Error, Debug)]
+pub enum Error {
+	#[error("The following real path could not be mapped to a virtual path: `{0}`")]
+	CouldNotMapToVirtualPath(PathBuf),
+	#[error("The following virtual path could not be mapped to a real path: `{0}`")]
+	CouldNotMapToRealPath(PathBuf),
+	#[error(transparent)]
+	DatabaseConnection(#[from] db::Error),
+	#[error(transparent)]
+	Database(#[from] diesel::result::Error),
+	#[error("Unspecified")]
+	Unspecified,
+}
+
+impl From<anyhow::Error> for Error {
+	fn from(_: anyhow::Error) -> Self {
+		Error::Unspecified
+	}
+}
 
 #[derive(Clone, Debug, Deserialize, Insertable, PartialEq, Eq, Queryable, Serialize)]
 #[diesel(table_name = mount_points)]
@@ -44,7 +63,7 @@ impl VFS {
 		VFS { mounts }
 	}
 
-	pub fn real_to_virtual<P: AsRef<Path>>(&self, real_path: P) -> Result<PathBuf> {
+	pub fn real_to_virtual<P: AsRef<Path>>(&self, real_path: P) -> Result<PathBuf, Error> {
 		for mount in &self.mounts {
 			if let Ok(p) = real_path.as_ref().strip_prefix(&mount.source) {
 				let mount_path = Path::new(&mount.name);
@@ -55,10 +74,10 @@ impl VFS {
 				};
 			}
 		}
-		bail!("Real path has no match in VFS")
+		Err(Error::CouldNotMapToVirtualPath(real_path.as_ref().into()))
 	}
 
-	pub fn virtual_to_real<P: AsRef<Path>>(&self, virtual_path: P) -> Result<PathBuf> {
+	pub fn virtual_to_real<P: AsRef<Path>>(&self, virtual_path: P) -> Result<PathBuf, Error> {
 		for mount in &self.mounts {
 			let mount_path = Path::new(&mount.name);
 			if let Ok(p) = virtual_path.as_ref().strip_prefix(mount_path) {
@@ -69,7 +88,7 @@ impl VFS {
 				};
 			}
 		}
-		bail!("Virtual path has no match in VFS")
+		Err(Error::CouldNotMapToRealPath(virtual_path.as_ref().into()))
 	}
 
 	pub fn mounts(&self) -> &Vec<Mount> {
@@ -87,13 +106,13 @@ impl Manager {
 		Self { db }
 	}
 
-	pub fn get_vfs(&self) -> Result<VFS> {
+	pub fn get_vfs(&self) -> Result<VFS, Error> {
 		let mount_dirs = self.mount_dirs()?;
 		let mounts = mount_dirs.into_iter().map(|p| p.into()).collect();
 		Ok(VFS::new(mounts))
 	}
 
-	pub fn mount_dirs(&self) -> Result<Vec<MountDir>> {
+	pub fn mount_dirs(&self) -> Result<Vec<MountDir>, Error> {
 		use self::mount_points::dsl::*;
 		let mut connection = self.db.connect()?;
 		let mount_dirs: Vec<MountDir> = mount_points
@@ -102,7 +121,7 @@ impl Manager {
 		Ok(mount_dirs)
 	}
 
-	pub fn set_mount_dirs(&self, mount_dirs: &[MountDir]) -> Result<()> {
+	pub fn set_mount_dirs(&self, mount_dirs: &[MountDir]) -> Result<(), Error> {
 		let mut connection = self.db.connect()?;
 		connection
 			.transaction::<_, diesel::result::Error, _>(|connection| {

src/db.rs

@@ -1,10 +1,9 @@
-use anyhow::{bail, Error, Result};
 use diesel::r2d2::{self, ConnectionManager, PooledConnection};
 use diesel::sqlite::SqliteConnection;
 use diesel::RunQueryDsl;
 use diesel_migrations::EmbeddedMigrations;
 use diesel_migrations::MigrationHarness;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 
 mod schema;
 
@@ -12,6 +11,18 @@ pub use self::schema::*;
 
 const MIGRATIONS: EmbeddedMigrations = embed_migrations!("migrations");
 
+#[derive(thiserror::Error, Debug)]
+pub enum Error {
+	#[error("Could not initialize database connection pool")]
+	ConnectionPoolBuild,
+	#[error("Could not acquire database connection from pool")]
+	ConnectionPool,
+	#[error("Filesystem error for `{0}`: `{1}`")]
+	Io(PathBuf, std::io::Error),
+	#[error("Could not apply database migrations")]
+	Migration,
+}
+
 #[derive(Clone)]
 pub struct DB {
 	pool: r2d2::Pool<ConnectionManager<SqliteConnection>>,
@@ -39,34 +50,38 @@ impl diesel::r2d2::CustomizeConnection<SqliteConnection, diesel::r2d2::Error>
 }
 
 impl DB {
-	pub fn new(path: &Path) -> Result<DB> {
-		std::fs::create_dir_all(path.parent().unwrap())?;
+	pub fn new(path: &Path) -> Result<DB, Error> {
+		let directory = path.parent().unwrap();
+		std::fs::create_dir_all(directory).map_err(|e| Error::Io(directory.to_owned(), e))?;
 		let manager = ConnectionManager::<SqliteConnection>::new(path.to_string_lossy());
 		let pool = diesel::r2d2::Pool::builder()
 			.connection_customizer(Box::new(ConnectionCustomizer {}))
-			.build(manager)?;
+			.build(manager)
+			.or(Err(Error::ConnectionPoolBuild))?;
 		let db = DB { pool };
 		db.migrate_up()?;
 		Ok(db)
 	}
 
-	pub fn connect(&self) -> Result<PooledConnection<ConnectionManager<SqliteConnection>>> {
-		self.pool.get().map_err(Error::new)
+	pub fn connect(&self) -> Result<PooledConnection<ConnectionManager<SqliteConnection>>, Error> {
+		self.pool.get().or(Err(Error::ConnectionPool))
 	}
 
-	#[allow(dead_code)]
-	fn migrate_down(&self) -> Result<()> {
-		let mut connection = self.connect().unwrap();
-		if let Err(e) = connection.revert_all_migrations(MIGRATIONS) {
-			bail!(e);
-		}
-		Ok(())
+	#[cfg(test)]
+	fn migrate_down(&self) -> Result<(), Error> {
+		let mut connection = self.connect()?;
+		connection
+			.revert_all_migrations(MIGRATIONS)
+			.and(Ok(()))
+			.or(Err(Error::Migration))
 	}
 
-	fn migrate_up(&self) -> Result<()> {
-		let mut connection = self.connect().unwrap();
-		connection.run_pending_migrations(MIGRATIONS).unwrap();
-		Ok(())
+	fn migrate_up(&self) -> Result<(), Error> {
+		let mut connection = self.connect()?;
+		connection
+			.run_pending_migrations(MIGRATIONS)
+			.and(Ok(()))
+			.or(Err(Error::Migration))
 	}
 }
 

src/service/actix/api.rs

@@ -504,7 +504,6 @@ async fn get_audio(
 		let vfs = vfs_manager.get_vfs()?;
 		let path = percent_decode_str(&path).decode_utf8_lossy();
 		vfs.virtual_to_real(Path::new(path.as_ref()))
-			.map_err(|_| APIError::VFSPathNotFound)
 	})
 	.await?;
 
@@ -525,9 +524,7 @@ async fn get_thumbnail(
 	let thumbnail_path = block(move || {
 		let vfs = vfs_manager.get_vfs()?;
 		let path = percent_decode_str(&path).decode_utf8_lossy();
-		let image_path = vfs
-			.virtual_to_real(Path::new(path.as_ref()))
-			.map_err(|_| APIError::VFSPathNotFound)?;
+		let image_path = vfs.virtual_to_real(Path::new(path.as_ref()))?;
 		thumbnails_manager
 			.get_thumbnail(&image_path, &options)
 			.map_err(|_| APIError::Unspecified)

src/service/error.rs

@@ -1,7 +1,8 @@
 use thiserror::Error;
 
 use crate::app::index::QueryError;
-use crate::app::{config, playlist, settings, user};
+use crate::app::{config, ddns, playlist, settings, user, vfs};
+use crate::db;
 
 #[derive(Error, Debug)]
 pub enum APIError {
@@ -48,9 +49,10 @@ impl From<anyhow::Error> for APIError {
 impl From<config::Error> for APIError {
 	fn from(error: config::Error) -> APIError {
 		match error {
+			config::Error::Ddns(e) => e.into(),
 			config::Error::Settings(e) => e.into(),
 			config::Error::User(e) => e.into(),
-			config::Error::Unspecified => APIError::Unspecified,
+			config::Error::Vfs(e) => e.into(),
 		}
 	}
 }
@@ -58,8 +60,10 @@ impl From<config::Error> for APIError {
 impl From<playlist::Error> for APIError {
 	fn from(error: playlist::Error) -> APIError {
 		match error {
+			playlist::Error::DatabaseConnection(e) => e.into(),
 			playlist::Error::PlaylistNotFound => APIError::PlaylistNotFound,
 			playlist::Error::UserNotFound => APIError::UserNotFound,
+			playlist::Error::Vfs(e) => e.into(),
 			playlist::Error::Unspecified => APIError::Unspecified,
 		}
 	}
@@ -68,7 +72,8 @@ impl From<playlist::Error> for APIError {
 impl From<QueryError> for APIError {
 	fn from(error: QueryError) -> APIError {
 		match error {
-			QueryError::VFSPathNotFound => APIError::VFSPathNotFound,
+			QueryError::DatabaseConnection(e) => e.into(),
+			QueryError::Vfs(e) => e.into(),
 			QueryError::Unspecified => APIError::Unspecified,
 		}
 	}
@@ -78,6 +83,7 @@ impl From<settings::Error> for APIError {
 	fn from(error: settings::Error) -> APIError {
 		match error {
 			settings::Error::AuthSecretNotFound => APIError::Internal,
+			settings::Error::DatabaseConnection(e) => e.into(),
 			settings::Error::InvalidAuthSecret => APIError::Internal,
 			settings::Error::MiscSettingsNotFound => APIError::Internal,
 			settings::Error::IndexAlbumArtPatternInvalid => APIError::Internal,
@@ -90,6 +96,7 @@ impl From<settings::Error> for APIError {
 impl From<user::Error> for APIError {
 	fn from(error: user::Error) -> APIError {
 		match error {
+			user::Error::DatabaseConnection(e) => e.into(),
 			user::Error::EmptyUsername => APIError::EmptyUsername,
 			user::Error::EmptyPassword => APIError::EmptyPassword,
 			user::Error::IncorrectUsername => APIError::IncorrectCredentials,
@@ -100,3 +107,37 @@ impl From<user::Error> for APIError {
 		}
 	}
 }
+
+impl From<vfs::Error> for APIError {
+	fn from(error: vfs::Error) -> APIError {
+		match error {
+			vfs::Error::CouldNotMapToVirtualPath(_) => APIError::VFSPathNotFound,
+			vfs::Error::CouldNotMapToRealPath(_) => APIError::VFSPathNotFound,
+			vfs::Error::Database(_) => APIError::Internal,
+			vfs::Error::DatabaseConnection(e) => e.into(),
+			vfs::Error::Unspecified => APIError::Unspecified,
+		}
+	}
+}
+
+impl From<ddns::Error> for APIError {
+	fn from(error: ddns::Error) -> APIError {
+		match error {
+			ddns::Error::DatabaseConnection(e) => e.into(),
+			ddns::Error::UpdateQueryFailed(_) => APIError::Internal,
+			ddns::Error::Database(_) => APIError::Internal,
+			ddns::Error::Unspecified => APIError::Unspecified,
+		}
+	}
+}
+
+impl From<db::Error> for APIError {
+	fn from(error: db::Error) -> APIError {
+		match error {
+			db::Error::ConnectionPoolBuild => APIError::Internal,
+			db::Error::ConnectionPool => APIError::Internal,
+			db::Error::Io(_, _) => APIError::Internal,
+			db::Error::Migration => APIError::Internal,
+		}
+	}
+}