Make SpotifyId base62 / base16 methods infallible (#1635)

* make SpotifyId::to_base62() infallible

The only error case is a non-utf8 string, which is impossible since we're building it from a fixed alphabet.

This allows removing a bunch of unnecessary error handling branches.

* make {SpotifyId, FileId}::to_base16() infallible too

* fix core unit tests

* changelog entry

Author: wfraser

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+
+- [core] Made `SpotifyId::to_base62`, `SpotifyId::to_base16`, `FileId::to_base16`, `SpotifyUri::to_id` infallible (breaking)
+
 ## [0.8.0] - 2025-11-10
 
 ### Added

core/src/cache.rs

@@ -378,17 +378,12 @@ impl Cache {
     }
 
     pub fn file_path(&self, file: FileId) -> Option<PathBuf> {
-        match file.to_base16() {
-            Ok(name) => self.audio_location.as_ref().map(|location| {
-                let mut path = location.join(&name[0..2]);
-                path.push(&name[2..]);
-                path
-            }),
-            Err(e) => {
-                warn!("Invalid FileId: {e}");
-                None
-            }
-        }
+        self.audio_location.as_ref().map(|location| {
+            let name = file.to_base16();
+            let mut path = location.join(&name[0..2]);
+            path.push(&name[2..]);
+            path
+        })
     }
 
     pub fn file(&self, file: FileId) -> Option<File> {

core/src/file_id.rs

@@ -1,9 +1,7 @@
-use std::fmt;
+use std::fmt::{self, Write};
 
 use librespot_protocol as protocol;
 
-use crate::{Error, spotify_id::to_base16};
-
 const RAW_LEN: usize = 20;
 
 #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
@@ -21,8 +19,12 @@ impl FileId {
     }
 
     #[allow(clippy::wrong_self_convention)]
-    pub fn to_base16(&self) -> Result<String, Error> {
-        to_base16(&self.0, &mut [0u8; 40])
+    pub fn to_base16(&self) -> String {
+        let mut s = String::new();
+        for b in &self.0 {
+            write!(&mut s, "{b:02x}").unwrap();
+        }
+        s
     }
 }
 
@@ -34,7 +36,7 @@ impl fmt::Debug for FileId {
 
 impl fmt::Display for FileId {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.write_str(&self.to_base16().unwrap_or_default())
+        f.write_str(&self.to_base16())
     }
 }
 

core/src/spclient.rs

@@ -645,7 +645,7 @@ impl SpClient {
     }
 
     pub async fn get_lyrics(&self, track_id: &SpotifyId) -> SpClientResult {
-        let endpoint = format!("/color-lyrics/v2/track/{}", track_id.to_base62()?);
+        let endpoint = format!("/color-lyrics/v2/track/{}", track_id.to_base62());
 
         self.request_as_json(&Method::GET, &endpoint, None, None)
             .await
@@ -658,7 +658,7 @@ impl SpClient {
     ) -> SpClientResult {
         let endpoint = format!(
             "/color-lyrics/v2/track/{}/image/spotify:image:{}",
-            track_id.to_base62()?,
+            track_id.to_base62(),
             image_id
         );
 
@@ -667,7 +667,7 @@ impl SpClient {
     }
 
     pub async fn get_playlist(&self, playlist_id: &SpotifyId) -> SpClientResult {
-        let endpoint = format!("/playlist/v2/playlist/{}", playlist_id.to_base62()?);
+        let endpoint = format!("/playlist/v2/playlist/{}", playlist_id.to_base62());
 
         self.request(&Method::GET, &endpoint, None, None).await
     }
@@ -750,7 +750,7 @@ impl SpClient {
         let previous_track_str = previous_tracks
             .iter()
             .map(|track| track.to_base62())
-            .collect::<Result<Vec<_>, _>>()?
+            .collect::<Vec<_>>()
             .join(",");
         // better than checking `previous_tracks.len() > 0` because the `filter_map` could still return 0 items
         if !previous_track_str.is_empty() {
@@ -773,7 +773,7 @@ impl SpClient {
     pub async fn get_audio_storage(&self, file_id: &FileId) -> SpClientResult {
         let endpoint = format!(
             "/storage-resolve/files/audio/interactive/{}",
-            file_id.to_base16()?
+            file_id.to_base16()
         );
         self.request(&Method::GET, &endpoint, None, None).await
     }
@@ -819,7 +819,7 @@ impl SpClient {
             .get_user_attribute(ATTRIBUTE)
             .ok_or_else(|| SpClientError::Attribute(ATTRIBUTE.to_string()))?;
 
-        let mut url = template.replace("{id}", &preview_id.to_base16()?);
+        let mut url = template.replace("{id}", &preview_id.to_base16());
         let separator = match url.find('?') {
             Some(_) => "&",
             None => "?",
@@ -837,7 +837,7 @@ impl SpClient {
             .get_user_attribute(ATTRIBUTE)
             .ok_or_else(|| SpClientError::Attribute(ATTRIBUTE.to_string()))?;
 
-        let url = template.replace("{file_id}", &file_id.to_base16()?);
+        let url = template.replace("{file_id}", &file_id.to_base16());
 
         self.request_url(&url).await
     }
@@ -848,7 +848,7 @@ impl SpClient {
             .session()
             .get_user_attribute(ATTRIBUTE)
             .ok_or_else(|| SpClientError::Attribute(ATTRIBUTE.to_string()))?;
-        let url = template.replace("{file_id}", &image_id.to_base16()?);
+        let url = template.replace("{file_id}", &image_id.to_base16());
 
         self.request_url(&url).await
     }

core/src/spotify_id.rs

@@ -29,11 +29,9 @@ impl From<SpotifyIdError> for Error {
 pub type SpotifyIdResult = Result<SpotifyId, Error>;
 
 const BASE62_DIGITS: &[u8; 62] = b"0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
-const BASE16_DIGITS: &[u8; 16] = b"0123456789abcdef";
 
 impl SpotifyId {
     const SIZE: usize = 16;
-    const SIZE_BASE16: usize = 32;
     const SIZE_BASE62: usize = 22;
 
     /// Parses a base16 (hex) encoded [Spotify ID] into a `SpotifyId`.
@@ -45,20 +43,9 @@ impl SpotifyId {
         if src.len() != 32 {
             return Err(SpotifyIdError::InvalidId.into());
         }
-        let mut dst: u128 = 0;
-
-        for c in src.as_bytes() {
-            let p = match c {
-                b'0'..=b'9' => c - b'0',
-                b'a'..=b'f' => c - b'a' + 10,
-                _ => return Err(SpotifyIdError::InvalidId.into()),
-            } as u128;
-
-            dst <<= 4;
-            dst += p;
-        }
+        let id = u128::from_str_radix(src, 16).map_err(|_| SpotifyIdError::InvalidId)?;
 
-        Ok(Self { id: dst })
+        Ok(Self { id })
     }
 
     /// Parses a base62 encoded [Spotify ID] into a `u128`.
@@ -99,19 +86,18 @@ impl SpotifyId {
         }
     }
 
-    /// Returns the `SpotifyId` as a base16 (hex) encoded, `SpotifyId::SIZE_BASE16` (32)
-    /// character long `String`.
+    /// Returns the `SpotifyId` as a base16 (hex) encoded, 32-character long `String`.
     #[allow(clippy::wrong_self_convention)]
-    pub fn to_base16(&self) -> Result<String, Error> {
-        to_base16(&self.to_raw(), &mut [0u8; Self::SIZE_BASE16])
+    pub fn to_base16(&self) -> String {
+        format!("{:032x}", self.id)
     }
 
     /// Returns the `SpotifyId` as a [canonically] base62 encoded, `SpotifyId::SIZE_BASE62` (22)
     /// character long `String`.
     ///
     /// [canonically]: https://developer.spotify.com/documentation/web-api/concepts/spotify-uris-ids
     #[allow(clippy::wrong_self_convention)]
-    pub fn to_base62(&self) -> Result<String, Error> {
+    pub fn to_base62(&self) -> String {
         let mut dst = [0u8; 22];
         let mut i = 0;
         let n = self.id;
@@ -143,13 +129,12 @@ impl SpotifyId {
             }
         }
 
-        for b in &mut dst {
-            *b = BASE62_DIGITS[*b as usize];
+        let mut s = String::with_capacity(dst.len());
+        for &b in dst.iter().rev() {
+            s.push(BASE62_DIGITS[b as usize] as char);
         }
 
-        dst.reverse();
-
-        String::from_utf8(dst.to_vec()).map_err(|_| SpotifyIdError::InvalidId.into())
+        s
     }
 
     /// Returns a copy of the `SpotifyId` as an array of `SpotifyId::SIZE` (16) bytes in
@@ -162,15 +147,13 @@ impl SpotifyId {
 
 impl fmt::Debug for SpotifyId {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.debug_tuple("SpotifyId")
-            .field(&self.to_base62().unwrap_or_else(|_| "invalid uri".into()))
-            .finish()
+        f.debug_tuple("SpotifyId").field(&self.to_base62()).finish()
     }
 }
 
 impl fmt::Display for SpotifyId {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.write_str(&self.to_base62().unwrap_or_else(|_| "invalid uri".into()))
+        f.write_str(&self.to_base62())
     }
 }
 
@@ -219,17 +202,6 @@ impl TryFrom<&SpotifyUri> for SpotifyId {
     }
 }
 
-pub fn to_base16(src: &[u8], buf: &mut [u8]) -> Result<String, Error> {
-    let mut i = 0;
-    for v in src {
-        buf[i] = BASE16_DIGITS[(v >> 4) as usize];
-        buf[i + 1] = BASE16_DIGITS[(v & 0x0f) as usize];
-        i += 2;
-    }
-
-    String::from_utf8(buf.to_vec()).map_err(|_| SpotifyIdError::InvalidId.into())
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -350,7 +322,7 @@ mod tests {
         for c in &CONV_VALID {
             let id = SpotifyId { id: c.id };
 
-            assert_eq!(id.to_base62().unwrap(), c.base62);
+            assert_eq!(id.to_base62(), c.base62);
         }
     }
 
@@ -370,7 +342,7 @@ mod tests {
         for c in &CONV_VALID {
             let id = SpotifyId { id: c.id };
 
-            assert_eq!(id.to_base16().unwrap(), c.base16);
+            assert_eq!(id.to_base16(), c.base16);
         }
     }
 

core/src/spotify_uri.rs

@@ -87,7 +87,7 @@ impl SpotifyUri {
 
     /// Gets the ID of this URI. The resource ID is the component of the URI that identifies
     /// the resource after its type label. If `self` is a named ID, the user will be omitted.
-    pub fn to_id(&self) -> Result<String, Error> {
+    pub fn to_id(&self) -> String {
         match &self {
             SpotifyUri::Album { id }
             | SpotifyUri::Artist { id }
@@ -102,11 +102,9 @@ impl SpotifyUri {
                 duration,
             } => {
                 let duration_secs = duration.as_secs();
-                Ok(format!(
-                    "{artist}:{album_title}:{track_title}:{duration_secs}"
-                ))
+                format!("{artist}:{album_title}:{track_title}:{duration_secs}")
             }
-            SpotifyUri::Unknown { id, .. } => Ok(id.clone()),
+            SpotifyUri::Unknown { id, .. } => id.clone(),
         }
     }
 
@@ -207,7 +205,7 @@ impl SpotifyUri {
     /// [Spotify URI]: https://developer.spotify.com/documentation/web-api/concepts/spotify-uris-ids
     pub fn to_uri(&self) -> Result<String, Error> {
         let item_type = self.item_type();
-        let name = self.to_id()?;
+        let name = self.to_id();
 
         if let SpotifyUri::Playlist {
             id,
@@ -226,7 +224,7 @@ impl SpotifyUri {
     /// Deprecated: not all IDs can be represented in Base62, so this function has been renamed to
     /// [SpotifyUri::to_id], which this implementation forwards to.
     #[deprecated(since = "0.8.0", note = "use to_name instead")]
-    pub fn to_base62(&self) -> Result<String, Error> {
+    pub fn to_base62(&self) -> String {
         self.to_id()
     }
 }
@@ -313,7 +311,7 @@ impl TryFrom<&protocol::playlist4_external::MetaItem> for SpotifyUri {
     fn try_from(item: &protocol::playlist4_external::MetaItem) -> Result<Self, Self::Error> {
         Ok(Self::Unknown {
             kind: "MetaItem".into(),
-            id: SpotifyId::try_from(item.revision())?.to_base62()?,
+            id: SpotifyId::try_from(item.revision())?.to_base62(),
         })
     }
 }
@@ -326,7 +324,7 @@ impl TryFrom<&protocol::playlist4_external::SelectedListContent> for SpotifyUri
     ) -> Result<Self, Self::Error> {
         Ok(Self::Unknown {
             kind: "SelectedListContent".into(),
-            id: SpotifyId::try_from(playlist.revision())?.to_base62()?,
+            id: SpotifyId::try_from(playlist.revision())?.to_base62(),
         })
     }
 }
@@ -488,7 +486,7 @@ mod tests {
     #[test]
     fn to_id() {
         for c in &CONV_VALID {
-            assert_eq!(c.parsed.to_id().unwrap(), c.base62);
+            assert_eq!(c.parsed.to_id(), c.base62);
         }
     }
 

metadata/src/playlist/annotation.rs

@@ -58,7 +58,7 @@ impl PlaylistAnnotation {
         let uri = format!(
             "hm://playlist-annotate/v1/annotation/user/{}/playlist/{}",
             username,
-            playlist_id.to_base62()?
+            playlist_id.to_base62()
         );
         <Self as MercuryRequest>::request(session, &uri).await
     }

playback/src/player.rs

@@ -988,10 +988,7 @@ impl PlayerTrackLoader {
             Ok(audio) => match self.find_available_alternative(audio).await {
                 Some(audio) => audio,
                 None => {
-                    warn!(
-                        "spotify:track:<{}> is not available",
-                        track_id.to_base62().unwrap_or_default()
-                    );
+                    warn!("spotify:track:<{}> is not available", track_id.to_base62());
                     return None;
                 }
             },