Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

handle changed error responses from crates.io API #2370

Merged
merged 1 commit into from
Dec 29, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 64 additions & 8 deletions src/web/releases.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::{
},
BuildQueue, Config, InstanceMetrics,
};
use anyhow::{anyhow, Context as _, Result};
use anyhow::{anyhow, bail, Context as _, Result};
use axum::{
extract::{Extension, Path, Query},
response::{IntoResponse, Response as AxumResponse},
Expand Down Expand Up @@ -141,10 +141,15 @@ async fn get_search_results(
config: &Config,
query_params: &str,
) -> Result<SearchResult, anyhow::Error> {
#[derive(Deserialize)]
struct CratesIoError {
detail: String,
}
#[derive(Deserialize)]
struct CratesIoSearchResult {
crates: Vec<CratesIoCrate>,
meta: CratesIoMeta,
crates: Option<Vec<CratesIoCrate>>,
meta: Option<CratesIoMeta>,
errors: Option<Vec<CratesIoError>>,
}
#[derive(Deserialize, Debug)]
struct CratesIoCrate {
Expand Down Expand Up @@ -186,7 +191,7 @@ async fn get_search_results(
}
});

let releases: CratesIoSearchResult = retry_async(
let response: CratesIoSearchResult = retry_async(
|| async {
Ok(HTTP_CLIENT
.get(url.clone())
Expand All @@ -200,9 +205,21 @@ async fn get_search_results(
.json()
.await?;

if let Some(errors) = response.errors {
let messages: Vec<_> = errors.into_iter().map(|e| e.detail).collect();
bail!("got error from crates.io: {}", messages.join("\n"));
}

let Some(crates) = response.crates else {
bail!("missing releases in crates.io response");
};

let Some(meta) = response.meta else {
bail!("missing metadata in crates.io response");
};

let names = Arc::new(
releases
.crates
crates
.into_iter()
.map(|krate| krate.name)
.collect::<Vec<_>>(),
Expand Down Expand Up @@ -261,8 +278,8 @@ async fn get_search_results(
.cloned()
.collect(),
executed_query,
prev_page: releases.meta.prev_page,
next_page: releases.meta.next_page,
prev_page: meta.prev_page,
next_page: meta.next_page,
})
}

Expand Down Expand Up @@ -1034,6 +1051,45 @@ mod tests {
})
}

#[test]
fn crates_io_errors_as_status_code_200() {
wrapper(|env| {
let mut crates_io = mockito::Server::new();
env.override_config(|config| {
config.crates_io_api_call_retries = 0;
config.registry_api_host = crates_io.url().parse().unwrap();
});

let _m = crates_io
.mock("GET", "/api/v1/crates")
.match_query(Matcher::AllOf(vec![
Matcher::UrlEncoded("q".into(), "doesnt_matter_here".into()),
Matcher::UrlEncoded("per_page".into(), "30".into()),
]))
.with_status(200)
.with_header("content-type", "application/json")
.with_body(
json!({
"errors": [
{ "detail": "error name 1" },
{ "detail": "error name 2" },
]
})
.to_string(),
)
.create();

let response = env
.frontend()
.get("/releases/search?query=doesnt_matter_here")
.send()?;
assert_eq!(response.status(), 500);

assert!(response.text()?.contains("error name 1\nerror name 2"));
Ok(())
})
}

#[test_case(StatusCode::NOT_FOUND)]
#[test_case(StatusCode::INTERNAL_SERVER_ERROR)]
#[test_case(StatusCode::BAD_GATEWAY)]
Expand Down
Loading