Skip to content

Commit

Permalink
feat: removed unnecessary panics and added custom panic handler system
Browse files Browse the repository at this point in the history
The very few edge cases of server failure in which Perseus would panic
have been rectified, and the user can now define a custom panic handler,
e.g. for displaying a message to the user that tells them that the app
has panicked (optimally with a button to reload).
  • Loading branch information
arctic-hen7 committed Nov 11, 2022
1 parent 8e3d55f commit c232aed
Show file tree
Hide file tree
Showing 7 changed files with 154 additions and 43 deletions.
1 change: 1 addition & 0 deletions packages/perseus/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ minify-html-onepass = "0.10.1"
[target.'cfg(target_arch = "wasm32")'.dependencies]
rexie = { version = "0.2", optional = true }
js-sys = { version = "0.3", optional = true }
# Note that this is not needed in production, but that can't be specified, so it will just be compiled away to nothing
console_error_panic_hook = { version = "0.1.6", optional = true }
# TODO review feature flags here
web-sys = { version = "0.3", features = [ "Headers", "Navigator", "NodeList", "Request", "RequestInit", "RequestMode", "Response", "ReadableStream", "Window" ] }
Expand Down
19 changes: 14 additions & 5 deletions packages/perseus/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@ use crate::{
router::{perseus_router, PerseusRouterProps},
template::TemplateNodeType,
};
use crate::{i18n::TranslationsManager, stores::MutableStore, PerseusAppBase};
use std::collections::HashMap;
use wasm_bindgen::JsValue;

use crate::{i18n::TranslationsManager, stores::MutableStore, PerseusAppBase};

/// The entrypoint into the app itself. This will be compiled to Wasm and
/// actually executed, rendering the rest of the app. Runs the app in the
/// browser on the client-side. This is designed to be executed in a function
Expand All @@ -22,12 +21,22 @@ use crate::{i18n::TranslationsManager, stores::MutableStore, PerseusAppBase};
pub fn run_client<M: MutableStore, T: TranslationsManager>(
app: impl Fn() -> PerseusAppBase<TemplateNodeType, M, T>,
) -> Result<(), JsValue> {
let app = app();
let mut app = app();
let plugins = app.get_plugins();
let panic_handler = app.take_panic_handler();

checkpoint("begin");
// Panics should always go to the console
std::panic::set_hook(Box::new(console_error_panic_hook::hook));

// Handle panics (this works for unwinds and aborts)
std::panic::set_hook(Box::new(move |panic_info| {
// Print to the console in development
#[cfg(debug_assertions)]
console_error_panic_hook::hook(panic_info);
// If the user wants a little warning dialogue, create that
if let Some(panic_handler) = &panic_handler {
panic_handler(panic_info);
}
}));

plugins
.functional_actions
Expand Down
2 changes: 2 additions & 0 deletions packages/perseus/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ pub enum ClientError {
#[source]
source: serde_json::Error,
},
#[error("server informed us that a valid locale was invald (this almost certainly requires a hard reload)")]
ValidLocaleNotProvided { locale: String },
#[error("the given path for preloading leads to a locale detection page; you probably wanted to wrap the path in `link!(...)`")]
PreloadLocaleDetection,
#[error("the given path for preloading was not found")]
Expand Down
30 changes: 11 additions & 19 deletions packages/perseus/src/i18n/client_translations_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,26 +125,18 @@ impl ClientTranslationsManager {
let asset_url = format!("{}/.perseus/translations/{}", path_prefix, locale);
// If this doesn't exist, then it's a 404 (we went here by explicit navigation
// after checking the locale, so that's a bug)
let translations_str = fetch(&asset_url).await;
let translations_str = fetch(&asset_url).await?;
let translator = match translations_str {
Ok(translations_str) => match translations_str {
Some(translations_str) => {
// All good, turn the translations into a translator
self.get_translator_for_translations_str(locale, &translations_str)?
}
// If we get a 404 for a supported locale, that's an exception
None => panic!(
"server returned 404 for translations for known supported locale '{}'",
locale
),
},
Err(err) => match err {
not_ok_err @ ClientError::FetchError(FetchError::NotOk { .. }) => {
return Err(not_ok_err)
}
// No other errors should be returned
_ => panic!("expected 'AssetNotOk' error, found other unacceptable error"),
},
Some(translations_str) => {
// All good, turn the translations into a translator
self.get_translator_for_translations_str(locale, &translations_str)?
}
// If we get a 404 for a supported locale, that's an exception
None => {
return Err(ClientError::ValidLocaleNotProvided {
locale: locale.to_string(),
})
}
};
// This caches and returns the translator
Ok(self.cache_translator(translator))
Expand Down
41 changes: 39 additions & 2 deletions packages/perseus/src/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use std::marker::PhantomData;
use std::pin::Pin;
#[cfg(not(target_arch = "wasm32"))]
use std::sync::Arc;
use std::{collections::HashMap, rc::Rc};
use std::{collections::HashMap, panic::PanicInfo, rc::Rc};
use sycamore::prelude::Scope;
use sycamore::utils::hydrate::with_no_hydration_context;
use sycamore::{
Expand Down Expand Up @@ -104,7 +104,6 @@ where
/// The options for constructing a Perseus app. This `struct` will tie
/// together all your code, declaring to Perseus where your templates,
/// error pages, static content, etc. are.
#[derive(Debug)]
pub struct PerseusAppBase<G: Html, M: MutableStore, T: TranslationsManager> {
/// The HTML ID of the root `<div>` element into which Perseus will be
/// injected.
Expand Down Expand Up @@ -154,6 +153,10 @@ pub struct PerseusAppBase<G: Html, M: MutableStore, T: TranslationsManager> {
/// here will only be used if it exists.
#[cfg(not(target_arch = "wasm32"))]
static_dir: String,
/// A handler for panics on the client-side. This could create an arbitrary
/// message for the user, or do anything else.
#[cfg(target_arch = "wasm32")]
panic_handler: Option<Box<dyn Fn(&PanicInfo) + Send + Sync + 'static>>,
// We need this on the client-side to account for the unused type parameters
#[cfg(target_arch = "wasm32")]
_marker: PhantomData<(M, T)>,
Expand Down Expand Up @@ -307,6 +310,8 @@ impl<G: Html, M: MutableStore, T: TranslationsManager> PerseusAppBase<G, M, T> {
#[cfg(not(target_arch = "wasm32"))]
static_dir: "./static".to_string(),
#[cfg(target_arch = "wasm32")]
panic_handler: None,
#[cfg(target_arch = "wasm32")]
_marker: PhantomData,
}
}
Expand Down Expand Up @@ -335,6 +340,7 @@ impl<G: Html, M: MutableStore, T: TranslationsManager> PerseusAppBase<G, M, T> {
plugins: Rc::new(Plugins::new()),
// Many users won't need anything fancy in the index view, so we provide a default
index_view: DFLT_INDEX_VIEW.to_string(),
panic_handler: None,
_marker: PhantomData,
}
}
Expand Down Expand Up @@ -562,6 +568,29 @@ impl<G: Html, M: MutableStore, T: TranslationsManager> PerseusAppBase<G, M, T> {
self.pss_max_size = val;
self
}
/// Sets the browser-side panic handler for your app. This is a function
/// that will be executed if your app panics (which should never be caused
/// by Perseus unless something is seriously wrong, it's much more likely
/// to come from your code, or third-party code). If this happens, your page
/// would become totally uninteractive, with no warning to the user, since
/// Wasm will simply abort. In such cases, it is strongly recommended to
/// generate a warning message that notifies the user.
///
/// Note that there is no access within this function to Sycamore, page
/// state, global state, or translators. Assume that your code has
/// completely imploded when you write this function.
///
/// This has no default value.
#[allow(unused_variables)]
#[allow(unused_mut)]
pub fn panic_handler(mut self, val: impl Fn(&PanicInfo) + Send + Sync + 'static) -> Self {
#[cfg(target_arch = "wasm32")]
{
self.panic_handler = Some(Box::new(val));
}
self
}

// Getters
/// Gets the HTML ID of the `<div>` at which to insert Perseus.
pub fn get_root(&self) -> String {
Expand Down Expand Up @@ -873,6 +902,14 @@ impl<G: Html, M: MutableStore, T: TranslationsManager> PerseusAppBase<G, M, T> {

scoped_static_aliases
}
/// Takes the user-set panic handler out and returns it as an owned value,
/// allowing it to be used as an actual panic hook.
#[cfg(target_arch = "wasm32")]
pub fn take_panic_handler(
&mut self,
) -> Option<Box<dyn Fn(&PanicInfo) + Send + Sync + 'static>> {
self.panic_handler.take()
}
}

/// The component that represents the entrypoint at which Perseus will inject
Expand Down
44 changes: 38 additions & 6 deletions packages/perseus/src/router/get_initial_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,42 @@ pub(crate) fn get_initial_view(
path: path_with_locale.clone(),
});
return InitialView::View(match &err {
// These errors happen because we couldn't get a translator, so they certainly don't get one
ClientError::FetchError(FetchError::NotOk { url, status, .. }) => error_pages.get_view_and_render_head(cx, url, *status, &fmt_err(&err), None),
ClientError::FetchError(FetchError::SerFailed { url, .. }) => error_pages.get_view_and_render_head(cx, url, 500, &fmt_err(&err), None),
ClientError::LocaleNotSupported { .. } => error_pages.get_view_and_render_head(cx, &format!("/{}/...", locale), 404, &fmt_err(&err), None),
// No other errors should be returned
_ => panic!("expected 'AssetNotOk'/'AssetSerFailed'/'LocaleNotSupported' error, found other unacceptable error")
// These errors happen because we couldn't get a translator, so they
// certainly don't get one
ClientError::FetchError(FetchError::NotOk {
url, status, ..
}) => error_pages.get_view_and_render_head(
cx,
url,
*status,
&fmt_err(&err),
None,
),
ClientError::FetchError(FetchError::SerFailed { url, .. }) => {
error_pages.get_view_and_render_head(
cx,
url,
500,
&fmt_err(&err),
None,
)
}
ClientError::LocaleNotSupported { .. } => error_pages
.get_view_and_render_head(
cx,
&format!("/{}/...", locale),
404,
&fmt_err(&err),
None,
),
// No other errors should be returned, but we'll give any a 400
_ => error_pages.get_view_and_render_head(
cx,
&format!("/{}/...", locale),
400,
&fmt_err(&err),
None,
),
});
}
};
Expand Down Expand Up @@ -300,6 +330,8 @@ fn get_translations() -> Option<String> {
fn get_head() -> String {
let document = web_sys::window().unwrap().document().unwrap();
// Get the current head
// The server sends through a head, so we can guarantee that one is present (and
// it's mandated for custom initial views)
let head_node = document.query_selector("head").unwrap().unwrap();
// Get all the elements after the head boundary (otherwise we'd be duplicating
// the initial stuff)
Expand Down
60 changes: 49 additions & 11 deletions packages/perseus/src/router/get_subsequent_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ pub(crate) struct GetSubsequentViewProps<'a> {
/// Note that this will automatically update the router state just before it
/// returns, meaning that any errors that may occur after this function has been
/// called need to reset the router state to be an error.
// TODO Eliminate all panics in this function
pub(crate) async fn get_subsequent_view(
GetSubsequentViewProps {
cx,
Expand Down Expand Up @@ -108,12 +107,18 @@ pub(crate) async fn get_subsequent_view(
pss.add_head(&path, page_data.head.to_string());
Ok(page_data)
}
// If the page failed to serialize, an exception has occurred
// If the page failed to serialize, it's a server error
Err(err) => {
router_state.set_load_state(RouterLoadState::ErrorLoaded {
path: path_with_locale.clone(),
});
panic!("page data couldn't be serialized: '{}'", err)
Err(error_pages.get_view_and_render_head(
cx,
&asset_url,
500,
&fmt_err(&err),
None,
))
}
}
}
Expand Down Expand Up @@ -146,8 +151,14 @@ pub(crate) async fn get_subsequent_view(
None,
))
}
// No other errors should be returned
_ => panic!("expected 'AssetNotOk' error, found other unacceptable error"),
// No other errors should be returned, but we'll give any a 400
_ => Err(error_pages.get_view_and_render_head(
cx,
&asset_url,
400,
&fmt_err(&err),
None,
)),
}
}
}
Expand Down Expand Up @@ -194,12 +205,39 @@ pub(crate) async fn get_subsequent_view(
path: path_with_locale.clone(),
});
match &err {
// These errors happen because we couldn't get a translator, so they certainly don't get one
ClientError::FetchError(FetchError::NotOk { url, status, .. }) => return error_pages.get_view_and_render_head(cx, url, *status, &fmt_err(&err), None),
ClientError::FetchError(FetchError::SerFailed { url, .. }) => return error_pages.get_view_and_render_head(cx, url, 500, &fmt_err(&err), None),
ClientError::LocaleNotSupported { locale } => return error_pages.get_view_and_render_head(cx, &format!("/{}/...", locale), 404, &fmt_err(&err), None),
// No other errors should be returned
_ => panic!("expected 'AssetNotOk'/'AssetSerFailed'/'LocaleNotSupported' error, found other unacceptable error")
// These errors happen because we couldn't get a translator, so they certainly don't
// get one
ClientError::FetchError(FetchError::NotOk { url, status, .. }) => {
return error_pages.get_view_and_render_head(
cx,
url,
*status,
&fmt_err(&err),
None,
)
}
ClientError::FetchError(FetchError::SerFailed { url, .. }) => {
return error_pages.get_view_and_render_head(cx, url, 500, &fmt_err(&err), None)
}
ClientError::LocaleNotSupported { locale } => {
return error_pages.get_view_and_render_head(
cx,
&format!("/{}/...", locale),
404,
&fmt_err(&err),
None,
)
}
// No other errors should be returned, but we'll give any a 400
_ => {
return error_pages.get_view_and_render_head(
cx,
&format!("/{}/...", locale),
400,
&fmt_err(&err),
None,
)
}
}
}
};
Expand Down

0 comments on commit c232aed

Please sign in to comment.