MrSphay/Modrinth-plus
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Improve error logging in project delete route (#5388)

Browse Source 
* Improve error logging in project delete route

* remove_documents more error logging

* fix ci

* try fix ci? idk man
This commit is contained in:
aecsocket
2026-02-18 05:06:50 +00:00
committed by GitHub
parent 4be2f77bb0
commit 9f558404bd
4 changed files with 69 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
apps/labrinth/src/routes/v2/projects.rs
View File

@@ -928,6 +928,7 @@ pub async fn project_delete(
session_queue,
)
.await
.map(|()| HttpResponse::NoContent().body(""))
.or_else(v2_reroute::flatten_404_error)
}

68
apps/labrinth/src/routes/v3/projects.rs
View File

@@ -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::<Vec<_>>(),
&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<RedisPool>,
search_config: web::Data<SearchConfig>,
session_queue: web::Data<AuthQueue>,
) -> Result<HttpResponse, ApiError> {
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::<Vec<_>>(),
&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)
}

5
apps/labrinth/src/routes/v3/versions.rs
View File

@@ -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(""))

24
apps/labrinth/src/search/indexing/mod.rs
View File

@@ -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::<Vec<_>>();
@@ -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:?}")
})
});
}
});