From fe3848c453d1598112aaa529bacc0e08f2637ad3 Mon Sep 17 00:00:00 2001 From: Priec Date: Sat, 15 Aug 2026 15:08:35 +0200 Subject: [PATCH] error messages --- web/src/pages/add_logic/loader.rs | 16 ++ web/src/pages/add_logic/logic.rs | 47 ++-- web/src/pages/add_validation/loader.rs | 16 ++ web/src/pages/add_validation/logic.rs | 245 +++++++------------ web/src/pages/import_export/common/loader.rs | 16 ++ web/src/pages/import_export/export/logic.rs | 91 +++---- web/src/pages/import_export/import/logic.rs | 155 +++++------- web/src/pages/login/logic.rs | 4 +- web/src/pages/register/logic.rs | 19 +- web/src/ui/mod.rs | 134 +++++++++- 10 files changed, 395 insertions(+), 348 deletions(-) diff --git a/web/src/pages/add_logic/loader.rs b/web/src/pages/add_logic/loader.rs index 91e00f86..2d7d14fa 100644 --- a/web/src/pages/add_logic/loader.rs +++ b/web/src/pages/add_logic/loader.rs @@ -62,3 +62,19 @@ pub(crate) enum LoadError { Forbidden, Backend(String), } + +impl LoadError { + /// Hands the page load's failure to the one place that decides statuses. + /// `permission_key` names the message shown when the load was refused. + pub(crate) fn into_form_error( + self, + locale: crate::i18n::Locale, + permission_key: &str, + ) -> crate::ui::FormError { + match self { + Self::Unauthenticated => crate::ui::FormError::Unauthenticated, + Self::Forbidden => crate::ui::FormError::Forbidden(locale.lookup(permission_key)), + Self::Backend(message) => crate::ui::FormError::Unavailable(message), + } + } +} diff --git a/web/src/pages/add_logic/logic.rs b/web/src/pages/add_logic/logic.rs index 5dc0f73a..2ced7467 100644 --- a/web/src/pages/add_logic/logic.rs +++ b/web/src/pages/add_logic/logic.rs @@ -1,14 +1,15 @@ use axum::{ Form, extract::State, - http::{HeaderMap, StatusCode}, + http::HeaderMap, response::{Html, IntoResponse, Redirect, Response}, }; use crate::{ AppState, - {i18n::Locale, tr}, + i18n::Locale, services::{authenticated_request, reject_cross_site}, + ui::FormError, }; use super::{ @@ -38,12 +39,7 @@ pub(crate) async fn create_logic( } let request = match form.into_request(Locale::from_headers(&headers)) { Ok(request) => request, - Err(message) => { - return render_loaded( - &headers, - load_page(state, &headers, submitted, Some(message)).await, - ); - } + Err(message) => return reject(&headers, message), }; let mut scripts = state.scripts; let request = match authenticated_request(&headers, request) { @@ -57,34 +53,25 @@ pub(crate) async fn create_logic( &response.get_ref().warnings, )) .into_response(), - Err(error) => ( - StatusCode::UNPROCESSABLE_ENTITY, - Html(ui::render_submission_error( - Locale::from_headers(&headers), - error.message(), - )), - ) - .into_response(), + Err(error) => FormError::from_status(&error) + .into_response(Locale::from_headers(&headers), ui::render_submission_error), } } +/// A submission this crate rejected before it reached the backend. Same +/// response as a backend rejection: the user has to change something either +/// way, and the form's `hx-target` is the status block in both cases. +fn reject(headers: &HeaderMap, message: String) -> Response { + FormError::Rejected(message) + .into_response(Locale::from_headers(headers), ui::render_submission_error) +} + fn render_loaded(headers: &HeaderMap, result: Result) -> Response { let locale = Locale::from_headers(headers); match result { Ok(page) => Html(ui::render_page(&page)).into_response(), - Err(LoadError::Unauthenticated) => Redirect::to("/login").into_response(), - Err(LoadError::Forbidden) => { - let message = tr!(locale, "add-logic-err-permission"); - ( - StatusCode::FORBIDDEN, - Html(ui::render_submission_error(locale, &message)), - ) - .into_response() - } - Err(LoadError::Backend(message)) => ( - StatusCode::BAD_GATEWAY, - Html(ui::render_submission_error(locale, &message)), - ) - .into_response(), + Err(error) => error + .into_form_error(locale, "add-logic-err-permission") + .into_response(locale, ui::render_submission_error), } } diff --git a/web/src/pages/add_validation/loader.rs b/web/src/pages/add_validation/loader.rs index 7e3ffaa5..43466997 100644 --- a/web/src/pages/add_validation/loader.rs +++ b/web/src/pages/add_validation/loader.rs @@ -68,3 +68,19 @@ pub(crate) enum LoadError { Forbidden, Backend(String), } + +impl LoadError { + /// Hands the page load's failure to the one place that decides statuses. + /// `permission_key` names the message shown when the load was refused. + pub(crate) fn into_form_error( + self, + locale: crate::i18n::Locale, + permission_key: &str, + ) -> crate::ui::FormError { + match self { + Self::Unauthenticated => crate::ui::FormError::Unauthenticated, + Self::Forbidden => crate::ui::FormError::Forbidden(locale.lookup(permission_key)), + Self::Backend(message) => crate::ui::FormError::Unavailable(message), + } + } +} diff --git a/web/src/pages/add_validation/logic.rs b/web/src/pages/add_validation/logic.rs index 1edbe70d..d0f7bd0a 100644 --- a/web/src/pages/add_validation/logic.rs +++ b/web/src/pages/add_validation/logic.rs @@ -1,14 +1,15 @@ use axum::{ Form, extract::State, - http::{HeaderMap, StatusCode}, + http::HeaderMap, response::{Html, IntoResponse, Redirect, Response}, }; use crate::{ AppState, - {i18n::Locale, tr}, + i18n::Locale, services::{authenticated_request, reject_cross_site}, + ui::FormError, }; use super::{ @@ -17,6 +18,9 @@ use super::{ ui, }; +/// The message shown when the page load itself was refused. +const PERMISSION_KEY: &str = "validation-err-permission"; + pub(crate) async fn new_field_validation_page(State(state): State, headers: HeaderMap) -> Response { render_loaded(&headers, load_page(state, &headers, ValidationForm::default(), false, None).await) } @@ -36,85 +40,51 @@ pub(crate) async fn save_field_validation( return rejection; } let submitted = form.clone(); - if let Err(error) = load_page(state.clone(), &headers, submitted.clone(), false, None).await { - return render_loaded(&headers, Err(error)); + if let Err(error) = load_page(state.clone(), &headers, submitted, false, None).await { + return load_failed(&headers, error); } + // A set name turns this into "apply that set to this column" instead of + // "save these checks on this column". if !form.validation_set_name.trim().is_empty() { let request = match form.apply_set_request(Locale::from_headers(&headers)) { Ok(request) => request, - Err(message) => { - return render_loaded( - &headers, - load_page(state, &headers, submitted, false, Some(message)).await, - ) - } + Err(message) => return reject(&headers, message), }; let mut validations = state.validations; let request = match authenticated_request(&headers, request) { Ok(request) => request, Err(_) => return Redirect::to("/login").into_response(), }; - return match validations.apply_validation_set(request).await { - Ok(response) if response.get_ref().success => Html(ui::render_success( - Locale::from_headers(&headers), - &response.get_ref().message, - )) - .into_response(), - Ok(response) => ( - StatusCode::UNPROCESSABLE_ENTITY, - Html(ui::render_error( - Locale::from_headers(&headers), - &response.get_ref().message, - )), - ) - .into_response(), - Err(error) => ( - StatusCode::UNPROCESSABLE_ENTITY, - Html(ui::render_error( - Locale::from_headers(&headers), - error.message(), - )), - ) - .into_response(), - }; + return answer( + &headers, + validations + .apply_validation_set(request) + .await + .map(|response| { + let response = response.into_inner(); + (response.success, response.message) + }), + ); } let request = match form.field_request(Locale::from_headers(&headers)) { Ok(request) => request, - Err(message) => { - return render_loaded( - &headers, - load_page(state, &headers, submitted, false, Some(message)).await, - ) - } + Err(message) => return reject(&headers, message), }; let mut validations = state.validations; let request = match authenticated_request(&headers, request) { Ok(request) => request, Err(_) => return Redirect::to("/login").into_response(), }; - match validations.update_field_validation(request).await { - Ok(response) if response.get_ref().success => Html(ui::render_success( - Locale::from_headers(&headers), - &response.get_ref().message, - )) - .into_response(), - Ok(response) => ( - StatusCode::UNPROCESSABLE_ENTITY, - Html(ui::render_error( - Locale::from_headers(&headers), - &response.get_ref().message, - )), - ) - .into_response(), - Err(error) => ( - StatusCode::UNPROCESSABLE_ENTITY, - Html(ui::render_error( - Locale::from_headers(&headers), - error.message(), - )), - ) - .into_response(), - } + answer( + &headers, + validations + .update_field_validation(request) + .await + .map(|response| { + let response = response.into_inner(); + (response.success, response.message) + }), + ) } pub(crate) async fn save_validation_rule( @@ -124,46 +94,28 @@ pub(crate) async fn save_validation_rule( return rejection; } let submitted = form.clone(); - if let Err(error) = load_page(state.clone(), &headers, submitted.clone(), true, None).await { - return render_loaded(&headers, Err(error)); + if let Err(error) = load_page(state.clone(), &headers, submitted, true, None).await { + return load_failed(&headers, error); } let request = match form.rule_request(Locale::from_headers(&headers)) { Ok(request) => request, - Err(message) => { - return render_loaded( - &headers, - load_page(state, &headers, submitted, true, Some(message)).await, - ) - } + Err(message) => return reject(&headers, message), }; let mut validations = state.validations; let request = match authenticated_request(&headers, request) { Ok(request) => request, Err(_) => return Redirect::to("/login").into_response(), }; - match validations.upsert_validation_rule(request).await { - Ok(response) if response.get_ref().success => Html(ui::render_success( - Locale::from_headers(&headers), - &response.get_ref().message, - )) - .into_response(), - Ok(response) => ( - StatusCode::UNPROCESSABLE_ENTITY, - Html(ui::render_error( - Locale::from_headers(&headers), - &response.get_ref().message, - )), - ) - .into_response(), - Err(error) => ( - StatusCode::UNPROCESSABLE_ENTITY, - Html(ui::render_error( - Locale::from_headers(&headers), - error.message(), - )), - ) - .into_response(), - } + answer( + &headers, + validations + .upsert_validation_rule(request) + .await + .map(|response| { + let response = response.into_inner(); + (response.success, response.message) + }), + ) } pub(crate) async fn save_validation_set( @@ -173,48 +125,59 @@ pub(crate) async fn save_validation_set( return rejection; } let submitted = form.clone(); - if let Err(error) = load_set_page(state.clone(), &headers, submitted.clone(), None).await { - return render_set_loaded(&headers, Err(error)); + if let Err(error) = load_set_page(state.clone(), &headers, submitted, None).await { + return load_failed(&headers, error); } let request = match form.request(Locale::from_headers(&headers)) { Ok(request) => request, - Err(message) => { - return render_set_loaded( - &headers, - load_set_page(state, &headers, submitted, Some(message)).await, - ) - } + Err(message) => return reject(&headers, message), }; let mut validations = state.validations; let request = match authenticated_request(&headers, request) { Ok(request) => request, Err(_) => return Redirect::to("/login").into_response(), }; - match validations.upsert_validation_set(request).await { - Ok(response) if response.get_ref().success => Html(ui::render_success( - Locale::from_headers(&headers), - &response.get_ref().message, - )) - .into_response(), - Ok(response) => ( - StatusCode::UNPROCESSABLE_ENTITY, - Html(ui::render_error( - Locale::from_headers(&headers), - &response.get_ref().message, - )), - ) - .into_response(), - Err(error) => ( - StatusCode::UNPROCESSABLE_ENTITY, - Html(ui::render_error( - Locale::from_headers(&headers), - error.message(), - )), - ) - .into_response(), + answer( + &headers, + validations + .upsert_validation_set(request) + .await + .map(|response| { + let response = response.into_inner(); + (response.success, response.message) + }), + ) +} + +/// What every one of these three POSTs does with the backend's answer. The +/// endpoints report a refusal in the body (`success: false`) as readily as +/// through a gRPC status, and both are the same thing to the user. +fn answer(headers: &HeaderMap, outcome: Result<(bool, String), tonic::Status>) -> Response { + let locale = Locale::from_headers(headers); + match outcome { + Ok((true, message)) => Html(ui::render_success(locale, &message)).into_response(), + Ok((false, message)) => { + FormError::Rejected(message).into_response(locale, ui::render_error) + } + Err(status) => FormError::from_status(&status).into_response(locale, ui::render_error), } } +/// A submission this crate rejected before it reached the backend — a pattern +/// line in the wrong shape, a missing column. It answers exactly like a +/// backend rejection: the form's `hx-target` is the status block either way, +/// and the user has to change something either way. +fn reject(headers: &HeaderMap, message: String) -> Response { + FormError::Rejected(message).into_response(Locale::from_headers(headers), ui::render_error) +} + +fn load_failed(headers: &HeaderMap, error: LoadError) -> Response { + let locale = Locale::from_headers(headers); + error + .into_form_error(locale, PERMISSION_KEY) + .into_response(locale, ui::render_error) +} + fn render_set_loaded( headers: &HeaderMap, result: Result<(crate::ui::Nav, ValidationSetForm, Option), LoadError>, @@ -223,23 +186,7 @@ fn render_set_loaded( Ok((nav, form, error)) => { Html(ui::render_set_page(nav, &form, error.as_deref())).into_response() } - Err(LoadError::Unauthenticated) => Redirect::to("/login").into_response(), - Err(LoadError::Forbidden) => ( - StatusCode::FORBIDDEN, - Html(ui::render_error( - Locale::from_headers(headers), - &tr!( - Locale::from_headers(headers), - "validation-err-permission" - ), - )), - ) - .into_response(), - Err(LoadError::Backend(message)) => ( - StatusCode::BAD_GATEWAY, - Html(ui::render_error(Locale::from_headers(headers), &message)), - ) - .into_response(), + Err(error) => load_failed(headers, error), } } @@ -249,22 +196,6 @@ fn render_loaded( ) -> Response { match result { Ok(page) => Html(ui::render_page(&page)).into_response(), - Err(LoadError::Unauthenticated) => Redirect::to("/login").into_response(), - Err(LoadError::Forbidden) => ( - StatusCode::FORBIDDEN, - Html(ui::render_error( - Locale::from_headers(headers), - &tr!( - Locale::from_headers(headers), - "validation-err-permission" - ), - )), - ) - .into_response(), - Err(LoadError::Backend(message)) => ( - StatusCode::BAD_GATEWAY, - Html(ui::render_error(Locale::from_headers(headers), &message)), - ) - .into_response(), + Err(error) => load_failed(headers, error), } } diff --git a/web/src/pages/import_export/common/loader.rs b/web/src/pages/import_export/common/loader.rs index bbe1a2f8..b5e8d06d 100644 --- a/web/src/pages/import_export/common/loader.rs +++ b/web/src/pages/import_export/common/loader.rs @@ -113,3 +113,19 @@ pub(crate) enum LoadError { Forbidden, Backend(String), } + +impl LoadError { + /// Hands the page load's failure to the one place that decides statuses. + /// `permission_key` names the message shown when the load was refused. + pub(crate) fn into_form_error( + self, + locale: crate::i18n::Locale, + permission_key: &str, + ) -> crate::ui::FormError { + match self { + Self::Unauthenticated => crate::ui::FormError::Unauthenticated, + Self::Forbidden => crate::ui::FormError::Forbidden(locale.lookup(permission_key)), + Self::Backend(message) => crate::ui::FormError::Unavailable(message), + } + } +} diff --git a/web/src/pages/import_export/export/logic.rs b/web/src/pages/import_export/export/logic.rs index 6d328fe8..c85e7c79 100644 --- a/web/src/pages/import_export/export/logic.rs +++ b/web/src/pages/import_export/export/logic.rs @@ -1,7 +1,7 @@ use axum::{ Form, extract::State, - http::{HeaderMap, HeaderValue, StatusCode, header}, + http::{HeaderMap, HeaderValue, header}, response::{Html, IntoResponse, Redirect, Response}, }; @@ -53,39 +53,22 @@ pub(crate) async fn export_csv( }; let (profile_name, table_names) = match form.targets(Locale::from_headers(&headers)) { Ok(targets) => targets, - Err(message) => { - return ( - StatusCode::BAD_REQUEST, - Html(ui::render_error( - Locale::from_headers(&headers), - &message, - )), - ) - .into_response() - } + Err(message) => return reject(&headers, message), }; let Some(profile) = catalog.profiles.iter().find(|profile| profile.name == profile_name) else { - return ( - StatusCode::BAD_REQUEST, - Html(ui::render_error( - Locale::from_headers(&headers), - &tr!(Locale::from_headers(&headers), "export-err-unknown-profile"), - )), - ) - .into_response(); + return reject( + &headers, + tr!(Locale::from_headers(&headers), "export-err-unknown-profile"), + ); }; if table_names.iter().any(|name| !profile.tables.contains(name)) { - return ( - StatusCode::BAD_REQUEST, - Html(ui::render_error( + return reject( + &headers, + tr!( Locale::from_headers(&headers), - &tr!( - Locale::from_headers(&headers), - "export-err-tables-not-in-profile" - ), - )), - ) - .into_response(); + "export-err-tables-not-in-profile" + ), + ); } let mut tables = Vec::new(); @@ -103,7 +86,7 @@ pub(crate) async fn export_csv( .await { Ok(response) => response.into_inner().table_structures.remove(table_name), - Err(error) => return backend_error(&headers, error.message()), + Err(error) => return grpc_error(&headers, &error), }; let Some(structure) = structure else { return backend_error( @@ -127,7 +110,7 @@ pub(crate) async fn export_csv( .await { Ok(response) => response.into_inner().count, - Err(error) => return backend_error(&headers, error.message()), + Err(error) => return grpc_error(&headers, &error), }; let Ok(count) = u64::try_from(count) else { return backend_error( @@ -170,7 +153,7 @@ pub(crate) async fn export_csv( Err(_) => return Redirect::to("/login").into_response(), }).await { Ok(response) => response.into_inner(), - Err(error) => return backend_error(&headers, error.message()), + Err(error) => return grpc_error(&headers, &error), }; row.extend(table.columns.iter().map(|column| response.data.get(column).cloned().unwrap_or_default())); } @@ -187,27 +170,29 @@ pub(crate) async fn export_csv( } fn load_error(headers: &HeaderMap, error: LoadError) -> Response { - match error { - LoadError::Unauthenticated => Redirect::to("/login").into_response(), - LoadError::Forbidden => ( - StatusCode::FORBIDDEN, - Html(ui::render_error( - Locale::from_headers(headers), - &tr!( - Locale::from_headers(headers), - "export-err-permission" - ), - )), - ) - .into_response(), - LoadError::Backend(message) => backend_error(headers, &message), - } + let locale = Locale::from_headers(headers); + error + .into_form_error(locale, "export-err-permission") + .into_response(locale, ui::render_error) } -fn backend_error(headers: &HeaderMap, message: &str) -> Response { - ( - StatusCode::BAD_GATEWAY, - Html(ui::render_error(Locale::from_headers(headers), message)), - ) - .into_response() +/// An export the user has to change: a profile that is not theirs to read, a +/// table that is not in it. +fn reject(headers: &HeaderMap, message: String) -> Response { + crate::ui::FormError::Rejected(message) + .into_response(Locale::from_headers(headers), ui::render_error) +} + +/// A gRPC failure, classified by its code rather than assumed to be a dead +/// backend. +fn grpc_error(headers: &HeaderMap, error: &tonic::Status) -> Response { + crate::ui::FormError::from_status(error) + .into_response(Locale::from_headers(headers), ui::render_error) +} + +/// The backend answered, with something the export cannot use — a structure +/// it does not know, a row count that is negative or past `i32`. +fn backend_error(headers: &HeaderMap, message: &str) -> Response { + crate::ui::FormError::Unavailable(message.to_string()) + .into_response(Locale::from_headers(headers), ui::render_error) } diff --git a/web/src/pages/import_export/import/logic.rs b/web/src/pages/import_export/import/logic.rs index 225dcf83..fde25eb8 100644 --- a/web/src/pages/import_export/import/logic.rs +++ b/web/src/pages/import_export/import/logic.rs @@ -3,7 +3,7 @@ use std::collections::{HashMap, HashSet}; use axum::{ Form, extract::State, - http::{HeaderMap, StatusCode}, + http::HeaderMap, response::{Html, IntoResponse, Redirect, Response}, }; @@ -53,61 +53,31 @@ pub(crate) async fn import_csv( }; let (profile_name, table_names) = match form.targets(Locale::from_headers(&headers)) { Ok(targets) => targets, - Err(message) => { - return render_loaded( - &headers, - load_page(state, &headers, submitted, Some(message)).await, - ) - } + Err(message) => return reject(&headers, message), }; let Some(profile) = page.catalog.profiles.iter().find(|profile| profile.name == profile_name) else { - return render_loaded( + return reject( &headers, - load_page( - state, - &headers, - submitted, - Some(tr!( - Locale::from_headers(&headers), - "import-err-unknown-profile" - )), - ) - .await, + tr!(Locale::from_headers(&headers), "import-err-unknown-profile"), ); }; if table_names.iter().any(|name| !profile.tables.contains(name)) { - return render_loaded( + return reject( &headers, - load_page( - state, - &headers, - submitted, - Some(tr!( - Locale::from_headers(&headers), - "import-err-tables-not-in-profile" - )), - ) - .await, + tr!( + Locale::from_headers(&headers), + "import-err-tables-not-in-profile" + ), ); } let rows = match parse_csv(Locale::from_headers(&headers), &form.csv_data) { Ok(rows) => rows, - Err(message) => { - return render_loaded( - &headers, - load_page(state, &headers, submitted, Some(message)).await, - ) - } + Err(message) => return reject(&headers, message), }; let (table_headers, columns, data_rows) = match split_headers(Locale::from_headers(&headers), rows, &table_names) { Ok(parts) => parts, - Err(message) => { - return render_loaded( - &headers, - load_page(state, &headers, submitted, Some(message)).await, - ) - } + Err(message) => return reject(&headers, message), }; let mut tables = Vec::new(); @@ -122,10 +92,10 @@ pub(crate) async fn import_csv( Err(_) => return Redirect::to("/login").into_response(), }).await { Ok(response) => response.into_inner().table_structures.remove(table_name), - Err(error) => return backend_error(&headers, error.message().to_string()), + Err(error) => return grpc_error(&headers, &error), }; let Some(structure) = structure else { - return backend_error( + return unavailable( &headers, tr!( Locale::from_headers(&headers), @@ -153,38 +123,26 @@ pub(crate) async fn import_csv( .map(|(index, column)| (index, column.clone())) .collect::>(); if positions.is_empty() { - return render_loaded( + return reject( &headers, - load_page( - state, - &headers, - submitted, - Some(tr!( - Locale::from_headers(&headers), - "import-err-no-importable-columns", - "table" => table.name.clone(), - )), - ) - .await, + tr!( + Locale::from_headers(&headers), + "import-err-no-importable-columns", + "table" => table.name.clone(), + ), ); } for (index, column) in columns.iter().enumerate() { let belongs = table_headers.as_ref().map_or(table_names.len() == 1, |headers| headers.get(index).is_some_and(|name| name == &table.name)); if belongs && !table.columns.contains(column) { - return render_loaded( + return reject( &headers, - load_page( - state, - &headers, - submitted, - Some(tr!( - Locale::from_headers(&headers), - "import-err-column-not-importable", - "column" => column.clone(), - "table" => table.name.clone(), - )), - ) - .await, + tr!( + Locale::from_headers(&headers), + "import-err-column-not-importable", + "column" => column.clone(), + "table" => table.name.clone(), + ), ); } } @@ -206,12 +164,7 @@ pub(crate) async fn import_csv( .collect::, _>>() { Ok(rows) => rows, - Err(message) => { - return render_loaded( - &headers, - load_page(state, &headers, submitted, Some(message)).await, - ) - } + Err(message) => return reject(&headers, message), }; for chunk in converted.chunks(1_000) { let request = PostTableDataBulkRequest { @@ -225,7 +178,7 @@ pub(crate) async fn import_csv( Err(_) => return Redirect::to("/login").into_response(), }).await { Ok(response) => response.into_inner(), - Err(error) => return backend_error(&headers, error.message().to_string()), + Err(error) => return grpc_error(&headers, &error), }; inserted += response.responses.iter().filter(|row| row.inserted_id > 0).count(); } @@ -309,30 +262,38 @@ fn render_loaded(headers: &HeaderMap, result: Result Response { - match error { - LoadError::Unauthenticated => Redirect::to("/login").into_response(), - LoadError::Forbidden => ( - StatusCode::FORBIDDEN, - Html(ui::render_error( - Locale::from_headers(headers), - &tr!( - Locale::from_headers(headers), - "import-err-permission" - ), - )), - ) - .into_response(), - LoadError::Backend(message) => backend_error(headers, message), - } +/// A CSV this crate rejected before any of it reached the backend: an unknown +/// profile, a column the table has no place for, an unparseable cell. The +/// form's `hx-target` is `#submission-status`, so the answer is the alert +/// fragment — re-rendering the page here sent a whole `` document to be +/// swapped into a status block. +fn reject(headers: &HeaderMap, message: String) -> Response { + crate::ui::FormError::Rejected(message) + .into_response(Locale::from_headers(headers), ui::render_error) } -fn backend_error(headers: &HeaderMap, message: String) -> Response { - ( - StatusCode::BAD_GATEWAY, - Html(ui::render_error(Locale::from_headers(headers), &message)), - ) - .into_response() +/// A gRPC failure, classified by its code. An import that the server refused +/// because of what was *in* the CSV — a script-consistency mismatch, a +/// validation pattern the value does not satisfy — is the user's to fix and +/// answers 422; only an actually unreachable backend answers 502. +fn grpc_error(headers: &HeaderMap, error: &tonic::Status) -> Response { + crate::ui::FormError::from_status(error) + .into_response(Locale::from_headers(headers), ui::render_error) +} + +fn load_error(headers: &HeaderMap, error: LoadError) -> Response { + let locale = Locale::from_headers(headers); + error + .into_form_error(locale, "import-err-permission") + .into_response(locale, ui::render_error) +} + +/// The backend answered, but not with what the import needs — a table the +/// catalogue lists and the structure service does not know. Nothing the user +/// can fix from the form. +fn unavailable(headers: &HeaderMap, message: String) -> Response { + crate::ui::FormError::Unavailable(message) + .into_response(Locale::from_headers(headers), ui::render_error) } #[cfg(test)] diff --git a/web/src/pages/login/logic.rs b/web/src/pages/login/logic.rs index 622c3b6d..9190bd74 100644 --- a/web/src/pages/login/logic.rs +++ b/web/src/pages/login/logic.rs @@ -84,9 +84,11 @@ pub(crate) async fn login( Locale::from_headers(&headers), "login-error-identifier-required" ); + // 422, not 400: the form parsed fine, its contents were refused — + // the same class as every other rejected submission in the app. return error( Locale::from_headers(&headers), - StatusCode::BAD_REQUEST, + StatusCode::UNPROCESSABLE_ENTITY, &message, ); } diff --git a/web/src/pages/register/logic.rs b/web/src/pages/register/logic.rs index 59b8fc52..d4a3e43e 100644 --- a/web/src/pages/register/logic.rs +++ b/web/src/pages/register/logic.rs @@ -1,7 +1,7 @@ use axum::{ Form, extract::State, - http::{HeaderMap, StatusCode}, + http::HeaderMap, response::{Html, IntoResponse, Response}, }; use tonic::Request; @@ -44,14 +44,15 @@ pub(crate) async fn register( { Ok(response) => response.into_inner(), Err(status) => { - return ( - StatusCode::BAD_REQUEST, - Html(ui::render_error( - Locale::from_headers(&headers), - status.message(), - )), - ) - .into_response(); + // /register is public, so `Unauthenticated` here is the backend + // refusing this submission, not a lost session to redirect on. + let failure = match crate::ui::FormError::from_status(&status) { + crate::ui::FormError::Unauthenticated => { + crate::ui::FormError::Rejected(status.message().to_string()) + } + other => other, + }; + return failure.into_response(Locale::from_headers(&headers), ui::render_error); } }; diff --git a/web/src/ui/mod.rs b/web/src/ui/mod.rs index e66c5ee1..dcc9a467 100644 --- a/web/src/ui/mod.rs +++ b/web/src/ui/mod.rs @@ -5,10 +5,96 @@ //! `templates//`. use askama::Template; -use axum::http::HeaderMap; +use axum::{ + http::{HeaderMap, StatusCode}, + response::{Html, IntoResponse, Redirect, Response}, +}; pub(crate) const SESSION_COOKIE: &str = "analytics_token"; +/// Why a form submission failed. +/// +/// The status a failure answers with is decided here, once, by *what went +/// wrong* — never by which handler happened to notice. Handlers used to pick +/// their own, and the same class of failure ended up as 200 on one form, 422 +/// on another and 502 on a third; the browser hid it, because +/// `ui/base.html` swaps every response body regardless of status. +pub(crate) enum FormError { + /// The submission was understood and rejected: the user has to change + /// something before it can succeed. Whether this crate spotted it while + /// building the request or the backend spotted it afterwards is an + /// implementation detail of *when*, not a difference in kind. + Rejected(String), + /// The session is gone. Nothing is rendered; the browser goes to /login. + Unauthenticated, + /// Signed in, but not allowed to do this. + Forbidden(String), + /// Nothing wrong with the submission — the backend could not answer. + Unavailable(String), +} + +impl FormError { + /// Classifies a gRPC failure by its code. The blanket `Err(_) =>` arms + /// this replaces reported an unreachable backend as a rejected submission + /// on some pages and a rejected submission as an unreachable backend on + /// others. + pub(crate) fn from_status(status: &tonic::Status) -> Self { + let message = status.message().to_string(); + match status.code() { + tonic::Code::Unauthenticated => Self::Unauthenticated, + tonic::Code::PermissionDenied => Self::Forbidden(message), + // Everything the caller can fix by editing the form: a value the + // server rejected, a name already taken, a row that does not + // satisfy the table's own rules. + tonic::Code::InvalidArgument + | tonic::Code::FailedPrecondition + | tonic::Code::OutOfRange + | tonic::Code::AlreadyExists + | tonic::Code::NotFound => Self::Rejected(message), + _ => Self::Unavailable(message), + } + } + + pub(crate) fn status_code(&self) -> StatusCode { + match self { + // Understood, and refused. Not 400: the request parsed fine. + Self::Rejected(_) => StatusCode::UNPROCESSABLE_ENTITY, + Self::Unauthenticated => StatusCode::UNAUTHORIZED, + Self::Forbidden(_) => StatusCode::FORBIDDEN, + Self::Unavailable(_) => StatusCode::BAD_GATEWAY, + } + } + + pub(crate) fn message(&self) -> &str { + match self { + Self::Rejected(message) | Self::Forbidden(message) | Self::Unavailable(message) => { + message + } + Self::Unauthenticated => "", + } + } + + /// The response, rendered through the page's own alert renderer so each + /// form keeps its own error title. + /// + /// The body is always the alert *fragment*, because that is what a form's + /// `hx-target` is: a status block. Re-rendering the whole page here — as + /// the locally-detected errors used to — sends a complete `` + /// document to be swapped into a `
` inside the page that is already + /// open. + pub(crate) fn into_response( + self, + locale: crate::i18n::Locale, + render_alert: fn(crate::i18n::Locale, &str) -> String, + ) -> Response { + if matches!(self, Self::Unauthenticated) { + return Redirect::to("/login").into_response(); + } + let status = self.status_code(); + (status, Html(render_alert(locale, self.message()))).into_response() + } +} + /// Navbar state. Every full-page template struct carries one of these, because /// `ui/base.html` renders `ui/navbar.html` unconditionally. #[derive(Clone, Debug)] @@ -268,6 +354,52 @@ pub(crate) fn render(template: &T) -> String { mod tests { use super::*; + /// The status is a property of what went wrong, and a gRPC failure knows + /// which it was. The blanket arms this replaced answered 422 for an + /// unreachable backend on one page and 502 for a rejected row on another. + #[test] + fn a_backend_failure_is_classified_by_its_code() { + let rejected = FormError::from_status(&tonic::Status::invalid_argument( + "Script calculated '3', but user provided '99'", + )); + assert_eq!(rejected.status_code(), StatusCode::UNPROCESSABLE_ENTITY); + + let unreachable = + FormError::from_status(&tonic::Status::unavailable("connection refused")); + assert_eq!(unreachable.status_code(), StatusCode::BAD_GATEWAY); + + let forbidden = + FormError::from_status(&tonic::Status::permission_denied("not your profile")); + assert_eq!(forbidden.status_code(), StatusCode::FORBIDDEN); + + assert!(matches!( + FormError::from_status(&tonic::Status::unauthenticated("expired")), + FormError::Unauthenticated + )); + } + + /// A submission refused here and a submission refused by the backend are + /// the same event to the user, so they are the same response: 422, and the + /// alert *fragment* the form's `hx-target` expects — never a whole page. + #[test] + fn a_rejection_answers_with_a_fragment_and_not_a_document() { + fn alert(locale: crate::i18n::Locale, message: &str) -> String { + render(&Alert::error(locale, "Could not save", message)) + } + + let response = FormError::Rejected("Pattern line 1 is malformed.".to_string()) + .into_response(crate::i18n::Locale::default(), alert); + assert_eq!(response.status(), StatusCode::UNPROCESSABLE_ENTITY); + + // What that response carries. A form targets a status block, so the + // body has to be a fragment: the locally-detected errors used to + // answer with a re-rendered page, i.e. a whole document to be swapped + // into a `
` inside the document already open. + let body = alert(crate::i18n::Locale::default(), "Pattern line 1 is malformed."); + assert!(!body.contains("