implement default 404 error page

... and update routing logic to serve it; remove old 404 handling code; add tests for new behavior

Author: lovasoa

CHANGELOG.md

@@ -4,6 +4,7 @@
  - Fix a bug with zero values being displayed with a non-zero height in stacked bar charts.
  - Updated dependencies, including the embedded SQLite database.
  - Release binaries are now dynamically linked again, but use GLIBC 2.28 ([released in 2018](https://sourceware.org/glibc/wiki/Glibc%20Timeline)), with is compatible with older linux distributions.
+ - When an user requests a page that does not exist (and the site owner did not provide a custom 404.sql file), we now serve a nice visual 404 web page instead of the ugly textual message and the verbose log messages we used to have.
 
 ## v0.35.1
  - improve color palette for charts

_default_404.sql

@@ -0,0 +1,31 @@
+SELECT
+    'shell' as component,
+    'Page Not Found' as title,
+    'error-404' as body_class,
+    '/' as link;
+
+SELECT
+    'empty_state' as component,
+    'Page Not Found' as title,
+    '404' as header,
+    'The page you were looking for does not exist.' as description_md,
+    'Go to Homepage' as link_text,
+    'home' as link_icon,
+    '/' as link;
+
+select
+    'text' as component,
+    '
+> **Routing Debug Info**  
+> When a URL is requested, SQLPage looks for matching files in this order:
+> 1. **Exact filename match** (e.g. `page.html` for `/page.html`)
+> 2. **For paths ending with `/`**:
+>    - Looks for `index.sql` in that directory (e.g. `/dir/` → `dir/index.sql`)
+> 3. **For paths without extensions**:
+>    - First tries adding `.sql` extension (e.g. `/dir/page` → `dir/page.sql`)
+>    - If not found, redirects to add trailing `/` (e.g. `/dir` → `/dir/`)
+> 4. **If no matches found**:
+>    - Searches for `404.sql` in current and parent directories (e.g. `dir/x/y/` could use `dir/404.sql`)
+>
+> Try creating one of these files to handle this route.
+' as contents_md;

src/lib.rs

@@ -120,6 +120,14 @@ impl AppState {
             PathBuf::from("index.sql"),
             ParsedSqlFile::new(&db, include_str!("../index.sql"), Path::new("index.sql")),
         );
+        sql_file_cache.add_static(
+            PathBuf::from("_default_404.sql"),
+            ParsedSqlFile::new(
+                &db,
+                include_str!("../_default_404.sql"),
+                Path::new("_default_404.sql"),
+            ),
+        );
 
         let oidc_state = crate::webserver::oidc::initialize_oidc_state(config).await?;
 

src/webserver/http.rs

@@ -357,29 +357,6 @@ fn strip_site_prefix<'a>(path: &'a str, state: &AppState) -> &'a str {
     path.strip_prefix(&state.config.site_prefix).unwrap_or(path)
 }
 
-/// Serves a fallback 404 error page (when no 404 handler is found)
-fn serve_not_found(service_request: &mut ServiceRequest) -> actix_web::Error {
-    let app_state: &web::Data<AppState> = service_request.app_data().expect("app_state");
-    let source_err = anyhow::anyhow!(ErrorWithStatus {
-        status: StatusCode::NOT_FOUND
-    });
-    let path = service_request.path();
-    let mut message = format!("{path} does not exist.");
-    if !app_state.config.environment.is_prod() {
-        message.push_str("\n\nRouting Debug Info:\n\
-        - SQLPage first looks for an exact match of your file (e.g. 'page.sql')\n\
-        - For paths without extensions that end in '/', SQLPage looks for 'index.sql' in that directory (e.g. '/dir/' loads 'dir/index.sql')\n\
-        - For paths without extensions that don't end in '/', SQLPage tries:\n\
-          1. Looking for the path with '.sql' extension (e.g. '/dir/page' loads 'dir/page.sql')\n\
-          2. If not found, redirects by adding a trailing '/' (e.g. '/dir' redirects to '/dir/')\n\
-        - When no file is found, SQLPage looks for '404.sql' in the current and parent directories (e.g. 'dir/x/y' may load 'dir/404.sql')\n\
-        \nTry creating one of these files to handle this route.");
-    }
-
-    let err = source_err.context(message);
-    anyhow_err_to_actix(err, app_state.config.environment)
-}
-
 pub async fn main_handler(
     mut service_request: ServiceRequest,
 ) -> actix_web::Result<ServiceResponse> {
@@ -399,7 +376,13 @@ pub async fn main_handler(
         }
     };
     match routing_action {
-        NotFound => Err(serve_not_found(&mut service_request)),
+        NotFound => {
+            let mut response =
+                process_sql_request(&mut service_request, PathBuf::from("_default_404.sql"))
+                    .await?;
+            *response.status_mut() = StatusCode::NOT_FOUND;
+            Ok(response)
+        }
         Execute(path) => process_sql_request(&mut service_request, path).await,
         CustomNotFound(path) => {
             // Currently, we do not set a 404 status when the user provides a fallback 404.sql file.

tests/basic/mod.rs

@@ -30,19 +30,6 @@ async fn test_access_config_forbidden() {
     );
 }
 
-#[actix_web::test]
-async fn test_404() {
-    for f in [
-        "/does_not_exist.sql",
-        "/does_not_exist.html",
-        "/does_not_exist/",
-    ] {
-        let resp_result = req_path(f).await;
-        let resp = resp_result.unwrap_err().error_response();
-        assert_eq!(resp.status(), http::StatusCode::NOT_FOUND, "{f} isnt 404");
-    }
-}
-
 #[actix_web::test]
 async fn test_static_files() {
     let resp = req_path("/tests/it_works.txt").await.unwrap();

tests/core/mod.rs

@@ -104,11 +104,12 @@ async fn test_routing_with_prefix() {
 
     let resp = req_path_with_app_data("/prefix/nonexistent.sql", app_data.clone())
         .await
-        .expect_err("Expected 404 error")
-        .to_string();
+        .expect("should handle 404");
+    let body = test::read_body(resp).await;
+    let body_str = String::from_utf8(body.to_vec()).unwrap();
     assert!(
-        resp.contains("404"),
-        "Response should contain \"404\", but got:\n{resp}"
+        body_str.contains("404"),
+        "Response should contain \"404\", but got:\n{body_str}"
     );
 
     let resp = req_path_with_app_data("/prefix/sqlpage/migrations/0001_init.sql", app_data.clone())

tests/errors/mod.rs

@@ -41,3 +41,62 @@ async fn test_404_fallback() {
         assert!(!body.contains("error"));
     }
 }
+
+#[actix_web::test]
+async fn test_default_404() {
+    for f in [
+        "/i-do-not-exist.html",
+        "/i-do-not-exist.sql",
+        "/i-do-not-exist/",
+    ] {
+        let resp_result = req_path(f).await;
+        let resp = resp_result.unwrap();
+        assert_eq!(
+            resp.status(),
+            http::StatusCode::NOT_FOUND,
+            "{f} should return 404"
+        );
+
+        let body = test::read_body(resp).await;
+        assert!(body.starts_with(b"<!DOCTYPE html>"));
+        let body = String::from_utf8(body.to_vec()).unwrap();
+        let msg = "The page you were looking for does not exist";
+        assert!(
+            body.contains(msg),
+            "{f} should contain '{msg}' but got:\n{body}"
+        );
+        assert!(!body.contains("error"));
+    }
+}
+
+#[actix_web::test]
+async fn test_default_404_with_redirect() {
+    let resp_result = req_path("/i-do-not-exist").await;
+    let resp = resp_result.unwrap();
+    assert_eq!(
+        resp.status(),
+        http::StatusCode::MOVED_PERMANENTLY,
+        "/i-do-not-exist should return 301"
+    );
+
+    let location = resp.headers().get(http::header::LOCATION).unwrap();
+    assert_eq!(location, "/i-do-not-exist/");
+
+    let resp_result = req_path("/i-do-not-exist/").await;
+    let resp = resp_result.unwrap();
+    assert_eq!(
+        resp.status(),
+        http::StatusCode::NOT_FOUND,
+        "/i-do-not-exist/ should return 404"
+    );
+
+    let body = test::read_body(resp).await;
+    assert!(body.starts_with(b"<!DOCTYPE html>"));
+    let body = String::from_utf8(body.to_vec()).unwrap();
+    let msg = "The page you were looking for does not exist";
+    assert!(
+        body.contains(msg),
+        "/i-do-not-exist/ should contain '{msg}' but got:\n{body}"
+    );
+    assert!(!body.contains("error"));
+}