Skip to content

Commit 34c4409

Browse files
committed
refs #1979: add workaround for toolchain JS files that were referenced wrong
1 parent 6c24e02 commit 34c4409

File tree

2 files changed

+83
-43
lines changed

2 files changed

+83
-43
lines changed

src/web/error.rs

+8-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
use std::borrow::Cow;
22

3-
use crate::web::{releases::Search, AxumErrorPage};
3+
use crate::{
4+
storage::PathNotFoundError,
5+
web::{releases::Search, AxumErrorPage},
6+
};
47
use axum::{
58
http::StatusCode,
69
response::{IntoResponse, Response as AxumResponse},
@@ -120,7 +123,10 @@ impl From<anyhow::Error> for AxumNope {
120123
fn from(err: anyhow::Error) -> Self {
121124
match err.downcast::<AxumNope>() {
122125
Ok(axum_nope) => axum_nope,
123-
Err(err) => AxumNope::InternalError(err),
126+
Err(err) => match err.downcast::<PathNotFoundError>() {
127+
Ok(_) => AxumNope::ResourceNotFound,
128+
Err(err) => AxumNope::InternalError(err),
129+
},
124130
}
125131
}
126132
}

src/web/rustdoc.rs

+75-41
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
use crate::{
44
db::Pool,
55
repositories::RepositoryStatsUpdater,
6-
storage::{rustdoc_archive_path, PathNotFoundError},
6+
storage::rustdoc_archive_path,
77
utils::{self, spawn_blocking},
88
web::{
99
axum_cached_redirect, axum_parse_uri_with_params,
@@ -58,6 +58,37 @@ pub(crate) struct RustdocRedirectorParams {
5858
target: Option<String>,
5959
}
6060

61+
/// try to serve a toolchain specific asset from the legacy location.
62+
///
63+
/// Newer rustdoc builds use a specific subfolder on the bucket,
64+
/// a new `static-root-path` prefix (`/-/rustdoc.static/...`), which
65+
/// is served via our `static_asset_handler`.
66+
///
67+
/// The legacy location is the root, both on the bucket & the URL
68+
/// path, which is suboptimal since the route overlaps with other routes.
69+
///
70+
/// See also https://github.com/rust-lang/docs.rs/pull/1889
71+
async fn try_serve_legacy_toolchain_asset(
72+
storage: Arc<Storage>,
73+
config: Arc<Config>,
74+
path: impl AsRef<str>,
75+
) -> AxumResult<AxumResponse> {
76+
let path = path.as_ref().to_owned();
77+
// FIXME: this could be optimized: when a path doesn't exist
78+
// in storage, we don't need to recheck on every request.
79+
// Existing files are returned with caching headers, so
80+
// are cached by the CDN.
81+
// If cached, it doesn't need to be invalidated,
82+
// since new nightly versions will always put their
83+
// toolchain specific resources into the new folder,
84+
// which is reached via the new handler.
85+
Ok(
86+
spawn_blocking(move || File::from_path(&storage, &path, &config))
87+
.await
88+
.map(IntoResponse::into_response)?,
89+
)
90+
}
91+
6192
/// Handler called for `/:crate` and `/:crate/:version` URLs. Automatically redirects to the docs
6293
/// or crate details page based on whether the given crate version was successfully built.
6394
#[instrument(skip_all)]
@@ -93,41 +124,13 @@ pub(crate) async fn rustdoc_redirector_handler(
93124

94125
// global static assets for older builds are served from the root, which ends up
95126
// in this handler as `params.name`.
96-
// Newer builds use a different static-root, and the static assets are served via
97-
// `static_asset_handler`
98127
if let Some((_, extension)) = params.name.rsplit_once('.') {
99128
if ["css", "js", "png", "svg", "woff", "woff2"]
100129
.binary_search(&extension)
101130
.is_ok()
102131
{
103132
rendering_time.step("serve static asset");
104-
// FIXME: this could be optimized: when a path doesn't exist
105-
// in storage, we don't need to recheck on every request.
106-
// Existing files are returned with caching headers, so
107-
// are cached by the CDN.
108-
// If cached, it doesn't need to be invalidated,
109-
// since new nightly versions will always put their
110-
// toolchain specific resources into the new folder,
111-
// which is reached via the new handler.
112-
return match spawn_blocking({
113-
let storage = storage.clone();
114-
let name = params.name.clone();
115-
let config = config.clone();
116-
move || File::from_path(&storage, &name, &config)
117-
})
118-
.await
119-
{
120-
Ok(file) => Ok(file.into_response()),
121-
Err(err) => {
122-
if matches!(err.downcast_ref(), Some(AxumNope::ResourceNotFound))
123-
|| matches!(err.downcast_ref(), Some(crate::storage::PathNotFoundError))
124-
{
125-
Err(AxumNope::ResourceNotFound)
126-
} else {
127-
Err(AxumNope::InternalError(err))
128-
}
129-
}
130-
};
133+
return try_serve_legacy_toolchain_asset(storage, config, params.name).await;
131134
}
132135
}
133136

@@ -210,7 +213,16 @@ pub(crate) async fn rustdoc_redirector_handler(
210213
{
211214
debug!(?target, ?err, "got error serving file");
212215
}
213-
return Err(AxumNope::ResourceNotFound);
216+
// FIXME: we sometimes still get requests for toolchain
217+
// specific static assets under the crate/version/ path.
218+
// This is fixed in rustdoc, but pending a rebuild for
219+
// docs that were affected by this bug.
220+
// https://github.com/rust-lang/docs.rs/issues/1979
221+
if target.starts_with("search-") {
222+
return try_serve_legacy_toolchain_asset(storage, config, target).await;
223+
} else {
224+
return Err(err.into());
225+
}
214226
}
215227
}
216228
}
@@ -912,16 +924,7 @@ pub(crate) async fn static_asset_handler(
912924
) -> AxumResult<impl IntoResponse> {
913925
let storage_path = format!("{}{}", RUSTDOC_STATIC_STORAGE_PREFIX, path);
914926

915-
Ok(spawn_blocking(
916-
move || match File::from_path(&storage, &storage_path, &config) {
917-
Ok(file) => Ok(file),
918-
Err(err) if err.downcast_ref::<PathNotFoundError>().is_some() => {
919-
Err(AxumNope::ResourceNotFound.into())
920-
}
921-
Err(err) => Err(AxumNope::InternalError(err).into()),
922-
},
923-
)
924-
.await?)
927+
Ok(spawn_blocking(move || File::from_path(&storage, &storage_path, &config)).await?)
925928
}
926929

927930
#[cfg(test)]
@@ -2591,4 +2594,35 @@ mod test {
25912594
Ok(())
25922595
});
25932596
}
2597+
2598+
#[test]
2599+
fn fallback_to_root_storage_for_search_js_assets() {
2600+
// test workaround for https://github.com/rust-lang/docs.rs/issues/1979
2601+
wrapper(|env| {
2602+
env.fake_release()
2603+
.name("dummy")
2604+
.version("0.1.0")
2605+
.archive_storage(true)
2606+
.create()?;
2607+
2608+
env.storage().store_one("asset.js", *b"content")?;
2609+
env.storage()
2610+
.store_one("search-1234.js", *b"more_content")?;
2611+
2612+
let web = env.frontend();
2613+
2614+
assert_eq!(
2615+
web.get("/dummy/0.1.0/asset.js").send()?.status(),
2616+
StatusCode::NOT_FOUND
2617+
);
2618+
assert!(web.get("/asset.js").send()?.status().is_success());
2619+
2620+
assert!(web.get("/search-1234.js").send()?.status().is_success());
2621+
let response = web.get("/dummy/0.1.0/search-1234.js").send()?;
2622+
assert!(response.status().is_success());
2623+
assert_eq!(response.text()?, "more_content");
2624+
2625+
Ok(())
2626+
})
2627+
}
25942628
}

0 commit comments

Comments
 (0)