Skip to content

Commit 62b86c2

Browse files
committed
Show errors from cratesio directly to the user
1 parent 84e4ab9 commit 62b86c2

File tree

1 file changed

+63
-16
lines changed

1 file changed

+63
-16
lines changed

src/web/releases.rs

Lines changed: 63 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,12 @@ use super::cache::CachePolicy;
3636
// Introduce SearchError as new error type
3737
#[derive(Debug, thiserror::Error)]
3838
pub enum SearchError {
39-
#[error("crates.io error: {0}")]
39+
#[error("got error from crates.io: {0}")]
4040
CratesIo(String),
41+
#[error("missing releases in crates.io response")]
42+
MissingReleases,
43+
#[error("missing metadata in crates.io response")]
44+
MissingMetadata,
4145
#[error(transparent)]
4246
Other(#[from] anyhow::Error),
4347
}
@@ -169,7 +173,7 @@ async fn get_search_results(
169173
let result = registry.search(query_params).await;
170174
let crate::registry_api::Search { crates, meta } = match result {
171175
Ok(results_from_search_request) => results_from_search_request,
172-
Err(err) => return handle_registry_error(err),
176+
Err(err) => return Err(handle_registry_error(err)),
173177
};
174178

175179
let names = Arc::new(
@@ -254,18 +258,34 @@ async fn get_search_results(
254258
}
255259

256260
// Categorize errors from registry
257-
fn handle_registry_error(err: anyhow::Error) -> Result<SearchResult, SearchError> {
261+
fn handle_registry_error(err: anyhow::Error) -> SearchError {
258262
// Capture crates.io API error
259263
if let Some(registry_request_error) = err.downcast_ref::<reqwest::Error>()
260264
&& let Some(status) = registry_request_error.status()
261265
&& (status.is_client_error() || status.is_server_error())
262266
{
263-
return Err(SearchError::CratesIo(format!(
267+
return SearchError::CratesIo(format!(
264268
"crates.io returned {status}: {registry_request_error}"
265-
)));
269+
));
266270
}
267-
// Move all other error types to this wrapper
268-
Err(SearchError::Other(err))
271+
272+
// Errors from bail!() in RegistryApi::search()
273+
let msg = err.to_string();
274+
if msg.contains("missing releases in crates.io response") {
275+
return SearchError::MissingReleases;
276+
}
277+
if msg.contains("missing metadata in crates.io response") {
278+
return SearchError::MissingMetadata;
279+
}
280+
if msg.contains("got error from crates.io:") {
281+
return SearchError::CratesIo(
282+
msg.trim_start_matches("got error from crates.io:")
283+
.trim()
284+
.to_string(),
285+
);
286+
}
287+
// Move all other error to this fallback wrapper
288+
SearchError::Other(err)
269289
}
270290

271291
//Error message to gracefully display
@@ -654,12 +674,30 @@ pub(crate) async fn search_handler(
654674
let search_result = match search_result {
655675
Ok(result) => result,
656676
Err(SearchError::CratesIo(error_message)) => {
657-
// Return a user-friendly error response
658-
return Ok(create_search_error_response(query, sort_by, error_message).into_response());
677+
return Ok(create_search_error_response(
678+
query,
679+
sort_by,
680+
format!("We're having issues communicating with crates.io: {error_message}"),
681+
)
682+
.into_response());
683+
}
684+
Err(SearchError::MissingReleases) => {
685+
return Ok(create_search_error_response(
686+
query,
687+
sort_by,
688+
"missing releases in crates.io response".to_string(),
689+
)
690+
.into_response());
691+
}
692+
Err(SearchError::MissingMetadata) => {
693+
return Ok(create_search_error_response(
694+
query,
695+
sort_by,
696+
"missing metadata in crates.io response".to_string(),
697+
)
698+
.into_response());
659699
}
660700
Err(SearchError::Other(err)) => {
661-
// For other errors, propagate them normally
662-
// NOTE - Errrors that are not 400x or 500x will be logged to Sentry
663701
return Err(err.into());
664702
}
665703
};
@@ -1275,7 +1313,7 @@ mod tests {
12751313
.await
12761314
.get("/releases/search?query=doesnt_matter_here")
12771315
.await?;
1278-
assert_eq!(response.status(), 500);
1316+
assert_eq!(response.status(), http::StatusCode::SERVICE_UNAVAILABLE);
12791317

12801318
assert!(
12811319
response
@@ -2297,16 +2335,17 @@ mod tests {
22972335
#[test]
22982336
fn test_create_search_error_response() {
22992337
let response = create_search_error_response(
2300-
"test_query".to_string(),
2301-
"relevance".to_string(),
2302-
"Service temporarily unavailable".to_string(),
2338+
"test_query".to_string().clone(),
2339+
"relevance".to_string().clone(),
2340+
"Service temporarily unavailable".to_string().clone(),
23032341
);
23042342
assert_eq!(
23052343
response.title,
23062344
"Search service is not currently available: Service temporarily unavailable"
23072345
);
23082346
assert_eq!(response.status, http::StatusCode::SERVICE_UNAVAILABLE);
23092347
assert_eq!(response.release_type, ReleaseType::Search);
2348+
assert!(response.title.contains("Service temporarily unavailable"));
23102349
}
23112350

23122351
#[test]
@@ -2328,7 +2367,15 @@ mod tests {
23282367
.await
23292368
.get("/releases/search?query=anything_goes_here")
23302369
.await?;
2331-
assert_eq!(response.status(), 503);
2370+
2371+
assert_eq!(response.status(), http::StatusCode::SERVICE_UNAVAILABLE);
2372+
let parse_html = kuchikiki::parse_html().one(response.text().await?);
2373+
2374+
assert!(
2375+
parse_html
2376+
.text_contents()
2377+
.contains("We're having issues communicating with crates.io")
2378+
);
23322379
Ok(())
23332380
})
23342381
}

0 commit comments

Comments
 (0)