Skip to content

Commit 6c42373

Browse files
Nemo157syphar
authored andcommitted
Improve handling errors during readme fetch
A missing readme is ignored, other errors are reported to sentry
1 parent 8f0be88 commit 6c42373

File tree

1 file changed

+44
-41
lines changed

1 file changed

+44
-41
lines changed

Diff for: src/web/crate_details.rs

+44-41
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,15 @@ use crate::{
44
db::Pool,
55
impl_axum_webpage,
66
repositories::RepositoryStatsUpdater,
7+
storage::PathNotFoundError,
78
web::{
89
cache::CachePolicy,
910
encode_url_path,
1011
error::{AxumNope, AxumResult},
1112
},
1213
Storage,
1314
};
14-
use anyhow::{bail, Context, Result};
15+
use anyhow::{Context, Result};
1516
use axum::{
1617
extract::{Extension, Path},
1718
response::{IntoResponse, Response as AxumResponse},
@@ -92,34 +93,6 @@ pub struct Release {
9293
pub target_name: String,
9394
}
9495

95-
#[fn_error_context::context("fetching readme for {name} {version}")]
96-
fn fetch_readme_from_source(
97-
storage: &Storage,
98-
name: &str,
99-
version: &str,
100-
archive_storage: bool,
101-
) -> anyhow::Result<String> {
102-
let manifest = storage.fetch_source_file(name, version, "Cargo.toml", archive_storage)?;
103-
let manifest = String::from_utf8(manifest.content)
104-
.context("parsing Cargo.toml")?
105-
.parse::<toml::Value>()
106-
.context("parsing Cargo.toml")?;
107-
let paths = match manifest.get("package").and_then(|p| p.get("readme")) {
108-
Some(toml::Value::Boolean(true)) => vec!["README.md"],
109-
Some(toml::Value::Boolean(false)) => vec![],
110-
Some(toml::Value::String(path)) => vec![path.as_ref()],
111-
_ => vec!["README.md", "README.txt", "README"],
112-
};
113-
for path in &paths {
114-
if let Ok(readme) = storage.fetch_source_file(name, version, path, archive_storage) {
115-
let readme = String::from_utf8(readme.content)
116-
.with_context(|| format!("parsing {path} content"))?;
117-
return Ok(readme);
118-
}
119-
}
120-
bail!("couldn't find readme in stored source, checked {paths:?}")
121-
}
122-
12396
impl CrateDetails {
12497
pub fn new(
12598
conn: &mut impl GenericClient,
@@ -266,11 +239,40 @@ impl CrateDetails {
266239
Ok(Some(crate_details))
267240
}
268241

269-
fn enrich_readme(&mut self, storage: &Storage) -> Result<()> {
270-
let readme =
271-
fetch_readme_from_source(storage, &self.name, &self.version, self.archive_storage)?;
272-
self.readme = Some(readme);
273-
Ok(())
242+
#[fn_error_context::context("fetching readme for {} {}", self.name, self.version)]
243+
fn fetch_readme(&self, storage: &Storage) -> anyhow::Result<Option<String>> {
244+
let manifest = storage.fetch_source_file(
245+
&self.name,
246+
&self.version,
247+
"Cargo.toml",
248+
self.archive_storage,
249+
)?;
250+
let manifest = String::from_utf8(manifest.content)
251+
.context("parsing Cargo.toml")?
252+
.parse::<toml::Value>()
253+
.context("parsing Cargo.toml")?;
254+
let paths = match manifest.get("package").and_then(|p| p.get("readme")) {
255+
Some(toml::Value::Boolean(true)) => vec!["README.md"],
256+
Some(toml::Value::Boolean(false)) => vec![],
257+
Some(toml::Value::String(path)) => vec![path.as_ref()],
258+
_ => vec!["README.md", "README.txt", "README"],
259+
};
260+
for path in &paths {
261+
match storage.fetch_source_file(&self.name, &self.version, path, self.archive_storage) {
262+
Ok(readme) => {
263+
let readme = String::from_utf8(readme.content)
264+
.with_context(|| format!("parsing {path} content"))?;
265+
return Ok(Some(readme));
266+
}
267+
Err(err) if err.is::<PathNotFoundError>() => {
268+
continue;
269+
}
270+
Err(err) => {
271+
return Err(err);
272+
}
273+
}
274+
}
275+
Ok(None)
274276
}
275277

276278
/// Returns the latest non-yanked, non-prerelease release of this crate (or latest
@@ -395,16 +397,17 @@ pub(crate) async fn crate_details_handler(
395397
&version,
396398
&version_or_latest,
397399
Some(&repository_stats_updater),
398-
)?;
399-
if let Some(ref mut details) = details {
400-
if let Err(e) = details.enrich_readme(&storage) {
401-
tracing::debug!("{e:?}")
402-
}
400+
)?
401+
.ok_or(AxumNope::VersionNotFound)?;
402+
403+
match details.fetch_readme(&storage) {
404+
Ok(readme) => details.readme = readme.or(details.readme),
405+
Err(e) => report_error(&e),
403406
}
407+
404408
Ok(details)
405409
})
406-
.await?
407-
.ok_or(AxumNope::VersionNotFound)?;
410+
.await?;
408411

409412
let mut res = CrateDetailsPage { details }.into_response();
410413
res.extensions_mut()

0 commit comments

Comments
 (0)