Skip to content

Commit

Permalink
Merge pull request #7851 from Turbo87/version-not-found
Browse files Browse the repository at this point in the history
Adjust "version invalid / not found" errors to reply with "404 Not Found"
  • Loading branch information
Turbo87 authored Jan 3, 2024
2 parents e28d7b0 + 29f0e38 commit 341c90a
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 12 deletions.
3 changes: 2 additions & 1 deletion src/controllers/version/downloads.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use crate::db::PoolError;
use crate::middleware::log_request::RequestLogExt;
use crate::models::VersionDownload;
use crate::schema::*;
use crate::util::errors::version_not_found;
use crate::views::EncodableVersionDownload;
use chrono::{Duration, NaiveDate, Utc};
use tracing::Instrument;
Expand Down Expand Up @@ -135,7 +136,7 @@ pub async fn downloads(
) -> AppResult<Json<Value>> {
spawn_blocking(move || {
if semver::Version::parse(&version).is_err() {
return Err(cargo_err(format_args!("invalid semver: {version}")));
return Err(version_not_found(&crate_name, &version));
}

let conn = &mut *app.db_read()?;
Expand Down
5 changes: 3 additions & 2 deletions src/controllers/version/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use crate::controllers::frontend_prelude::*;

use crate::models::VersionOwnerAction;
use crate::util::errors::version_not_found;
use crate::views::{EncodableDependency, EncodableVersion};

use super::version_and_crate;
Expand All @@ -24,7 +25,7 @@ pub async fn dependencies(
) -> AppResult<Json<Value>> {
spawn_blocking(move || {
if semver::Version::parse(&version).is_err() {
return Err(cargo_err(format_args!("invalid semver: {version}")));
return Err(version_not_found(&crate_name, &version));
}

let conn = &mut state.db_read()?;
Expand Down Expand Up @@ -61,7 +62,7 @@ pub async fn show(
) -> AppResult<Json<Value>> {
spawn_blocking(move || {
if semver::Version::parse(&version).is_err() {
return Err(cargo_err(format_args!("invalid semver: {version}")));
return Err(version_not_found(&crate_name, &version));
}

let conn = &mut state.db_read()?;
Expand Down
3 changes: 2 additions & 1 deletion src/controllers/version/yank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use crate::models::Rights;
use crate::models::{insert_version_owner_action, VersionAction};
use crate::rate_limiter::LimitedAction;
use crate::schema::versions;
use crate::util::errors::version_not_found;
use crate::worker::jobs;
use tokio::runtime::Handle;

Expand Down Expand Up @@ -49,7 +50,7 @@ fn modify_yank(
// lifetime issues with `req`.

if semver::Version::parse(version).is_err() {
return Err(cargo_err(format_args!("invalid semver: {version}")));
return Err(version_not_found(crate_name, version));
}

let conn = &mut *state.db_write()?;
Expand Down
9 changes: 2 additions & 7 deletions src/models/krate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::models::{
CrateOwner, CrateOwnerInvitation, Dependency, NewCrateOwnerInvitationOutcome, Owner, OwnerKind,
ReverseDependency, User, Version,
};
use crate::util::errors::{cargo_err, AppResult};
use crate::util::errors::{version_not_found, AppResult};

use crate::models::helpers::with_count::*;
use crate::schema::*;
Expand Down Expand Up @@ -191,12 +191,7 @@ impl Crate {
.filter(versions::num.eq(version))
.first(conn)
.optional()?
.ok_or_else(|| {
cargo_err(format_args!(
"crate `{}` does not have a version `{}`",
self.name, version
))
})
.ok_or_else(|| version_not_found(&self.name, version))
}

// Validates the name is a valid crate name.
Expand Down
2 changes: 1 addition & 1 deletion src/tests/routes/crates/versions/dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ fn dependencies() {
assert_eq!(deps.dependencies[0].crate_id, "bar_deps");

let response = anon.get::<()>("/api/v1/crates/foo_deps/1.0.2/dependencies");
assert_eq!(response.status(), StatusCode::OK);
assert_eq!(response.status(), StatusCode::NOT_FOUND);
assert_eq!(
response.into_json(),
json!({ "errors": [{ "detail": "crate `foo_deps` does not have a version `1.0.2`" }] })
Expand Down
5 changes: 5 additions & 0 deletions src/util/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ pub fn service_unavailable() -> BoxedAppError {
custom(StatusCode::SERVICE_UNAVAILABLE, "Service unavailable")
}

pub fn version_not_found(krate: &str, version: &str) -> BoxedAppError {
let detail = format!("crate `{krate}` does not have a version `{version}`");
custom(StatusCode::NOT_FOUND, detail)
}

// =============================================================================
// AppError trait

Expand Down

0 comments on commit 341c90a

Please sign in to comment.