Remove ModelError in favour of strongly-typed errors

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<IncludeExternal>` instead of `Option<&IncludeExternal>`
 - ([#330](https://github.com/ramsayleung/rspotify/pull/330)) `featured_playlists` now accepts `Option<chrono::DateTime<chrono::Utc>>` instead of `Option<&chrono::DateTime<chrono::Utc>>`
 - ([#330](https://github.com/ramsayleung/rspotify/pull/330)) `current_user_top_artists[_manual]` and `current_user_top_tracks[_manual]` now accept `Option<TimeRange>` instead of `Option<&TimeRange>`

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<T: AsRef<Path>>(path: T) -> ModelResult<Self> {
-        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::<Token>(&tok_str)?;
+    pub fn from_cache<T: AsRef<Path>>(path: T) -> Result<Self, ReadTokenCacheError> {
+        let json = read_file(path.as_ref()).map_err(ReadTokenCacheError::Reading)?;
+
+        let tok =
+            serde_json::from_slice::<Token>(&*json).map_err(ReadTokenCacheError::Deserializing)?;
 
         Ok(tok)
     }
 
     /// Saves the token information into its cache file.
-    pub fn write_cache<T: AsRef<Path>>(&self, path: T) -> ModelResult<()> {
-        let token_info = serde_json::to_string(&self)?;
+    pub fn write_cache<T: AsRef<Path>>(&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<Vec<u8>, 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<Path>,
+}
+
+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<Path>,
+}
+
+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;

rspotify-model/src/error.rs

@@ -2,7 +2,6 @@ use serde::Deserialize;
 use thiserror::Error;
 
 pub type ApiResult<T> = Result<T, ApiError>;
-pub type ModelResult<T> = Result<T, ModelError>;
 
 /// 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),
-}

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<Option<Token>> {
+    pub async fn read_token_cache(&self) -> Result<Option<Token>, ReadTokenCacheError> {
         if !self.get_config().token_cached {
             log::info!("Token cache read ignored (not configured)");
             return Ok(None);

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<Option<Token>> {
+    async fn read_token_cache(
+        &self,
+        allow_expired: bool,
+    ) -> Result<Option<Token>, ReadTokenCacheError> {
         if !self.get_config().token_cached {
             log::info!("Auth token cache read ignored (not configured)");
             return Ok(None);

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<T>`