diff --git a/CHANGELOG.md b/CHANGELOG.md index cd2e2dc6..48cb4108 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ **Breaking changes**: - ([#325](https://github.com/ramsayleung/rspotify/pull/325)) The `auth_code`, `auth_code_pkce`, `client_creds`, `clients::base` and `clients::oauth` modules have been removed from the public API; you should access the same types from their parent modules instead - ([#326](https://github.com/ramsayleung/rspotify/pull/326)) The `rspotify::clients::mutex` module has been renamed to `rspotify::sync` +- ([#327](https://github.com/ramsayleung/rspotify/pull/327)) `ModelError` has been replaced with the more precise error types `ReadTokenCacheError` and `WriteTokenCacheError` - ([#330](https://github.com/ramsayleung/rspotify/pull/330)) `search` now accepts `Option` instead of `Option<&IncludeExternal>` - ([#330](https://github.com/ramsayleung/rspotify/pull/330)) `featured_playlists` now accepts `Option>` instead of `Option<&chrono::DateTime>` - ([#330](https://github.com/ramsayleung/rspotify/pull/330)) `current_user_top_artists[_manual]` and `current_user_top_tracks[_manual]` now accept `Option` instead of `Option<&TimeRange>` diff --git a/rspotify-model/src/auth.rs b/rspotify-model/src/auth.rs index 53c7586e..1225043d 100644 --- a/rspotify-model/src/auth.rs +++ b/rspotify-model/src/auth.rs @@ -1,19 +1,16 @@ //! All objects related to the auth flows defined by Spotify API -use crate::{ - custom_serde::{duration_second, space_separated_scopes}, - ModelResult, -}; +use crate::custom_serde::{duration_second, space_separated_scopes}; use std::{ collections::{HashMap, HashSet}, - fs, - io::{Read, Write}, + fs, io, path::Path, }; use chrono::{DateTime, Duration, Utc}; use serde::{Deserialize, Serialize}; +use thiserror::Error; /// Spotify access token information /// @@ -56,22 +53,23 @@ impl Default for Token { impl Token { /// Tries to initialize the token from a cache file. - pub fn from_cache>(path: T) -> ModelResult { - let mut file = fs::File::open(path)?; - let mut tok_str = String::new(); - file.read_to_string(&mut tok_str)?; - let tok = serde_json::from_str::(&tok_str)?; + pub fn from_cache>(path: T) -> Result { + let json = read_file(path.as_ref()).map_err(ReadTokenCacheError::Reading)?; + + let tok = + serde_json::from_slice::(&*json).map_err(ReadTokenCacheError::Deserializing)?; Ok(tok) } /// Saves the token information into its cache file. - pub fn write_cache>(&self, path: T) -> ModelResult<()> { - let token_info = serde_json::to_string(&self)?; + pub fn write_cache>(&self, path: T) -> Result<(), WriteTokenCacheError> { + // `serde_json::to_vec` only errors if our `Serialize` implementation + // fails, which it shouldn’t, or we try to serialize a map with + // non-string keys, which we don’t. + let json = serde_json::to_vec(&self).unwrap(); - let mut file = fs::OpenOptions::new().write(true).create(true).open(path)?; - file.set_len(0)?; - file.write_all(token_info.as_bytes())?; + write_file(path.as_ref(), &*json).map_err(|inner| WriteTokenCacheError { inner })?; Ok(()) } @@ -95,6 +93,109 @@ impl Token { } } +/// An error reading a cached [`Token`]. +#[derive(Debug, Error)] +#[error("failed to read token from cache")] +pub enum ReadTokenCacheError { + /// There was an error reading the cache file into memory. + Reading(#[source] ReadFileError), + + /// There was an error deserializing the contents of the cache file. + Deserializing(#[source] serde_json::Error), +} + +/// An error writing a [`Token`] to its cache. +#[derive(Debug, Error)] +#[non_exhaustive] +#[error("failed to write token to cache")] +pub struct WriteTokenCacheError { + /// The underlying error in writing the [`Token`] file cache. + #[source] + pub inner: WriteFileError, +} + +fn read_file(path: &Path) -> Result, ReadFileError> { + fs::read(path).map_err(|inner| ReadFileError { + inner, + path: Box::from(path), + }) +} + +/// An error reading a file. +#[derive(Debug, Error)] +#[error("failed to read file {}", path.display())] +pub struct ReadFileError { + // Intentionally not exposed to allow future API evolution, e.g., moving + // this to a enum variants `Open(io::Error)` and `Read(io::Error)` + #[source] + inner: io::Error, + path: Box, +} + +impl ReadFileError { + /// Returns a shared reference to the underlying I/O that caused this. + #[must_use] + pub fn io(&self) -> &io::Error { + &self.inner + } + + /// Consumes this error type, returning the underlying inner I/O error. + /// + /// It is not recommended to use this method just to unify error types, as + /// you will lose valuable information in the error message. + #[must_use] + pub fn into_io(self) -> io::Error { + self.inner + } + + /// Returns a shared reference to the path that could not be read. + #[must_use] + pub fn path(&self) -> &Path { + &*self.path + } +} + +fn write_file(path: &Path, bytes: &[u8]) -> Result<(), WriteFileError> { + fs::write(path, bytes).map_err(|inner| WriteFileError { + inner, + path: Box::from(path), + }) +} + +/// An error writing a file. +#[derive(Debug, Error)] +#[error("failed to write file {}", path.display())] +pub struct WriteFileError { + // Intentionally not exposed to allow future API evolution, e.g., moving + // this to an enum variant `Open(io::Error)` and `Write(io::Error)` + #[source] + inner: io::Error, + path: Box, +} + +impl WriteFileError { + /// Returns a shared reference to the underlying I/O that caused this. + #[must_use] + pub fn io(&self) -> &io::Error { + &self.inner + } + + /// Consumes this error type, returning the underlying inner I/O error. + /// + /// It is not recommended to use this method just to unify error types, as + /// you will lose valuable information in the error message. + #[must_use] + pub fn into_io(self) -> io::Error { + self.inner + } + + /// Returns a shared reference to the path that could not be written to. + #[must_use] + pub fn path(&self) -> &Path { + &*self.path + } +} + #[cfg(test)] mod test { use crate::Token; diff --git a/rspotify-model/src/error.rs b/rspotify-model/src/error.rs index faf9a9b8..a8f068bb 100644 --- a/rspotify-model/src/error.rs +++ b/rspotify-model/src/error.rs @@ -2,7 +2,6 @@ use serde::Deserialize; use thiserror::Error; pub type ApiResult = Result; -pub type ModelResult = Result; /// Matches errors that are returned from the Spotfiy /// API as part of the JSON response object. @@ -22,13 +21,3 @@ pub enum ApiError { reason: String, }, } - -/// Groups up the kinds of errors that may happen in this crate. -#[derive(Debug, Error)] -pub enum ModelError { - #[error("json parse error: {0}")] - ParseJson(#[from] serde_json::Error), - - #[error("input/output error: {0}")] - Io(#[from] std::io::Error), -} diff --git a/src/client_creds.rs b/src/client_creds.rs index f6cd7709..827d2df2 100644 --- a/src/client_creds.rs +++ b/src/client_creds.rs @@ -1,6 +1,7 @@ use crate::{ clients::BaseClient, http::{Form, HttpClient}, + model::ReadTokenCacheError, params, sync::Mutex, ClientResult, Config, Credentials, Token, @@ -95,7 +96,7 @@ impl ClientCredsSpotify { /// * The read token is expired /// * The cached token is disabled in the config #[maybe_async] - pub async fn read_token_cache(&self) -> ClientResult> { + pub async fn read_token_cache(&self) -> Result, ReadTokenCacheError> { if !self.get_config().token_cached { log::info!("Token cache read ignored (not configured)"); return Ok(None); diff --git a/src/clients/oauth.rs b/src/clients/oauth.rs index 61d3c307..4969934e 100644 --- a/src/clients/oauth.rs +++ b/src/clients/oauth.rs @@ -56,7 +56,10 @@ pub trait OAuthClient: BaseClient { /// the application re-authenticate. /// /// [`ClientCredsSpotify::read_token_cache`]: crate::client_creds::ClientCredsSpotify::read_token_cache - async fn read_token_cache(&self, allow_expired: bool) -> ClientResult> { + async fn read_token_cache( + &self, + allow_expired: bool, + ) -> Result, ReadTokenCacheError> { if !self.get_config().token_cached { log::info!("Auth token cache read ignored (not configured)"); return Ok(None); diff --git a/src/lib.rs b/src/lib.rs index 0da9dc15..1c67cdc5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -210,11 +210,8 @@ pub enum ClientError { #[error("cli error: {0}")] Cli(String), - #[error("cache file error: {0}")] - CacheFile(String), - - #[error("model error: {0}")] - Model(#[from] model::ModelError), + #[error("error updating token cache: {0}")] + UpdateTokenCache(#[from] model::WriteTokenCacheError), } // The conversion has to be done manually because it's in a `Box`