Error cleanup

src/app/index/query.rs

@@ -1,16 +1,19 @@
-use anyhow::bail;
 use diesel::dsl::sql;
 use diesel::prelude::*;
 use diesel::sql_types;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 
 use super::*;
 use crate::db::{self, directories, songs};
 
 #[derive(thiserror::Error, Debug)]
 pub enum QueryError {
+	#[error(transparent)]
+	Database(#[from] diesel::result::Error),
 	#[error(transparent)]
 	DatabaseConnection(#[from] db::Error),
+	#[error("Song was not found: `{0}`")]
+	SongNotFound(PathBuf),
 	#[error(transparent)]
 	Vfs(#[from] vfs::Error),
 	#[error("Unspecified")]
@@ -178,7 +181,7 @@ impl Index {
 		Ok(output)
 	}
 
-	pub fn get_song(&self, virtual_path: &Path) -> anyhow::Result<Song> {
+	pub fn get_song(&self, virtual_path: &Path) -> Result<Song, QueryError> {
 		let vfs = self.vfs_manager.get_vfs()?;
 		let mut connection = self.db.connect()?;
 
@@ -192,7 +195,7 @@ impl Index {
 
 		match real_song.virtualize(&vfs) {
 			Some(s) => Ok(s),
-			_ => bail!("Missing VFS mapping"),
+			None => Err(QueryError::SongNotFound(real_path)),
 		}
 	}
 }

src/app/lastfm.rs

@@ -1,13 +1,29 @@
-use anyhow::*;
 use rustfm_scrobble::{Scrobble, Scrobbler};
 use std::path::Path;
 use user::AuthToken;
 
-use crate::app::{index::Index, user};
+use crate::app::{
+	index::{Index, QueryError},
+	user,
+};
 
 const LASTFM_API_KEY: &str = "02b96c939a2b451c31dfd67add1f696e";
 const LASTFM_API_SECRET: &str = "0f25a80ceef4b470b5cb97d99d4b3420";
 
+#[derive(thiserror::Error, Debug)]
+pub enum Error {
+	#[error("Failed to authenticate with last.fm")]
+	ScrobblerAuthentication(rustfm_scrobble::ScrobblerError),
+	#[error("Failed to emit last.fm scrobble")]
+	Scrobble(rustfm_scrobble::ScrobblerError),
+	#[error("Failed to emit last.fm now playing update")]
+	NowPlaying(rustfm_scrobble::ScrobblerError),
+	#[error(transparent)]
+	Query(#[from] QueryError),
+	#[error(transparent)]
+	User(#[from] user::Error),
+}
+
 #[derive(Clone)]
 pub struct Manager {
 	index: Index,
@@ -22,44 +38,50 @@ impl Manager {
 		}
 	}
 
-	pub fn generate_link_token(&self, username: &str) -> Result<AuthToken> {
+	pub fn generate_link_token(&self, username: &str) -> Result<AuthToken, Error> {
 		self.user_manager
 			.generate_lastfm_link_token(username)
 			.map_err(|e| e.into())
 	}
 
-	pub fn link(&self, username: &str, lastfm_token: &str) -> Result<()> {
+	pub fn link(&self, username: &str, lastfm_token: &str) -> Result<(), Error> {
 		let mut scrobbler = Scrobbler::new(LASTFM_API_KEY, LASTFM_API_SECRET);
-		let auth_response = scrobbler.authenticate_with_token(lastfm_token)?;
+		let auth_response = scrobbler
+			.authenticate_with_token(lastfm_token)
+			.map_err(Error::ScrobblerAuthentication)?;
 
 		self.user_manager
 			.lastfm_link(username, &auth_response.name, &auth_response.key)
 			.map_err(|e| e.into())
 	}
 
-	pub fn unlink(&self, username: &str) -> Result<()> {
-		self.user_manager.lastfm_unlink(username)
+	pub fn unlink(&self, username: &str) -> Result<(), Error> {
+		self.user_manager
+			.lastfm_unlink(username)
+			.map_err(|e| e.into())
 	}
 
-	pub fn scrobble(&self, username: &str, track: &Path) -> Result<()> {
+	pub fn scrobble(&self, username: &str, track: &Path) -> Result<(), Error> {
 		let mut scrobbler = Scrobbler::new(LASTFM_API_KEY, LASTFM_API_SECRET);
 		let scrobble = self.scrobble_from_path(track)?;
 		let auth_token = self.user_manager.get_lastfm_session_key(username)?;
 		scrobbler.authenticate_with_session_key(&auth_token);
-		scrobbler.scrobble(&scrobble)?;
+		scrobbler.scrobble(&scrobble).map_err(Error::Scrobble)?;
 		Ok(())
 	}
 
-	pub fn now_playing(&self, username: &str, track: &Path) -> Result<()> {
+	pub fn now_playing(&self, username: &str, track: &Path) -> Result<(), Error> {
 		let mut scrobbler = Scrobbler::new(LASTFM_API_KEY, LASTFM_API_SECRET);
 		let scrobble = self.scrobble_from_path(track)?;
 		let auth_token = self.user_manager.get_lastfm_session_key(username)?;
 		scrobbler.authenticate_with_session_key(&auth_token);
-		scrobbler.now_playing(&scrobble)?;
+		scrobbler
+			.now_playing(&scrobble)
+			.map_err(Error::NowPlaying)?;
 		Ok(())
 	}
 
-	fn scrobble_from_path(&self, track: &Path) -> Result<Scrobble> {
+	fn scrobble_from_path(&self, track: &Path) -> Result<Scrobble, Error> {
 		let song = self.index.get_song(track)?;
 		Ok(Scrobble::new(
 			song.artist.as_deref().unwrap_or(""),

src/app/user.rs

@@ -1,4 +1,3 @@
-use anyhow::anyhow;
 use diesel::prelude::*;
 use pbkdf2::password_hash::{PasswordHash, PasswordHasher, PasswordVerifier, SaltString};
 use pbkdf2::Pbkdf2;
@@ -11,6 +10,8 @@ use crate::db::{self, users, DB};
 
 #[derive(thiserror::Error, Debug)]
 pub enum Error {
+	#[error(transparent)]
+	Database(#[from] diesel::result::Error),
 	#[error(transparent)]
 	DatabaseConnection(#[from] db::Error),
 	#[error("Cannot use empty username")]
@@ -25,14 +26,14 @@ pub enum Error {
 	InvalidAuthToken,
 	#[error("Incorrect authorization scope")]
 	IncorrectAuthorizationScope,
-	#[error("Unspecified")]
-	Unspecified,
-}
-
-impl From<anyhow::Error> for Error {
-	fn from(_: anyhow::Error) -> Self {
-		Error::Unspecified
-	}
+	#[error("Last.fm session key is missing")]
+	MissingLastFMSessionKey,
+	#[error("Failed to hash password")]
+	PasswordHashing,
+	#[error("Failed to encode authorization token")]
+	AuthorizationTokenEncoding,
+	#[error("Failed to encode Branca token")]
+	BrancaTokenEncoding,
 }
 
 #[derive(Debug, Insertable, Queryable)]
@@ -104,17 +105,14 @@ impl Manager {
 
 		diesel::insert_into(users::table)
 			.values(&new_user)
-			.execute(&mut connection)
-			.map_err(|_| Error::Unspecified)?;
+			.execute(&mut connection)?;
 		Ok(())
 	}
 
 	pub fn delete(&self, username: &str) -> Result<(), Error> {
 		use crate::db::users::dsl::*;
 		let mut connection = self.db.connect()?;
-		diesel::delete(users.filter(name.eq(username)))
-			.execute(&mut connection)
-			.map_err(|_| Error::Unspecified)?;
+		diesel::delete(users.filter(name.eq(username))).execute(&mut connection)?;
 		Ok(())
 	}
 
@@ -124,8 +122,7 @@ impl Manager {
 		use crate::db::users::dsl::*;
 		diesel::update(users.filter(name.eq(username)))
 			.set(password_hash.eq(hash))
-			.execute(&mut connection)
-			.map_err(|_| Error::Unspecified)?;
+			.execute(&mut connection)?;
 		Ok(())
 	}
 
@@ -134,8 +131,7 @@ impl Manager {
 		let mut connection = self.db.connect()?;
 		diesel::update(users.filter(name.eq(username)))
 			.set(admin.eq(is_admin as i32))
-			.execute(&mut connection)
-			.map_err(|_| Error::Unspecified)?;
+			.execute(&mut connection)?;
 		Ok(())
 	}
 
@@ -160,7 +156,7 @@ impl Manager {
 					Err(Error::IncorrectPassword)
 				}
 			}
-			Err(_) => Err(Error::Unspecified),
+			Err(e) => Err(e.into()),
 		}
 	}
 
@@ -199,16 +195,16 @@ impl Manager {
 
 	fn generate_auth_token(&self, authorization: &Authorization) -> Result<AuthToken, Error> {
 		let serialized_authorization =
-			serde_json::to_string(&authorization).map_err(|_| Error::Unspecified)?;
+			serde_json::to_string(&authorization).or(Err(Error::AuthorizationTokenEncoding))?;
 		branca::encode(
 			serialized_authorization.as_bytes(),
 			&self.auth_secret.key,
 			SystemTime::now()
 				.duration_since(UNIX_EPOCH)
-				.map_err(|_| Error::Unspecified)?
+				.unwrap_or_default()
 				.as_secs() as u32,
 		)
-		.map_err(|_| Error::Unspecified)
+		.or(Err(Error::BrancaTokenEncoding))
 		.map(AuthToken)
 	}
 
@@ -222,10 +218,10 @@ impl Manager {
 	pub fn list(&self) -> Result<Vec<User>, Error> {
 		use crate::db::users::dsl::*;
 		let mut connection = self.db.connect()?;
-		users
+		let listed_users = users
 			.select((name, password_hash, admin))
-			.get_results(&mut connection)
-			.map_err(|_| Error::Unspecified)
+			.get_results(&mut connection)?;
+		Ok(listed_users)
 	}
 
 	pub fn exists(&self, username: &str) -> Result<bool, Error> {
@@ -234,8 +230,7 @@ impl Manager {
 		let results: Vec<String> = users
 			.select(name)
 			.filter(name.eq(username))
-			.get_results(&mut connection)
-			.map_err(|_| Error::Unspecified)?;
+			.get_results(&mut connection)?;
 		Ok(!results.is_empty())
 	}
 
@@ -245,8 +240,7 @@ impl Manager {
 		let is_admin: i32 = users
 			.filter(name.eq(username))
 			.select(admin)
-			.get_result(&mut connection)
-			.map_err(|_| Error::Unspecified)?;
+			.get_result(&mut connection)?;
 		Ok(is_admin != 0)
 	}
 
@@ -256,8 +250,7 @@ impl Manager {
 		let (theme_base, theme_accent, read_lastfm_username) = users
 			.select((web_theme_base, web_theme_accent, lastfm_username))
 			.filter(name.eq(username))
-			.get_result(&mut connection)
-			.map_err(|_| Error::Unspecified)?;
+			.get_result(&mut connection)?;
 		Ok(Preferences {
 			web_theme_base: theme_base,
 			web_theme_accent: theme_accent,
@@ -277,8 +270,7 @@ impl Manager {
 				web_theme_base.eq(&preferences.web_theme_base),
 				web_theme_accent.eq(&preferences.web_theme_accent),
 			))
-			.execute(&mut connection)
-			.map_err(|_| Error::Unspecified)?;
+			.execute(&mut connection)?;
 		Ok(())
 	}
 
@@ -295,8 +287,7 @@ impl Manager {
 				lastfm_username.eq(lastfm_login),
 				lastfm_session_key.eq(session_key),
 			))
-			.execute(&mut connection)
-			.map_err(|_| Error::Unspecified)?;
+			.execute(&mut connection)?;
 		Ok(())
 	}
 
@@ -307,24 +298,21 @@ impl Manager {
 		})
 	}
 
-	pub fn get_lastfm_session_key(&self, username: &str) -> anyhow::Result<String> {
+	pub fn get_lastfm_session_key(&self, username: &str) -> Result<String, Error> {
 		use crate::db::users::dsl::*;
 		let mut connection = self.db.connect()?;
-		let token = users
+		let token: Option<String> = users
 			.filter(name.eq(username))
 			.select(lastfm_session_key)
 			.get_result(&mut connection)?;
-		match token {
-			Some(t) => Ok(t),
-			_ => Err(anyhow!("Missing LastFM credentials")),
-		}
+		token.ok_or(Error::MissingLastFMSessionKey)
 	}
 
 	pub fn is_lastfm_linked(&self, username: &str) -> bool {
 		self.get_lastfm_session_key(username).is_ok()
 	}
 
-	pub fn lastfm_unlink(&self, username: &str) -> anyhow::Result<()> {
+	pub fn lastfm_unlink(&self, username: &str) -> Result<(), Error> {
 		use crate::db::users::dsl::*;
 		let mut connection = self.db.connect()?;
 		let null: Option<String> = None;
@@ -342,7 +330,7 @@ fn hash_password(password: &str) -> Result<String, Error> {
 	let salt = SaltString::generate(&mut OsRng);
 	match Pbkdf2.hash_password(password.as_bytes(), &salt) {
 		Ok(h) => Ok(h.to_string()),
-		Err(_) => Err(Error::Unspecified),
+		Err(_) => Err(Error::PasswordHashing),
 	}
 }
 

src/service/actix/api.rs

@@ -73,22 +73,23 @@ pub fn make_config() -> impl FnOnce(&mut ServiceConfig) + Clone {
 impl ResponseError for APIError {
 	fn status_code(&self) -> StatusCode {
 		match self {
-			APIError::AuthenticationRequired => StatusCode::UNAUTHORIZED,
-			APIError::IncorrectCredentials => StatusCode::UNAUTHORIZED,
-			APIError::EmptyUsername => StatusCode::BAD_REQUEST,
-			APIError::EmptyPassword => StatusCode::BAD_REQUEST,
-			APIError::DeletingOwnAccount => StatusCode::CONFLICT,
-			APIError::OwnAdminPrivilegeRemoval => StatusCode::CONFLICT,
 			APIError::AudioFileIOError => StatusCode::NOT_FOUND,
-			APIError::ThumbnailFileIOError => StatusCode::NOT_FOUND,
+			APIError::AuthenticationRequired => StatusCode::UNAUTHORIZED,
+			APIError::DeletingOwnAccount => StatusCode::CONFLICT,
+			APIError::EmptyPassword => StatusCode::BAD_REQUEST,
+			APIError::EmptyUsername => StatusCode::BAD_REQUEST,
+			APIError::IncorrectCredentials => StatusCode::UNAUTHORIZED,
+			APIError::Internal => StatusCode::INTERNAL_SERVER_ERROR,
 			APIError::LastFMAccountNotLinked => StatusCode::NO_CONTENT,
 			APIError::LastFMLinkContentBase64DecodeError => StatusCode::BAD_REQUEST,
 			APIError::LastFMLinkContentEncodingError => StatusCode::BAD_REQUEST,
-			APIError::UserNotFound => StatusCode::NOT_FOUND,
+			APIError::OwnAdminPrivilegeRemoval => StatusCode::CONFLICT,
 			APIError::PlaylistNotFound => StatusCode::NOT_FOUND,
-			APIError::VFSPathNotFound => StatusCode::NOT_FOUND,
-			APIError::Internal => StatusCode::INTERNAL_SERVER_ERROR,
+			APIError::SongMetadataNotFound => StatusCode::NOT_FOUND,
+			APIError::ThumbnailFileIOError => StatusCode::NOT_FOUND,
 			APIError::Unspecified => StatusCode::INTERNAL_SERVER_ERROR,
+			APIError::UserNotFound => StatusCode::NOT_FOUND,
+			APIError::VFSPathNotFound => StatusCode::NOT_FOUND,
 		}
 	}
 }

src/service/error.rs

@@ -1,7 +1,7 @@
 use thiserror::Error;
 
 use crate::app::index::QueryError;
-use crate::app::{config, ddns, playlist, settings, user, vfs};
+use crate::app::{config, ddns, lastfm, playlist, settings, user, vfs};
 use crate::db;
 
 #[derive(Error, Debug)]
@@ -34,6 +34,8 @@ pub enum APIError {
 	UserNotFound,
 	#[error("Playlist not found")]
 	PlaylistNotFound,
+	#[error("Song not found")]
+	SongMetadataNotFound,
 	#[error("Internal server error")]
 	Internal,
 	#[error("Unspecified")]
@@ -72,7 +74,9 @@ impl From<playlist::Error> for APIError {
 impl From<QueryError> for APIError {
 	fn from(error: QueryError) -> APIError {
 		match error {
+			QueryError::Database(_) => APIError::Internal,
 			QueryError::DatabaseConnection(e) => e.into(),
+			QueryError::SongNotFound(_) => APIError::SongMetadataNotFound,
 			QueryError::Vfs(e) => e.into(),
 			QueryError::Unspecified => APIError::Unspecified,
 		}
@@ -96,6 +100,9 @@ impl From<settings::Error> for APIError {
 impl From<user::Error> for APIError {
 	fn from(error: user::Error) -> APIError {
 		match error {
+			user::Error::AuthorizationTokenEncoding => APIError::Internal,
+			user::Error::BrancaTokenEncoding => APIError::Internal,
+			user::Error::Database(_) => APIError::Internal,
 			user::Error::DatabaseConnection(e) => e.into(),
 			user::Error::EmptyUsername => APIError::EmptyUsername,
 			user::Error::EmptyPassword => APIError::EmptyPassword,
@@ -103,7 +110,8 @@ impl From<user::Error> for APIError {
 			user::Error::IncorrectPassword => APIError::IncorrectCredentials,
 			user::Error::InvalidAuthToken => APIError::IncorrectCredentials,
 			user::Error::IncorrectAuthorizationScope => APIError::IncorrectCredentials,
-			user::Error::Unspecified => APIError::Unspecified,
+			user::Error::PasswordHashing => APIError::Internal,
+			user::Error::MissingLastFMSessionKey => APIError::IncorrectCredentials,
 		}
 	}
 }
@@ -141,3 +149,15 @@ impl From<db::Error> for APIError {
 		}
 	}
 }
+
+impl From<lastfm::Error> for APIError {
+	fn from(error: lastfm::Error) -> APIError {
+		match error {
+			lastfm::Error::ScrobblerAuthentication(_) => APIError::Internal,
+			lastfm::Error::Scrobble(_) => APIError::Internal,
+			lastfm::Error::NowPlaying(_) => APIError::Internal,
+			lastfm::Error::Query(e) => e.into(),
+			lastfm::Error::User(e) => e.into(),
+		}
+	}
+}