From 8fa01b937de417c43a3d47a4753eeb9aa94e0c48 Mon Sep 17 00:00:00 2001
From: Josiah Glosson <soujournme@gmail.com>
Date: Fri, 29 Aug 2025 07:08:26 -0700
Subject: [PATCH] Small friends fixes (#4270)

* Ensure that fetch errors are properly propagated

* Handle user not found errors better in add_friend

* Cargo fmt

* Introduce new LabrinthError returnable by fetch_advanced

* Allow enter key to send a friend request
---
 Cargo.lock                                    |   1 +
 Cargo.toml                                    |   1 +
 .../src/components/ui/friends/FriendsList.vue |   8 +-
 apps/labrinth/src/models/v3/users.rs          |   2 +-
 apps/labrinth/src/routes/v3/friends.rs        | 190 ++++++++----------
 packages/app-lib/Cargo.toml                   |   1 +
 packages/app-lib/src/error.rs                 |  12 ++
 packages/app-lib/src/state/friends.rs         |  16 +-
 packages/app-lib/src/util/fetch.rs            |  53 +++--
 9 files changed, 149 insertions(+), 135 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 902e2289..630858ee 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -9026,6 +9026,7 @@ dependencies = [
  "daedalus",
  "dashmap",
  "data-url",
+ "derive_more 2.0.1",
  "dirs",
  "discord-rich-presence",
  "dotenvy",
diff --git a/Cargo.toml b/Cargo.toml
index 94a28210..8bdf0437 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -48,6 +48,7 @@ daedalus = { path = "packages/daedalus" }
 dashmap = "6.1.0"
 data-url = "0.3.1"
 deadpool-redis = "0.22.0"
+derive_more = "2.0.1"
 dirs = "6.0.0"
 discord-rich-presence = "0.2.5"
 dotenv-build = "0.1.1"
diff --git a/apps/app-frontend/src/components/ui/friends/FriendsList.vue b/apps/app-frontend/src/components/ui/friends/FriendsList.vue
index a23f911c..edd27712 100644
--- a/apps/app-frontend/src/components/ui/friends/FriendsList.vue
+++ b/apps/app-frontend/src/components/ui/friends/FriendsList.vue
@@ -250,7 +250,13 @@ onUnmounted(() => {
 		<div class="mb-4">
 			<h2 class="m-0 text-lg font-extrabold text-contrast">Username</h2>
 			<p class="m-0 mt-1 leading-tight">You can add friends with their Modrinth username.</p>
-			<input v-model="username" class="mt-2 w-full" type="text" placeholder="Enter username..." />
+			<input
+				v-model="username"
+				class="mt-2 w-full"
+				type="text"
+				placeholder="Enter username..."
+				@keyup.enter="addFriendFromModal"
+			/>
 		</div>
 		<ButtonStyled color="brand">
 			<button :disabled="username.length === 0" @click="addFriendFromModal">
diff --git a/apps/labrinth/src/models/v3/users.rs b/apps/labrinth/src/models/v3/users.rs
index 42d50535..c658a86e 100644
--- a/apps/labrinth/src/models/v3/users.rs
+++ b/apps/labrinth/src/models/v3/users.rs
@@ -189,7 +189,7 @@ impl Role {
 pub struct UserFriend {
     // The user who accepted the friend request
     pub id: UserId,
-    /// THe user who sent the friend request
+    /// The user who sent the friend request
     pub friend_id: UserId,
     pub accepted: bool,
     pub created: DateTime<Utc>,
diff --git a/apps/labrinth/src/routes/v3/friends.rs b/apps/labrinth/src/routes/v3/friends.rs
index fb307fb0..5e99db83 100644
--- a/apps/labrinth/src/routes/v3/friends.rs
+++ b/apps/labrinth/src/routes/v3/friends.rs
@@ -1,5 +1,6 @@
 use crate::auth::get_user_from_headers;
-use crate::database::models::DBUserId;
+use crate::database::models::friend_item::DBFriend;
+use crate::database::models::{DBUser, DBUserId};
 use crate::database::redis::RedisPool;
 use crate::models::pats::Scopes;
 use crate::models::users::UserFriend;
@@ -42,102 +43,94 @@ pub async fn add_friend(
     .1;
 
     let string = info.into_inner().0;
-    let friend =
-        crate::database::models::DBUser::get(&string, &**pool, &redis).await?;
+    let Some(friend) = DBUser::get(&string, &**pool, &redis).await? else {
+        return Err(ApiError::NotFound);
+    };
 
-    if let Some(friend) = friend {
-        let mut transaction = pool.begin().await?;
+    let mut transaction = pool.begin().await?;
 
-        if let Some(friend) =
-            crate::database::models::friend_item::DBFriend::get_friend(
-                user.id.into(),
-                friend.id,
-                &**pool,
-            )
-            .await?
-        {
-            if friend.accepted {
-                return Err(ApiError::InvalidInput(
-                    "You are already friends with this user!".to_string(),
-                ));
-            }
-
-            if !friend.accepted && user.id != friend.friend_id.into() {
-                return Err(ApiError::InvalidInput(
-                    "You cannot accept your own friend request!".to_string(),
-                ));
-            }
-
-            crate::database::models::friend_item::DBFriend::update_friend(
-                friend.user_id,
-                friend.friend_id,
-                true,
-                &mut transaction,
-            )
-            .await?;
-
-            async fn send_friend_status(
-                user_id: DBUserId,
-                friend_id: DBUserId,
-                sockets: &ActiveSockets,
-                redis: &RedisPool,
-            ) -> Result<(), ApiError> {
-                if let Some(friend_status) =
-                    get_user_status(user_id.into(), sockets, redis).await
-                {
-                    broadcast_friends_message(
-                        redis,
-                        RedisFriendsMessage::DirectStatusUpdate {
-                            to_user: friend_id.into(),
-                            status: friend_status,
-                        },
-                    )
-                    .await?;
-                }
-
-                Ok(())
-            }
-
-            send_friend_status(friend.user_id, friend.friend_id, &db, &redis)
-                .await?;
-            send_friend_status(friend.friend_id, friend.user_id, &db, &redis)
-                .await?;
-        } else {
-            if friend.id == user.id.into() {
-                return Err(ApiError::InvalidInput(
-                    "You cannot add yourself as a friend!".to_string(),
-                ));
-            }
-
-            if !friend.allow_friend_requests {
-                return Err(ApiError::InvalidInput(
-                    "Friend requests are disabled for this user!".to_string(),
-                ));
-            }
-
-            crate::database::models::friend_item::DBFriend {
-                user_id: user.id.into(),
-                friend_id: friend.id,
-                created: Utc::now(),
-                accepted: false,
-            }
-            .insert(&mut transaction)
-            .await?;
-
-            send_message_to_user(
-                &db,
-                friend.id.into(),
-                &ServerToClientMessage::FriendRequest { from: user.id },
-            )
-            .await?;
+    if let Some(friend) =
+        DBFriend::get_friend(user.id.into(), friend.id, &**pool).await?
+    {
+        if friend.accepted {
+            return Err(ApiError::InvalidInput(
+                "You are already friends with this user!".to_string(),
+            ));
         }
 
-        transaction.commit().await?;
+        if !friend.accepted && user.id != friend.friend_id.into() {
+            return Err(ApiError::InvalidInput(
+                "You cannot accept your own friend request!".to_string(),
+            ));
+        }
 
-        Ok(HttpResponse::NoContent().body(""))
+        DBFriend::update_friend(
+            friend.user_id,
+            friend.friend_id,
+            true,
+            &mut transaction,
+        )
+        .await?;
+
+        async fn send_friend_status(
+            user_id: DBUserId,
+            friend_id: DBUserId,
+            sockets: &ActiveSockets,
+            redis: &RedisPool,
+        ) -> Result<(), ApiError> {
+            if let Some(friend_status) =
+                get_user_status(user_id.into(), sockets, redis).await
+            {
+                broadcast_friends_message(
+                    redis,
+                    RedisFriendsMessage::DirectStatusUpdate {
+                        to_user: friend_id.into(),
+                        status: friend_status,
+                    },
+                )
+                .await?;
+            }
+
+            Ok(())
+        }
+
+        send_friend_status(friend.user_id, friend.friend_id, &db, &redis)
+            .await?;
+        send_friend_status(friend.friend_id, friend.user_id, &db, &redis)
+            .await?;
     } else {
-        Err(ApiError::NotFound)
+        if friend.id == user.id.into() {
+            return Err(ApiError::InvalidInput(
+                "You cannot add yourself as a friend!".to_string(),
+            ));
+        }
+
+        if !friend.allow_friend_requests {
+            return Err(ApiError::InvalidInput(
+                "Friend requests are disabled for this user!".to_string(),
+            ));
+        }
+
+        DBFriend {
+            user_id: user.id.into(),
+            friend_id: friend.id,
+            created: Utc::now(),
+            accepted: false,
+        }
+        .insert(&mut transaction)
+        .await?;
+
+        send_message_to_user(
+            &db,
+            friend.id.into(),
+            &ServerToClientMessage::FriendRequest { from: user.id },
+        )
+        .await?;
     }
+
+    transaction.commit().await?;
+
+    Ok(HttpResponse::NoContent().body(""))
 }
 
 #[delete("friend/{id}")]
@@ -160,18 +153,12 @@ pub async fn remove_friend(
     .1;
 
     let string = info.into_inner().0;
-    let friend =
-        crate::database::models::DBUser::get(&string, &**pool, &redis).await?;
+    let friend = DBUser::get(&string, &**pool, &redis).await?;
 
     if let Some(friend) = friend {
         let mut transaction = pool.begin().await?;
 
-        crate::database::models::friend_item::DBFriend::remove(
-            user.id.into(),
-            friend.id,
-            &mut transaction,
-        )
-        .await?;
+        DBFriend::remove(user.id.into(), friend.id, &mut transaction).await?;
 
         send_message_to_user(
             &db,
@@ -205,12 +192,7 @@ pub async fn friends(
     .await?
     .1;
 
-    let friends =
-        crate::database::models::friend_item::DBFriend::get_user_friends(
-            user.id.into(),
-            None,
-            &**pool,
-        )
+    let friends = DBFriend::get_user_friends(user.id.into(), None, &**pool)
         .await?
         .into_iter()
         .map(UserFriend::from)
diff --git a/packages/app-lib/Cargo.toml b/packages/app-lib/Cargo.toml
index dc01dd23..659a9d9c 100644
--- a/packages/app-lib/Cargo.toml
+++ b/packages/app-lib/Cargo.toml
@@ -35,6 +35,7 @@ bytemuck.workspace = true
 rgb.workspace = true
 phf.workspace = true
 itertools.workspace = true
+derive_more = { workspace = true, features = ["display"] }
 
 chrono = { workspace = true, features = ["serde"] }
 daedalus.workspace = true
diff --git a/packages/app-lib/src/error.rs b/packages/app-lib/src/error.rs
index 773d55da..2f993f71 100644
--- a/packages/app-lib/src/error.rs
+++ b/packages/app-lib/src/error.rs
@@ -3,8 +3,17 @@ use std::sync::Arc;
 
 use crate::{profile, util};
 use data_url::DataUrlError;
+use derive_more::Display;
+use serde::{Deserialize, Serialize};
 use tracing_error::InstrumentError;
 
+#[derive(Serialize, Deserialize, Debug, Display)]
+#[display("{description}")]
+pub struct LabrinthError {
+    pub error: String,
+    pub description: String,
+}
+
 #[derive(thiserror::Error, Debug)]
 pub enum ErrorKind {
     #[error("Filesystem error: {0}")]
@@ -56,6 +65,9 @@ pub enum ErrorKind {
     #[error("Error fetching URL: {0}")]
     FetchError(#[from] reqwest::Error),
 
+    #[error("{0}")]
+    LabrinthError(LabrinthError),
+
     #[error("Websocket error: {0}")]
     WSError(#[from] async_tungstenite::tungstenite::Error),
 
diff --git a/packages/app-lib/src/state/friends.rs b/packages/app-lib/src/state/friends.rs
index 0d3a9e2a..5cbd086e 100644
--- a/packages/app-lib/src/state/friends.rs
+++ b/packages/app-lib/src/state/friends.rs
@@ -1,3 +1,4 @@
+use crate::ErrorKind;
 use crate::data::ModrinthCredentials;
 use crate::event::FriendPayload;
 use crate::event::emit::emit_friend;
@@ -322,7 +323,7 @@ impl FriendsSocket {
         exec: impl sqlx::Executor<'_, Database = sqlx::Sqlite> + Copy,
         semaphore: &FetchSemaphore,
     ) -> crate::Result<()> {
-        fetch_advanced(
+        let result = fetch_advanced(
             Method::POST,
             &format!("{}friend/{user_id}", env!("MODRINTH_API_URL_V3")),
             None,
@@ -332,7 +333,18 @@ impl FriendsSocket {
             semaphore,
             exec,
         )
-        .await?;
+        .await;
+
+        if let Err(ref e) = result
+            && let ErrorKind::LabrinthError(e) = &*e.raw
+            && e.error == "not_found"
+        {
+            return Err(ErrorKind::OtherError(format!(
+                "No user found with username \"{user_id}\""
+            ))
+            .into());
+        }
+        result?;
 
         Ok(())
     }
diff --git a/packages/app-lib/src/util/fetch.rs b/packages/app-lib/src/util/fetch.rs
index 24d94aee..6e14fd04 100644
--- a/packages/app-lib/src/util/fetch.rs
+++ b/packages/app-lib/src/util/fetch.rs
@@ -1,5 +1,6 @@
 //! Functions for fetching information from the Internet
 use super::io::{self, IOError};
+use crate::ErrorKind;
 use crate::event::LoadingBarId;
 use crate::event::emit::emit_loading;
 use bytes::Bytes;
@@ -108,32 +109,31 @@ pub async fn fetch_advanced(
 
         let result = req.send().await;
         match result {
-            Ok(x) => {
-                if x.status().is_server_error() {
-                    if attempt <= FETCH_ATTEMPTS {
-                        continue;
-                    } else {
-                        return Err(crate::Error::from(
-                            crate::ErrorKind::OtherError(
-                                "Server error when fetching content"
-                                    .to_string(),
-                            ),
-                        ));
+            Ok(resp) => {
+                if resp.status().is_server_error() && attempt <= FETCH_ATTEMPTS
+                {
+                    continue;
+                }
+                if resp.status().is_client_error()
+                    || resp.status().is_server_error()
+                {
+                    let backup_error = resp.error_for_status_ref().unwrap_err();
+                    if let Ok(error) = resp.json().await {
+                        return Err(ErrorKind::LabrinthError(error).into());
                     }
+                    return Err(backup_error.into());
                 }
 
                 let bytes = if let Some((bar, total)) = &loading_bar {
-                    let length = x.content_length();
+                    let length = resp.content_length();
                     if let Some(total_size) = length {
                         use futures::StreamExt;
-                        let mut stream = x.bytes_stream();
+                        let mut stream = resp.bytes_stream();
                         let mut bytes = Vec::new();
                         while let Some(item) = stream.next().await {
-                            let chunk = item.or(Err(
-                                crate::error::ErrorKind::NoValueFor(
-                                    "fetch bytes".to_string(),
-                                ),
-                            ))?;
+                            let chunk = item.or(Err(ErrorKind::NoValueFor(
+                                "fetch bytes".to_string(),
+                            )))?;
                             bytes.append(&mut chunk.to_vec());
                             emit_loading(
                                 bar,
@@ -145,10 +145,10 @@ pub async fn fetch_advanced(
 
                         Ok(bytes::Bytes::from(bytes))
                     } else {
-                        x.bytes().await
+                        resp.bytes().await
                     }
                 } else {
-                    x.bytes().await
+                    resp.bytes().await
                 };
 
                 if let Ok(bytes) = bytes {
@@ -158,7 +158,7 @@ pub async fn fetch_advanced(
                             if attempt <= FETCH_ATTEMPTS {
                                 continue;
                             } else {
-                                return Err(crate::ErrorKind::HashError(
+                                return Err(ErrorKind::HashError(
                                     sha1.to_string(),
                                     hash,
                                 )
@@ -194,10 +194,9 @@ pub async fn fetch_mirrors(
     exec: impl sqlx::Executor<'_, Database = sqlx::Sqlite> + Copy,
 ) -> crate::Result<Bytes> {
     if mirrors.is_empty() {
-        return Err(crate::ErrorKind::InputError(
-            "No mirrors provided!".to_string(),
-        )
-        .into());
+        return Err(
+            ErrorKind::InputError("No mirrors provided!".to_string()).into()
+        );
     }
 
     for (index, mirror) in mirrors.iter().enumerate() {
@@ -276,8 +275,8 @@ pub async fn write(
 }
 
 pub async fn copy(
-    src: impl AsRef<std::path::Path>,
-    dest: impl AsRef<std::path::Path>,
+    src: impl AsRef<Path>,
+    dest: impl AsRef<Path>,
     semaphore: &IoSemaphore,
 ) -> crate::Result<()> {
     let src: &Path = src.as_ref();