diff --git a/apps/labrinth/src/routes/v2/projects.rs b/apps/labrinth/src/routes/v2/projects.rs index 5750cd6fd..1390b4c63 100644 --- a/apps/labrinth/src/routes/v2/projects.rs +++ b/apps/labrinth/src/routes/v2/projects.rs @@ -928,6 +928,7 @@ pub async fn project_delete( session_queue, ) .await + .map(|()| HttpResponse::NoContent().body("")) .or_else(v2_reroute::flatten_404_error) } diff --git a/apps/labrinth/src/routes/v3/projects.rs b/apps/labrinth/src/routes/v3/projects.rs index 60590e8f3..1917793e7 100644 --- a/apps/labrinth/src/routes/v3/projects.rs +++ b/apps/labrinth/src/routes/v3/projects.rs @@ -29,12 +29,14 @@ use crate::queue::session::AuthQueue; use crate::routes::ApiError; use crate::search::indexing::remove_documents; use crate::search::{SearchConfig, SearchError, search_for_project}; +use crate::util::error::Context; use crate::util::img; use crate::util::img::{delete_old_images, upload_image_optimized}; use crate::util::routes::read_limited_from_payload; use crate::util::validate::validation_errors_to_string; use actix_web::{HttpRequest, HttpResponse, web}; use chrono::Utc; +use eyre::eyre; use futures::TryStreamExt; use itertools::Itertools; use serde::{Deserialize, Serialize}; @@ -980,7 +982,8 @@ pub async fn project_edit( .collect::>(), &search_config, ) - .await?; + .await + .wrap_internal_err("failed to remove documents")?; } Ok(HttpResponse::NoContent().body("")) @@ -2157,25 +2160,29 @@ pub async fn project_delete( redis: web::Data, search_config: web::Data, session_queue: web::Data, -) -> Result { - let user = get_user_from_headers( +) -> Result<(), ApiError> { + let (_, user) = get_user_from_headers( &req, &**pool, &redis, &session_queue, Scopes::PROJECT_DELETE, ) - .await? - .1; + .await?; let string = info.into_inner().0; + // In two cases, we return `The specified project does not exist!`: + // - the project really doesn't exist + // - the project is hidden from the user + // + // We use an `ApiError::Auth` for this case instead of a `ApiError::Request`, + // because our permissions tests assert that failing under the 2nd use case + // gives a 401 or 404, but `Request` gives only a 400. + let project = db_models::DBProject::get(&string, &**pool, &redis) - .await? - .ok_or_else(|| { - ApiError::InvalidInput( - "The specified project does not exist!".to_string(), - ) - })?; + .await + .wrap_internal_err("failed to get project")? + .wrap_auth_err("The specified project does not exist!")?; if !user.role.is_admin() { let (team_member, organization_team_member) = @@ -2184,13 +2191,14 @@ pub async fn project_delete( user.id.into(), &**pool, ) - .await?; + .await + .wrap_internal_err("failed to get user team member permissions")?; // Hide the project if team_member.is_none() && organization_team_member.is_none() { - return Err(ApiError::CustomAuthentication( - "The specified project does not exist!".to_string(), - )); + return Err(ApiError::Auth(eyre!( + "The specified project does not exist!" + ))); } let permissions = ProjectPermissions::get_permissions_by_role( @@ -2207,15 +2215,23 @@ pub async fn project_delete( } } - let mut transaction = pool.begin().await?; + let mut transaction = pool + .begin() + .await + .wrap_internal_err("failed to start transaction")?; let context = ImageContext::Project { project_id: Some(project.inner.id.into()), }; let uploaded_images = db_models::DBImage::get_many_contexted(context, &mut transaction) - .await?; + .await + .wrap_internal_err("failed to get project images")?; for image in uploaded_images { - image_item::DBImage::remove(image.id, &mut transaction, &redis).await?; + image_item::DBImage::remove(image.id, &mut transaction, &redis) + .await + .wrap_internal_err_with(|| { + eyre!("failed to remove project image `{:?}`", image.id) + })?; } sqlx::query!( @@ -2226,16 +2242,21 @@ pub async fn project_delete( project.inner.id as db_ids::DBProjectId, ) .execute(&mut transaction) - .await?; + .await + .wrap_internal_err("failed to delete project from collections_mods")?; let result = db_models::DBProject::remove( project.inner.id, &mut transaction, &redis, ) - .await?; + .await + .wrap_internal_err("failed to remove project")?; - transaction.commit().await?; + transaction + .commit() + .await + .wrap_internal_err("failed to commit transaction")?; remove_documents( &project @@ -2245,10 +2266,11 @@ pub async fn project_delete( .collect::>(), &search_config, ) - .await?; + .await + .wrap_internal_err("failed to remove project version documents")?; if result.is_some() { - Ok(HttpResponse::NoContent().body("")) + Ok(()) } else { Err(ApiError::NotFound) } diff --git a/apps/labrinth/src/routes/v3/versions.rs b/apps/labrinth/src/routes/v3/versions.rs index c58cb7eb4..91a5a7809 100644 --- a/apps/labrinth/src/routes/v3/versions.rs +++ b/apps/labrinth/src/routes/v3/versions.rs @@ -27,6 +27,7 @@ use crate::models::teams::ProjectPermissions; use crate::queue::session::AuthQueue; use crate::search::SearchConfig; use crate::search::indexing::remove_documents; +use crate::util::error::Context; use crate::util::img; use crate::util::validate::validation_errors_to_string; use actix_web::{HttpRequest, HttpResponse, web}; @@ -985,7 +986,9 @@ pub async fn version_delete( &redis, ) .await?; - remove_documents(&[version.inner.id.into()], &search_config).await?; + remove_documents(&[version.inner.id.into()], &search_config) + .await + .wrap_internal_err("failed to remove documents")?; if result.is_some() { Ok(HttpResponse::NoContent().body("")) diff --git a/apps/labrinth/src/search/indexing/mod.rs b/apps/labrinth/src/search/indexing/mod.rs index 2da327ce1..909b2141d 100644 --- a/apps/labrinth/src/search/indexing/mod.rs +++ b/apps/labrinth/src/search/indexing/mod.rs @@ -6,7 +6,9 @@ use std::time::Duration; use crate::database::PgPool; use crate::database::redis::RedisPool; use crate::search::{SearchConfig, UploadSearchProject}; +use crate::util::error::Context; use ariadne::ids::base62_impl::to_base62; +use eyre::eyre; use futures::StreamExt; use futures::stream::FuturesOrdered; use local_import::index_local; @@ -52,9 +54,13 @@ fn search_operation_timeout() -> std::time::Duration { pub async fn remove_documents( ids: &[crate::models::ids::VersionId], config: &SearchConfig, -) -> Result<(), IndexingError> { - let mut indexes = get_indexes_for_indexing(config, false, false).await?; - let indexes_next = get_indexes_for_indexing(config, true, false).await?; +) -> eyre::Result<()> { + let mut indexes = get_indexes_for_indexing(config, false, false) + .await + .wrap_err("failed to get current indexes")?; + let indexes_next = get_indexes_for_indexing(config, true, false) + .await + .wrap_err("failed to get next indexes")?; for list in &mut indexes { for alt_list in &indexes_next { @@ -62,7 +68,9 @@ pub async fn remove_documents( } } - let client = config.make_batch_client()?; + let client = config + .make_batch_client() + .wrap_err("failed to create batch client")?; let client = &client; let ids_base62 = ids.iter().map(|x| to_base62(x.0)).collect::>(); @@ -75,13 +83,19 @@ pub async fn remove_documents( deletion_tasks.push_back(async move { index .delete_documents(ids_base62_ref) - .await? + .await + .wrap_err_with(|| { + eyre!("failed to request to delete documents {ids_base62_ref:?}") + })? .wait_for_completion( &owned_client, None, Some(Duration::from_secs(15)), ) .await + .wrap_err_with(|| { + eyre!("failed to delete documents {ids_base62_ref:?}") + }) }); } });