Skip to content

Commit

Permalink
feat: 🥅 set up proper error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
arctic-hen7 committed Aug 17, 2021
1 parent 5b403b2 commit 7ea3ec0
Show file tree
Hide file tree
Showing 7 changed files with 244 additions and 69 deletions.
38 changes: 30 additions & 8 deletions examples/showcase/app/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ pub mod pages;
use sycamore::prelude::*;
use sycamore_router::{Route, BrowserRouter};
use wasm_bindgen::prelude::*;
use perseus::shell::app_shell;
use perseus::shell::{app_shell, ErrorPages};

// Define our routes
#[derive(Route)]
Expand All @@ -30,6 +30,20 @@ enum AppRoute {
NotFound
}

fn get_error_pages() -> ErrorPages {
let mut error_pages = ErrorPages::new(Box::new(|_, _, _| template! {
p { "Another error occurred." }
}));
error_pages.add_page(404, Box::new(|_, _, _| template! {
p { "Page not found." }
}));
error_pages.add_page(400, Box::new(|_, _, _| template! {
p { "Client error occurred..." }
}));

error_pages
}

// This is deliberately purely client-side rendered
#[wasm_bindgen]
pub fn run() -> Result<(), JsValue> {
Expand All @@ -47,34 +61,42 @@ pub fn run() -> Result<(), JsValue> {
||
template! {
BrowserRouter(|route: AppRoute| {
// TODO improve performance rather than naively copying error pages for every template
match route {
AppRoute::Index => app_shell(
"index".to_string(),
pages::index::template_fn()
pages::index::template_fn(),
get_error_pages()
),
AppRoute::About => app_shell(
"about".to_string(),
pages::about::template_fn()
pages::about::template_fn(),
get_error_pages()
),
AppRoute::Post { slug } => app_shell(
format!("post/{}", slug),
pages::post::template_fn()
pages::post::template_fn(),
get_error_pages()
),
AppRoute::NewPost => app_shell(
"post/new".to_string(),
pages::new_post::template_fn()
pages::new_post::template_fn(),
get_error_pages()
),
AppRoute::Ip => app_shell(
"ip".to_string(),
pages::ip::template_fn()
pages::ip::template_fn(),
get_error_pages()
),
AppRoute::Time { slug } => app_shell(
format!("timeisr/{}", slug),
pages::time::template_fn()
pages::time::template_fn(),
get_error_pages()
),
AppRoute::TimeRoot => app_shell(
"time".to_string(),
pages::time_root::template_fn()
pages::time_root::template_fn(),
get_error_pages()
),
AppRoute::NotFound => template! {
p {"Not Found."}
Expand Down
4 changes: 3 additions & 1 deletion examples/showcase/app/src/pages/ip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use serde::{Serialize, Deserialize};
use sycamore::prelude::{template, component, GenericNode, Template as SycamoreTemplate};
use perseus::template::Template;
use perseus::errors::ErrorCause;

#[derive(Serialize, Deserialize)]
pub struct IpPageProps {
Expand All @@ -26,7 +27,8 @@ pub fn get_page<G: GenericNode>() -> Template<G> {
.template(template_fn())
}

pub async fn get_request_state(_path: String) -> Result<String, String> {
pub async fn get_request_state(_path: String) -> Result<String, (String, ErrorCause)> {
// Err(("this is a test error!".to_string(), ErrorCause::Client(None)))
Ok(serde_json::to_string(
&IpPageProps {
ip: "x.x.x.x".to_string()
Expand Down
2 changes: 1 addition & 1 deletion examples/showcase/app/src/pages/post.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub async fn get_static_props(path: String) -> Result<String, String> {
}
).unwrap())
}
// TODO

pub async fn get_static_paths() -> Result<Vec<String>, String> {
Ok(vec![
"test".to_string()
Expand Down
21 changes: 13 additions & 8 deletions examples/showcase/server/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
use actix_web::{web, App, HttpRequest, HttpServer, Result as ActixResult, error};
use actix_web::{web, App, HttpRequest, HttpServer, HttpResponse, http::StatusCode};
use actix_files::{NamedFile};
use sycamore::prelude::SsrNode;
use std::collections::HashMap;

use perseus::{
serve::{get_render_cfg, get_page},
config_manager::FsConfigManager,
template::TemplateMap
template::TemplateMap,
errors::ErrorKind as PerseusErr,
errors::err_to_status_code
};
use perseus_showcase_app::pages;

Expand Down Expand Up @@ -53,12 +55,15 @@ async fn page_data(
templates: web::Data<TemplateMap<SsrNode>>,
render_cfg: web::Data<HashMap<String, String>>,
config_manager: web::Data<FsConfigManager>
) -> ActixResult<String> {
) -> HttpResponse {
let path = req.match_info().query("filename");
// TODO match different types of errors here
let page_data = get_page(path, &render_cfg, &templates, config_manager.get_ref()).await.map_err(error::ErrorNotFound)?;
let page_data = get_page(path, &render_cfg, &templates, config_manager.get_ref()).await;
let http_res = match page_data {
Ok(page_data) => HttpResponse::Ok().body(
serde_json::to_string(&page_data).unwrap()
),
Err(err) => HttpResponse::build(StatusCode::from_u16(err_to_status_code(&err)).unwrap()).body(err.to_string()),
};

Ok(
serde_json::to_string(&page_data).unwrap()
)
http_res
}
67 changes: 60 additions & 7 deletions src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,39 +3,64 @@
pub use error_chain::bail;
use error_chain::error_chain;

/// Defines who caused an ambiguous error message so we can reliably create an HTTP status code. Specific status codes may be provided
/// in either case, or the defaults (400 for client, 500 for server) will be used.
#[derive(Debug)]
pub enum ErrorCause {
Client(Option<u16>),
Server(Option<u16>)
}

// TODO disclose what information may be revealed over the network through these errors in docs
// The `error_chain` setup for the whole crate
error_chain! {
// The custom errors for this crate (very broad)
errors {
/// For indistinct JavaScript errors.
/// For indistinct JavaScript errors (potentially sensitive, but only generated on the client-side).
JsErr(err: String) {
description("an error occurred while interfacing with javascript")
display("the following error occurred while interfacing with javascript: {:?}", err)
}
/// For when a fetched URL didn't return a string, which it must.
AssetNotString(url: String) {
description("the fetched asset wasn't a string")
display("the fetched asset at '{}' wasn't a string", url)
}
/// For when the server returned a non-200 error code (not including 404, that's handled separately).
AssetNotOk(url: String, status: u16, err: String) {
description("the asset couldn't be fecthed with a 200 OK")
display("the asset at '{}' returned status code '{}' with payload '{}'", url, status, err)
}

/// For when a necessary template feautre was expected but not present. This just pertains to rendering strategies, and shouldn't
/// ever be sensitive.
TemplateFeatureNotEnabled(name: String, feature: String) {
description("a template feature required by a function called was not present")
display("the template '{}' is missing the feature '{}'", name, feature)
}
/// For when the given path wasn't found, a 404 should never be sensitive.
PageNotFound(path: String) {
description("the requested page was not found")
display("the requested page at path '{}' was not found", path)
}
NoRenderOpts(template_path: String) {
description("a template had no rendering options for use at request-time")
display("the template '{}' had no rendering options for use at request-time", template_path)
}
/// For when the user misconfigured their revalidation length, which should be caught at build time, and hence shouldn't be
/// sensitive.
InvalidDatetimeIntervalIndicator(indicator: String) {
description("invalid indicator in timestring")
display("invalid indicator '{}' in timestring, must be one of: s, m, h, d, w, M, y", indicator)
}
/// For when a template defined both build and request states when it can't amalgamate them sensibly, which indicates a misconfiguration.
/// Revealing the rendering strategies of a template in this way should never be sensitive. Due to the execution context, this
/// doesn't disclose the offending template.
BothStatesDefined {
description("both build and request states were defined for a template when only one or fewer were expected")
display("both build and request states were defined for a template when only one or fewer were expected")
}
RenderFnFailed(fn_name: String, template: String, err_str: String) {
/// For when a render function failed. Only request-time functions can generate errors that will be transmitted over the network,
/// so **render functions must not disclose sensitive information in errors**. Other information shouldn't be sensitive.
RenderFnFailed(fn_name: String, template: String, cause: ErrorCause, err_str: String) {
description("error while calling render function")
display("an error occurred while calling render function '{}' on template '{}': '{}'", fn_name, template, err_str)
display("an error caused by '{:?}' occurred while calling render function '{}' on template '{}': '{}'", cause, fn_name, template, err_str)
}
}
links {
Expand All @@ -48,3 +73,31 @@ error_chain! {
ChronoParse(::chrono::ParseError);
}
}

pub fn err_to_status_code(err: &Error) -> u16 {
match err.kind() {
// Misconfiguration
ErrorKind::TemplateFeatureNotEnabled(_, _) => 500,
// Bad request
ErrorKind::PageNotFound(_) => 404,
// Misconfiguration
ErrorKind::InvalidDatetimeIntervalIndicator(_) => 500,
// Misconfiguration
ErrorKind::BothStatesDefined => 500,
// Ambiguous, we'll rely on the given cause
ErrorKind::RenderFnFailed(_, _, cause, _) => match cause {
ErrorCause::Client(code) => code.unwrap_or(400),
ErrorCause::Server(code) => code.unwrap_or(500),
},
// We shouldn't be generating JS errors on the server...
ErrorKind::JsErr(_) => panic!("function 'err_to_status_code' is only intended for server-side usage"),
// These are nearly always server-induced
ErrorKind::ConfigManager(_) => 500,
ErrorKind::Io(_) => 500,
ErrorKind::ChronoParse(_) => 500,
// JSON errors can be caused by the client, but we don't have enough information
ErrorKind::Json(_) => 500,
// Any other errors go to a 500
_ => 500
}
}
Loading

0 comments on commit 7ea3ec0

Please sign in to comment.