Skip to content

Commit bfcf6d5

Browse files
authored
🐛 fix(web): expose loader failures (#1860)
* 🐛 fix(web): expose loader failures Deserialize browser responses into strict endpoint wire types so malformed or failed requests cannot become valid empty UI models. Keep the last loaded data visible while displaying the bounded loader error. Refs #1358 * 🧪 test(web): cover loader failure state CI caught six untested error-formatting lines and one retained-state branch. The missing cases left the refresh failure contract unverified in native builds. Table-driven message cases cover each error kind. A state transition test verifies that a failed refresh reports the error while preserving the prior value.
1 parent 70abe41 commit bfcf6d5

22 files changed

Lines changed: 1101 additions & 293 deletions

File tree

crates/peryx-ecosystem-pypi/tests/frontend/tests/ui.spec.mjs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ test("admin table shows upstream and upload state per index", async ({
387387
await expect(table.locator("[class*='badge upload-']").first()).toBeVisible();
388388
});
389389

390-
test("admin status is read-only and tolerates failed stats fetches", async ({
390+
test("admin status is read-only and reports failed stats fetches", async ({
391391
page,
392392
}) => {
393393
await operatorPage(page);
@@ -430,9 +430,10 @@ test("admin status is read-only and tolerates failed stats fetches", async ({
430430
page.locator(".ops-table", { hasText: "veloxdemo-1.0.0" }),
431431
).toBeVisible();
432432
await expect(page.locator(".ops-table").first()).not.toContainText(TOKEN);
433-
await expect(
434-
page.locator(".dim", { hasText: "No usage recorded yet." }),
435-
).toBeVisible();
433+
await expect(page.getByRole("alert")).toHaveText(
434+
"/+stats returned HTTP 503.",
435+
);
436+
await expect(page.getByText("No usage recorded yet.")).toHaveCount(0);
436437
await expect(page.locator(".token")).toHaveCount(0);
437438
await expect(page.locator(".admin-table")).toHaveCount(0);
438439
});

crates/peryx-web/src/data/browse.rs

Lines changed: 60 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,52 +2,82 @@ use peryx_core::BrowsePage;
22
#[cfg(all(not(feature = "ssr"), feature = "hydrate"))]
33
use peryx_core::UiActionMethod;
44

5-
#[cfg(all(not(feature = "ssr"), feature = "hydrate"))]
6-
pub(super) async fn fetch_json(url: &str) -> Option<serde_json::Value> {
7-
fetch_json_required(url).await.ok()
5+
#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Deserialize, serde::Serialize)]
6+
pub enum LoaderEndpoint {
7+
Browse,
8+
Session,
9+
Stats,
10+
Status,
11+
Topology,
12+
}
13+
14+
impl std::fmt::Display for LoaderEndpoint {
15+
fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
16+
formatter.write_str(match self {
17+
Self::Browse => "/+ui/browse",
18+
Self::Session => "/_/session",
19+
Self::Stats => "/+stats",
20+
Self::Status => "/+status",
21+
Self::Topology => "/+availability/topology",
22+
})
23+
}
824
}
925

26+
#[derive(Debug, Clone, PartialEq, Eq, serde::Deserialize, serde::Serialize)]
27+
pub enum LoaderError {
28+
Request(LoaderEndpoint),
29+
Status { endpoint: LoaderEndpoint, status: u16 },
30+
Invalid(LoaderEndpoint),
31+
}
32+
33+
impl std::fmt::Display for LoaderError {
34+
fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
35+
match self {
36+
Self::Request(endpoint) => write!(formatter, "Request to {endpoint} failed."),
37+
Self::Status { endpoint, status } => write!(formatter, "{endpoint} returned HTTP {status}."),
38+
Self::Invalid(endpoint) => write!(formatter, "{endpoint} returned invalid data."),
39+
}
40+
}
41+
}
42+
43+
impl std::error::Error for LoaderError {}
44+
1045
#[cfg(all(not(feature = "ssr"), feature = "hydrate"))]
11-
pub(super) async fn fetch_json_required(url: &str) -> Result<serde_json::Value, String> {
12-
let Some(value) = fetch_json_optional(url).await? else {
13-
return Err(format!("404 from {url}: not found"));
46+
pub(super) async fn fetch_json_required<T>(url: &str, endpoint: LoaderEndpoint) -> Result<T, LoaderError>
47+
where
48+
T: serde::de::DeserializeOwned,
49+
{
50+
let Some(value) = fetch_json_optional(url, endpoint).await? else {
51+
return Err(LoaderError::Status { endpoint, status: 404 });
1452
};
1553
Ok(value)
1654
}
1755

1856
#[cfg(all(not(feature = "ssr"), feature = "hydrate"))]
19-
pub(super) async fn fetch_json_optional(url: &str) -> Result<Option<serde_json::Value>, String> {
57+
pub(super) async fn fetch_json_optional<T>(url: &str, endpoint: LoaderEndpoint) -> Result<Option<T>, LoaderError>
58+
where
59+
T: serde::de::DeserializeOwned,
60+
{
2061
send_wrapper::SendWrapper::new(async {
2162
let response = gloo_net::http::Request::get(url)
2263
.header("accept", "application/json")
2364
.send()
2465
.await
25-
.map_err(|error| format!("request failed for {url}: {error}"))?;
66+
.map_err(|_| LoaderError::Request(endpoint))?;
2667
if response.status() == 404 {
2768
return Ok(None);
2869
}
2970
if !response.ok() {
30-
return Err(response_error(response, url).await);
71+
return Err(LoaderError::Status {
72+
endpoint,
73+
status: response.status(),
74+
});
3175
}
3276
response
3377
.json()
3478
.await
3579
.map(Some)
36-
.map_err(|error| format!("invalid JSON from {url}: {error}"))
37-
})
38-
.await
39-
}
40-
41-
#[cfg(all(not(feature = "ssr"), feature = "hydrate"))]
42-
async fn response_error(response: gloo_net::http::Response, url: &str) -> String {
43-
send_wrapper::SendWrapper::new(async move {
44-
let status = response.status();
45-
let text = response.text().await.unwrap_or_default();
46-
if text.is_empty() {
47-
format!("{status} from {url}")
48-
} else {
49-
format!("{status} from {url}: {text}")
50-
}
80+
.map_err(|_| LoaderError::Invalid(endpoint))
5181
})
5282
.await
5383
}
@@ -63,12 +93,14 @@ pub async fn load_browse(raw_query: String) -> Result<Option<BrowsePage>, String
6393
#[cfg(all(not(feature = "ssr"), feature = "hydrate"))]
6494
{
6595
send_wrapper::SendWrapper::new(async move {
66-
let Some(value) = super::fetch_json_optional(&crate::url::ui_browse_url(&raw_query)).await? else {
96+
let Some(page) =
97+
super::fetch_json_optional(&crate::url::ui_browse_url(&raw_query), super::LoaderEndpoint::Browse)
98+
.await
99+
.map_err(|error| error.to_string())?
100+
else {
67101
return Ok(None);
68102
};
69-
serde_json::from_value(value)
70-
.map(Some)
71-
.map_err(|err| format!("invalid browse response: {err}"))
103+
Ok(Some(page))
72104
})
73105
.await
74106
}

crates/peryx-web/src/data/login.rs

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,45 @@
11
use crate::model::UiLoginState;
22

3+
#[cfg(all(not(feature = "ssr"), feature = "hydrate"))]
4+
use super::RequiredOption;
5+
6+
#[cfg(all(not(feature = "ssr"), feature = "hydrate"))]
7+
#[derive(serde::Deserialize)]
8+
struct SessionDocument {
9+
user: RequiredOption<SessionUser>,
10+
providers: Vec<String>,
11+
}
12+
13+
#[cfg(all(not(feature = "ssr"), feature = "hydrate"))]
14+
#[derive(serde::Deserialize)]
15+
struct SessionUser {
16+
name: String,
17+
}
18+
319
/// The browser login state: who is signed in and which providers to offer.
4-
pub async fn load_login() -> UiLoginState {
20+
///
21+
/// # Errors
22+
///
23+
/// Returns a typed error when the session endpoint cannot provide a valid document.
24+
pub async fn load_login() -> Result<UiLoginState, super::LoaderError> {
525
#[cfg(feature = "ssr")]
626
{
7-
crate::ssr::login_state().await
27+
Ok(crate::ssr::login_state().await)
828
}
929
#[cfg(all(not(feature = "ssr"), feature = "hydrate"))]
1030
{
1131
send_wrapper::SendWrapper::new(async {
12-
super::fetch_json("/_/session")
13-
.await
14-
.map_or_else(UiLoginState::default, |value| UiLoginState::from_session(&value))
32+
let document: SessionDocument =
33+
super::fetch_json_required("/_/session", super::LoaderEndpoint::Session).await?;
34+
Ok(UiLoginState {
35+
user: document.user.0.map(|user| user.name),
36+
providers: document.providers,
37+
})
1538
})
1639
.await
1740
}
1841
#[cfg(all(not(feature = "ssr"), not(feature = "hydrate")))]
1942
{
20-
UiLoginState::default()
43+
Ok(UiLoginState::default())
2144
}
2245
}

crates/peryx-web/src/data/mod.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,9 @@ mod topology;
1313
#[cfg(all(not(feature = "ssr"), feature = "hydrate"))]
1414
pub use browse::admin_request;
1515
pub use browse::load_browse;
16+
pub use browse::{LoaderEndpoint, LoaderError};
1617
#[cfg(all(not(feature = "ssr"), feature = "hydrate"))]
17-
use browse::{fetch_json, fetch_json_optional, fetch_json_required};
18+
use browse::{fetch_json_optional, fetch_json_required};
1819
pub use login::load_login;
1920
pub use operations::load_operations;
2021
#[cfg(all(not(feature = "ssr"), feature = "hydrate"))]
@@ -30,3 +31,8 @@ pub use status::{load_admin_snapshot, load_snapshot};
3031
pub use topology::load_topology;
3132
#[cfg(all(not(feature = "ssr"), feature = "hydrate"))]
3233
pub use topology::{TopologyStream, subscribe_topology};
34+
35+
#[cfg(all(not(feature = "ssr"), feature = "hydrate"))]
36+
#[derive(serde::Deserialize)]
37+
#[serde(transparent)]
38+
struct RequiredOption<T>(Option<T>);

0 commit comments

Comments
 (0)