fix(api): Axum API, error should return a 500 status code

If the handlers return an error Axum does not automatically return an error HTTP code. See https://docs.rs/axum/latest/axum/error_handling/index.html. You have to convert the error into an error response.
torrust · Jun 19, 2023 · b73d864 · b73d864
1 parent b4744e7
commit b73d864
Show file tree

Hide file tree

Showing 5 changed files with 102 additions and 86 deletions.
diff --git a/src/web/api/v1/contexts/category/handlers.rs b/src/web/api/v1/contexts/category/handlers.rs
@@ -3,15 +3,13 @@
 use std::sync::Arc;
 
 use axum::extract::{self, State};
-use axum::response::Json;
+use axum::response::{IntoResponse, Json, Response};
 
 use super::forms::{AddCategoryForm, DeleteCategoryForm};
 use super::responses::{added_category, deleted_category};
 use crate::common::AppData;
-use crate::databases::database::{self, Category};
-use crate::errors::ServiceError;
 use crate::web::api::v1::extractors::bearer_token::Extract;
-use crate::web::api::v1::responses::{self, OkResponseData};
+use crate::web::api::v1::responses::{self};
 
 /// It handles the request to get all the categories.
 ///
@@ -27,12 +25,10 @@ use crate::web::api::v1::responses::{self, OkResponseData};
 ///
 /// It returns an error if there is a database error.
 #[allow(clippy::unused_async)]
-pub async fn get_all_handler(
-    State(app_data): State<Arc<AppData>>,
-) -> Result<Json<responses::OkResponseData<Vec<Category>>>, database::Error> {
+pub async fn get_all_handler(State(app_data): State<Arc<AppData>>) -> Response {
     match app_data.category_repository.get_all().await {
-        Ok(categories) => Ok(Json(responses::OkResponseData { data: categories })),
-        Err(error) => Err(error),
+        Ok(categories) => Json(responses::OkResponseData { data: categories }).into_response(),
+        Err(error) => error.into_response(),
     }
 }
 
@@ -49,12 +45,15 @@ pub async fn add_handler(
     State(app_data): State<Arc<AppData>>,
     Extract(maybe_bearer_token): Extract,
     extract::Json(category_form): extract::Json<AddCategoryForm>,
-) -> Result<Json<OkResponseData<String>>, ServiceError> {
-    let user_id = app_data.auth.get_user_id_from_bearer_token(&maybe_bearer_token).await?;
+) -> Response {
+    let user_id = match app_data.auth.get_user_id_from_bearer_token(&maybe_bearer_token).await {
+        Ok(user_id) => user_id,
+        Err(error) => return error.into_response(),
+    };
 
     match app_data.category_service.add_category(&category_form.name, &user_id).await {
-        Ok(_) => Ok(added_category(&category_form.name)),
-        Err(error) => Err(error),
+        Ok(_) => added_category(&category_form.name).into_response(),
+        Err(error) => error.into_response(),
     }
 }
 
@@ -71,15 +70,18 @@ pub async fn delete_handler(
     State(app_data): State<Arc<AppData>>,
     Extract(maybe_bearer_token): Extract,
     extract::Json(category_form): extract::Json<DeleteCategoryForm>,
-) -> Result<Json<OkResponseData<String>>, ServiceError> {
+) -> Response {
     // code-review: why do we need to send the whole category object to delete it?
     // And we should use the ID instead of the name, because the name could change
     // or we could add support for multiple languages.
 
-    let user_id = app_data.auth.get_user_id_from_bearer_token(&maybe_bearer_token).await?;
+    let user_id = match app_data.auth.get_user_id_from_bearer_token(&maybe_bearer_token).await {
+        Ok(user_id) => user_id,
+        Err(error) => return error.into_response(),
+    };
 
     match app_data.category_service.delete_category(&category_form.name, &user_id).await {
-        Ok(_) => Ok(deleted_category(&category_form.name)),
-        Err(error) => Err(error),
+        Ok(_) => deleted_category(&category_form.name).into_response(),
+        Err(error) => error.into_response(),
     }
 }
diff --git a/src/web/api/v1/contexts/settings/handlers.rs b/src/web/api/v1/contexts/settings/handlers.rs
@@ -3,13 +3,12 @@
 use std::sync::Arc;
 
 use axum::extract::{self, State};
-use axum::response::Json;
+use axum::response::{IntoResponse, Json, Response};
 
 use crate::common::AppData;
-use crate::config::{ConfigurationPublic, TorrustBackend};
-use crate::errors::ServiceError;
+use crate::config::TorrustBackend;
 use crate::web::api::v1::extractors::bearer_token::Extract;
-use crate::web::api::v1::responses::{self, OkResponseData};
+use crate::web::api::v1::responses::{self};
 
 /// Get all settings.
 ///
@@ -18,31 +17,34 @@ use crate::web::api::v1::responses::{self, OkResponseData};
 /// This function will return an error if the user does not have permission to
 /// view all the settings.
 #[allow(clippy::unused_async)]
-pub async fn get_all_handler(
-    State(app_data): State<Arc<AppData>>,
-    Extract(maybe_bearer_token): Extract,
-) -> Result<Json<OkResponseData<TorrustBackend>>, ServiceError> {
-    let user_id = app_data.auth.get_user_id_from_bearer_token(&maybe_bearer_token).await?;
+pub async fn get_all_handler(State(app_data): State<Arc<AppData>>, Extract(maybe_bearer_token): Extract) -> Response {
+    let user_id = match app_data.auth.get_user_id_from_bearer_token(&maybe_bearer_token).await {
+        Ok(user_id) => user_id,
+        Err(error) => return error.into_response(),
+    };
 
-    let all_settings = app_data.settings_service.get_all(&user_id).await?;
+    let all_settings = match app_data.settings_service.get_all(&user_id).await {
+        Ok(all_settings) => all_settings,
+        Err(error) => return error.into_response(),
+    };
 
-    Ok(Json(responses::OkResponseData { data: all_settings }))
+    Json(responses::OkResponseData { data: all_settings }).into_response()
 }
 
 /// Get public Settings.
 #[allow(clippy::unused_async)]
-pub async fn get_public_handler(State(app_data): State<Arc<AppData>>) -> Json<responses::OkResponseData<ConfigurationPublic>> {
+pub async fn get_public_handler(State(app_data): State<Arc<AppData>>) -> Response {
     let public_settings = app_data.settings_service.get_public().await;
 
-    Json(responses::OkResponseData { data: public_settings })
+    Json(responses::OkResponseData { data: public_settings }).into_response()
 }
 
 /// Get website name.
 #[allow(clippy::unused_async)]
-pub async fn get_site_name_handler(State(app_data): State<Arc<AppData>>) -> Json<responses::OkResponseData<String>> {
+pub async fn get_site_name_handler(State(app_data): State<Arc<AppData>>) -> Response {
     let site_name = app_data.settings_service.get_site_name().await;
 
-    Json(responses::OkResponseData { data: site_name })
+    Json(responses::OkResponseData { data: site_name }).into_response()
 }
 
 /// Update all the settings.
@@ -59,10 +61,16 @@ pub async fn update_handler(
     State(app_data): State<Arc<AppData>>,
     Extract(maybe_bearer_token): Extract,
     extract::Json(torrust_backend): extract::Json<TorrustBackend>,
-) -> Result<Json<OkResponseData<TorrustBackend>>, ServiceError> {
-    let user_id = app_data.auth.get_user_id_from_bearer_token(&maybe_bearer_token).await?;
+) -> Response {
+    let user_id = match app_data.auth.get_user_id_from_bearer_token(&maybe_bearer_token).await {
+        Ok(user_id) => user_id,
+        Err(error) => return error.into_response(),
+    };
 
-    let new_settings = app_data.settings_service.update_all(torrust_backend, &user_id).await?;
+    let new_settings = match app_data.settings_service.update_all(torrust_backend, &user_id).await {
+        Ok(new_settings) => new_settings,
+        Err(error) => return error.into_response(),
+    };
 
-    Ok(Json(responses::OkResponseData { data: new_settings }))
+    Json(responses::OkResponseData { data: new_settings }).into_response()
 }
diff --git a/src/web/api/v1/contexts/tag/handlers.rs b/src/web/api/v1/contexts/tag/handlers.rs
@@ -3,16 +3,13 @@
 use std::sync::Arc;
 
 use axum::extract::{self, State};
-use axum::response::Json;
+use axum::response::{IntoResponse, Json, Response};
 
 use super::forms::{AddTagForm, DeleteTagForm};
 use super::responses::{added_tag, deleted_tag};
 use crate::common::AppData;
-use crate::databases::database;
-use crate::errors::ServiceError;
-use crate::models::torrent_tag::{TagId, TorrentTag};
 use crate::web::api::v1::extractors::bearer_token::Extract;
-use crate::web::api::v1::responses::{self, OkResponseData};
+use crate::web::api::v1::responses::{self};
 
 /// It handles the request to get all the tags.
 ///
@@ -28,12 +25,10 @@ use crate::web::api::v1::responses::{self, OkResponseData};
 ///
 /// It returns an error if there is a database error.
 #[allow(clippy::unused_async)]
-pub async fn get_all_handler(
-    State(app_data): State<Arc<AppData>>,
-) -> Result<Json<responses::OkResponseData<Vec<TorrentTag>>>, database::Error> {
+pub async fn get_all_handler(State(app_data): State<Arc<AppData>>) -> Response {
     match app_data.tag_repository.get_all().await {
-        Ok(tags) => Ok(Json(responses::OkResponseData { data: tags })),
-        Err(error) => Err(error),
+        Ok(tags) => Json(responses::OkResponseData { data: tags }).into_response(),
+        Err(error) => error.into_response(),
     }
 }
 
@@ -50,12 +45,15 @@ pub async fn add_handler(
     State(app_data): State<Arc<AppData>>,
     Extract(maybe_bearer_token): Extract,
     extract::Json(add_tag_form): extract::Json<AddTagForm>,
-) -> Result<Json<OkResponseData<String>>, ServiceError> {
-    let user_id = app_data.auth.get_user_id_from_bearer_token(&maybe_bearer_token).await?;
+) -> Response {
+    let user_id = match app_data.auth.get_user_id_from_bearer_token(&maybe_bearer_token).await {
+        Ok(user_id) => user_id,
+        Err(error) => return error.into_response(),
+    };
 
     match app_data.tag_service.add_tag(&add_tag_form.name, &user_id).await {
-        Ok(_) => Ok(added_tag(&add_tag_form.name)),
-        Err(error) => Err(error),
+        Ok(_) => added_tag(&add_tag_form.name).into_response(),
+        Err(error) => error.into_response(),
     }
 }
 
@@ -72,11 +70,14 @@ pub async fn delete_handler(
     State(app_data): State<Arc<AppData>>,
     Extract(maybe_bearer_token): Extract,
     extract::Json(delete_tag_form): extract::Json<DeleteTagForm>,
-) -> Result<Json<OkResponseData<TagId>>, ServiceError> {
-    let user_id = app_data.auth.get_user_id_from_bearer_token(&maybe_bearer_token).await?;
+) -> Response {
+    let user_id = match app_data.auth.get_user_id_from_bearer_token(&maybe_bearer_token).await {
+        Ok(user_id) => user_id,
+        Err(error) => return error.into_response(),
+    };
 
     match app_data.tag_service.delete_tag(&delete_tag_form.tag_id, &user_id).await {
-        Ok(_) => Ok(deleted_tag(delete_tag_form.tag_id)),
-        Err(error) => Err(error),
+        Ok(_) => deleted_tag(delete_tag_form.tag_id).into_response(),
+        Err(error) => error.into_response(),
     }
 }
diff --git a/src/web/api/v1/contexts/torrent/handlers.rs b/src/web/api/v1/contexts/torrent/handlers.rs
@@ -39,12 +39,12 @@ pub async fn upload_torrent_handler(
 ) -> Response {
     let user_id = match app_data.auth.get_user_id_from_bearer_token(&maybe_bearer_token).await {
         Ok(user_id) => user_id,
-        Err(err) => return err.into_response(),
+        Err(error) => return error.into_response(),
     };
 
     let torrent_request = match get_torrent_request_from_payload(multipart).await {
         Ok(torrent_request) => torrent_request,
-        Err(err) => return err.into_response(),
+        Err(error) => return error.into_response(),
     };
 
     let info_hash = torrent_request.torrent.info_hash().clone();
@@ -73,12 +73,12 @@ pub async fn download_torrent_handler(
 
     let opt_user_id = match get_optional_logged_in_user(maybe_bearer_token, app_data.clone()).await {
         Ok(opt_user_id) => opt_user_id,
-        Err(err) => return err.into_response(),
+        Err(error) => return error.into_response(),
     };
 
     let torrent = match app_data.torrent_service.get_torrent(&info_hash, opt_user_id).await {
         Ok(torrent) => torrent,
-        Err(err) => return err.into_response(),
+        Err(error) => return error.into_response(),
     };
 
     let Ok(bytes) = parse_torrent::encode_torrent(&torrent) else { return ServiceError::InternalServerError.into_response() };
@@ -97,7 +97,7 @@ pub async fn download_torrent_handler(
 pub async fn get_torrents_handler(State(app_data): State<Arc<AppData>>, Query(criteria): Query<ListingRequest>) -> Response {
     match app_data.torrent_service.generate_torrent_info_listing(&criteria).await {
         Ok(torrents_response) => Json(OkResponseData { data: torrents_response }).into_response(),
-        Err(err) => err.into_response(),
+        Err(error) => error.into_response(),
     }
 }
 
@@ -119,12 +119,12 @@ pub async fn get_torrent_info_handler(
 
     let opt_user_id = match get_optional_logged_in_user(maybe_bearer_token, app_data.clone()).await {
         Ok(opt_user_id) => opt_user_id,
-        Err(err) => return err.into_response(),
+        Err(error) => return error.into_response(),
     };
 
     match app_data.torrent_service.get_torrent_info(&info_hash, opt_user_id).await {
         Ok(torrent_response) => Json(OkResponseData { data: torrent_response }).into_response(),
-        Err(err) => err.into_response(),
+        Err(error) => error.into_response(),
     }
 }
 
@@ -148,7 +148,7 @@ pub async fn update_torrent_info_handler(
 
     let user_id = match app_data.auth.get_user_id_from_bearer_token(&maybe_bearer_token).await {
         Ok(user_id) => user_id,
-        Err(err) => return err.into_response(),
+        Err(error) => return error.into_response(),
     };
 
     match app_data
@@ -163,7 +163,7 @@ pub async fn update_torrent_info_handler(
         .await
     {
         Ok(torrent_response) => Json(OkResponseData { data: torrent_response }).into_response(),
-        Err(err) => err.into_response(),
+        Err(error) => error.into_response(),
     }
 }
 
@@ -186,15 +186,15 @@ pub async fn delete_torrent_handler(
 
     let user_id = match app_data.auth.get_user_id_from_bearer_token(&maybe_bearer_token).await {
         Ok(user_id) => user_id,
-        Err(err) => return err.into_response(),
+        Err(error) => return error.into_response(),
     };
 
     match app_data.torrent_service.delete_torrent(&info_hash, &user_id).await {
         Ok(deleted_torrent_response) => Json(OkResponseData {
             data: deleted_torrent_response,
         })
         .into_response(),
-        Err(err) => err.into_response(),
+        Err(error) => error.into_response(),
     }
 }
 
@@ -210,7 +210,7 @@ async fn get_optional_logged_in_user(
     match maybe_bearer_token {
         Some(bearer_token) => match app_data.auth.get_user_id_from_bearer_token(&Some(bearer_token)).await {
             Ok(user_id) => Ok(Some(user_id)),
-            Err(err) => Err(err),
+            Err(error) => Err(error),
         },
         None => Ok(None),
     }