From c55067ab7ca04540344a48d788b2ba9e7805e732 Mon Sep 17 00:00:00 2001 From: Greg Johnston Date: Fri, 23 Jun 2023 11:10:59 -0400 Subject: [PATCH] feat: improved error handling and version tracking for pending actions/`` (closes #1205) (#1225) --- leptos_server/src/action.rs | 46 +++- router/src/components/form.rs | 425 +++++++++++++++++++--------------- 2 files changed, 276 insertions(+), 195 deletions(-) diff --git a/leptos_server/src/action.rs b/leptos_server/src/action.rs index 285536c3d..36e6117a0 100644 --- a/leptos_server/src/action.rs +++ b/leptos_server/src/action.rs @@ -3,7 +3,7 @@ use leptos_reactive::{ create_rw_signal, signal_prelude::*, spawn_local, store_value, ReadSignal, RwSignal, Scope, StoredValue, }; -use std::{future::Future, pin::Pin, rc::Rc}; +use std::{cell::Cell, future::Future, pin::Pin, rc::Rc}; /// An action synchronizes an imperative `async` call to the synchronous reactive system. /// @@ -106,13 +106,30 @@ where self.0.with_value(|a| a.pending.read_only()) } - /// Updates whether the action is currently pending. + /// Updates whether the action is currently pending. If the action has been dispatched + /// multiple times, and some of them are still pending, it will *not* update the `pending` + /// signal. #[cfg_attr( any(debug_assertions, feature = "ssr"), tracing::instrument(level = "trace", skip_all,) )] pub fn set_pending(&self, pending: bool) { - self.0.try_with_value(|a| a.pending.set(pending)); + self.0.try_with_value(|a| { + let pending_dispatches = &a.pending_dispatches; + let still_pending = { + pending_dispatches.set(if pending { + pending_dispatches.get().wrapping_add(1) + } else { + pending_dispatches.get().saturating_sub(1) + }); + pending_dispatches.get() + }; + if still_pending == 0 { + a.pending.set(false); + } else { + a.pending.set(true); + } + }); } /// The URL associated with the action (typically as part of a server function.) @@ -186,6 +203,7 @@ where I: 'static, O: 'static, { + cx: Scope, /// How many times the action has successfully resolved. pub version: RwSignal, /// The current argument that was dispatched to the `async` function. @@ -195,6 +213,8 @@ where pub value: RwSignal>, pending: RwSignal, url: Option, + /// How many dispatched actions are still pending. + pending_dispatches: Rc>, #[allow(clippy::complexity)] action_fn: Rc Pin>>>, } @@ -215,14 +235,23 @@ where let input = self.input; let version = self.version; let pending = self.pending; + let pending_dispatches = Rc::clone(&self.pending_dispatches); let value = self.value; + let cx = self.cx; pending.set(true); + pending_dispatches.set(pending_dispatches.get().saturating_sub(1)); spawn_local(async move { let new_value = fut.await; - value.set(Some(new_value)); - input.set(None); - pending.set(false); - version.update(|n| *n += 1); + cx.batch(move || { + value.set(Some(new_value)); + input.set(None); + version.update(|n| *n += 1); + pending_dispatches + .set(pending_dispatches.get().saturating_sub(1)); + if pending_dispatches.get() == 0 { + pending.set(false); + } + }); }) } } @@ -316,6 +345,7 @@ where let input = create_rw_signal(cx, None); let value = create_rw_signal(cx, None); let pending = create_rw_signal(cx, false); + let pending_dispatches = Rc::new(Cell::new(0)); let action_fn = Rc::new(move |input: &I| { let fut = action_fn(input); Box::pin(fut) as Pin>> @@ -324,11 +354,13 @@ where Action(store_value( cx, ActionState { + cx, version, url: None, input, value, pending, + pending_dispatches, action_fn, }, )) diff --git a/router/src/components/form.rs b/router/src/components/form.rs index 7d39062df..1be6c628e 100644 --- a/router/src/components/form.rs +++ b/router/src/components/form.rs @@ -8,6 +8,7 @@ use web_sys::RequestRedirect; type OnFormData = Rc; type OnResponse = Rc; +type OnError = Rc; /// An HTML [`form`](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/form) progressively /// enhanced to use client-side routing. @@ -47,6 +48,9 @@ pub fn Form( /// to a form submission. #[prop(optional)] on_response: Option, + /// A callback will be called if the attempt to submit the form results in an error. + #[prop(optional)] + on_error: Option, /// A [`NodeRef`] in which the `
` element should be stored. #[prop(optional)] node_ref: Option>, @@ -68,196 +72,216 @@ where error: Option>>>, on_form_data: Option, on_response: Option, + on_error: Option, class: Option, children: Children, node_ref: Option>, attributes: Option>, ) -> HtmlElement { let action_version = version; - let on_submit = move |ev: web_sys::SubmitEvent| { - if ev.default_prevented() { - return; - } - let navigate = use_navigate(cx); + let on_submit = { + move |ev: web_sys::SubmitEvent| { + if ev.default_prevented() { + return; + } + let navigate = use_navigate(cx); - let (form, method, action, enctype) = extract_form_attributes(&ev); + let (form, method, action, enctype) = + extract_form_attributes(&ev); - let form_data = - web_sys::FormData::new_with_form(&form).unwrap_throw(); - if let Some(on_form_data) = on_form_data.clone() { - on_form_data(&form_data); - } - let params = - web_sys::UrlSearchParams::new_with_str_sequence_sequence( - &form_data, - ) - .unwrap_throw(); - let action = use_resolved_path(cx, move || action.clone()) - .get_untracked() - .unwrap_or_default(); - // multipart POST (setting Context-Type breaks the request) - if method == "post" && enctype == "multipart/form-data" { - ev.prevent_default(); - ev.stop_propagation(); - - let on_response = on_response.clone(); - spawn_local(async move { - let res = gloo_net::http::Request::post(&action) - .header("Accept", "application/json") - .redirect(RequestRedirect::Follow) - .body(form_data) - .send() - .await; - match res { - Err(e) => { - error!(" error while POSTing: {e:#?}"); - if let Some(error) = error { - error.set(Some(Box::new(e))); - } - } - Ok(resp) => { - if let Some(version) = action_version { - version.update(|n| *n += 1); - } - if let Some(error) = error { - error.set(None); - } - if let Some(on_response) = on_response.clone() { - on_response(resp.as_raw()); - } - // Check all the logical 3xx responses that might - // get returned from a server function - if resp.redirected() { - let resp_url = &resp.url(); - match Url::try_from(resp_url.as_str()) { - Ok(url) => { - if url.origin - != window() - .location() - .origin() - .unwrap_or_default() - { - _ = window() - .location() - .set_href(resp_url.as_str()); - } else { - request_animation_frame( - move || { - if let Err(e) = navigate( - &format!( - "{}{}{}", - url.pathname, - if url - .search - .is_empty() - { - "" - } else { - "?" - }, - url.search, - ), - Default::default(), - ) { - warn!("{}", e); - } - }, - ); - } - } - Err(e) => warn!("{}", e), - } - } - } - } - }); - } - // POST - else if method == "post" { - ev.prevent_default(); - ev.stop_propagation(); - - let on_response = on_response.clone(); - spawn_local(async move { - let res = gloo_net::http::Request::post(&action) - .header("Accept", "application/json") - .header("Content-Type", &enctype) - .redirect(RequestRedirect::Follow) - .body(params) - .send() - .await; - match res { - Err(e) => { - error!(" error while POSTing: {e:#?}"); - if let Some(error) = error { - error.set(Some(Box::new(e))); - } - } - Ok(resp) => { - if let Some(version) = action_version { - version.update(|n| *n += 1); - } - if let Some(error) = error { - error.set(None); - } - if let Some(on_response) = on_response.clone() { - on_response(resp.as_raw()); - } - // Check all the logical 3xx responses that might - // get returned from a server function - if resp.redirected() { - let resp_url = &resp.url(); - match Url::try_from(resp_url.as_str()) { - Ok(url) => { - if url.origin - != window() - .location() - .hostname() - .unwrap_or_default() - { - _ = window() - .location() - .set_href(resp_url.as_str()); - } else { - request_animation_frame( - move || { - if let Err(e) = navigate( - &format!( - "{}{}{}", - url.pathname, - if url - .search - .is_empty() - { - "" - } else { - "?" - }, - url.search, - ), - Default::default(), - ) { - warn!("{}", e); - } - }, - ); - } - } - Err(e) => warn!("{}", e), - } - } - } - } - }); - } - // otherwise, GET - else { - let params = params.to_string().as_string().unwrap_or_default(); - if navigate(&format!("{action}?{params}"), Default::default()) - .is_ok() - { + let form_data = + web_sys::FormData::new_with_form(&form).unwrap_throw(); + if let Some(on_form_data) = on_form_data.clone() { + on_form_data(&form_data); + } + let params = + web_sys::UrlSearchParams::new_with_str_sequence_sequence( + &form_data, + ) + .unwrap_throw(); + let action = use_resolved_path(cx, move || action.clone()) + .get_untracked() + .unwrap_or_default(); + // multipart POST (setting Context-Type breaks the request) + if method == "post" && enctype == "multipart/form-data" { ev.prevent_default(); ev.stop_propagation(); + + let on_response = on_response.clone(); + let on_error = on_error.clone(); + spawn_local(async move { + let res = gloo_net::http::Request::post(&action) + .header("Accept", "application/json") + .redirect(RequestRedirect::Follow) + .body(form_data) + .send() + .await; + match res { + Err(e) => { + error!(" error while POSTing: {e:#?}"); + if let Some(on_error) = on_error { + on_error(&e); + } + if let Some(error) = error { + error.try_set(Some(Box::new(e))); + } + } + Ok(resp) => { + if let Some(version) = action_version { + version.update(|n| *n += 1); + } + if let Some(error) = error { + error.try_set(None); + } + if let Some(on_response) = on_response.clone() { + on_response(resp.as_raw()); + } + // Check all the logical 3xx responses that might + // get returned from a server function + if resp.redirected() { + let resp_url = &resp.url(); + match Url::try_from(resp_url.as_str()) { + Ok(url) => { + if url.origin + != window() + .location() + .origin() + .unwrap_or_default() + { + _ = window() + .location() + .set_href( + resp_url.as_str(), + ); + } else { + request_animation_frame( + move || { + if let Err(e) = navigate( + &format!( + "{}{}{}", + url.pathname, + if url + .search + .is_empty() + { + "" + } else { + "?" + }, + url.search, + ), + Default::default(), + ) { + warn!("{}", e); + } + }, + ); + } + } + Err(e) => warn!("{}", e), + } + } + } + } + }); + } + // POST + else if method == "post" { + ev.prevent_default(); + ev.stop_propagation(); + + let on_response = on_response.clone(); + let on_error = on_error.clone(); + spawn_local(async move { + let res = gloo_net::http::Request::post(&action) + .header("Accept", "application/json") + .header("Content-Type", &enctype) + .redirect(RequestRedirect::Follow) + .body(params) + .send() + .await; + match res { + Err(e) => { + error!(" error while POSTing: {e:#?}"); + if let Some(on_error) = on_error { + on_error(&e); + } + if let Some(error) = error { + error.try_set(Some(Box::new(e))); + } + } + Ok(resp) => { + if let Some(version) = action_version { + version.update(|n| *n += 1); + } + if let Some(error) = error { + error.try_set(None); + } + if let Some(on_response) = on_response.clone() { + on_response(resp.as_raw()); + } + // Check all the logical 3xx responses that might + // get returned from a server function + if resp.redirected() { + let resp_url = &resp.url(); + match Url::try_from(resp_url.as_str()) { + Ok(url) => { + if url.origin + != window() + .location() + .hostname() + .unwrap_or_default() + { + _ = window() + .location() + .set_href( + resp_url.as_str(), + ); + } else { + request_animation_frame( + move || { + if let Err(e) = navigate( + &format!( + "{}{}{}", + url.pathname, + if url + .search + .is_empty() + { + "" + } else { + "?" + }, + url.search, + ), + Default::default(), + ) { + warn!("{}", e); + } + }, + ); + } + } + Err(e) => warn!("{}", e), + } + } + } + } + }); + } + // otherwise, GET + else { + let params = + params.to_string().as_string().unwrap_or_default(); + if navigate( + &format!("{action}?{params}"), + Default::default(), + ) + .is_ok() + { + ev.prevent_default(); + ev.stop_propagation(); + } } } }; @@ -296,6 +320,7 @@ where error, on_form_data, on_response, + on_error, class, children, node_ref, @@ -355,14 +380,36 @@ where let value = action.value(); let input = action.input(); + let on_error = Rc::new(move |e: &gloo_net::Error| { + cx.batch(move || { + action.set_pending(false); + let e = ServerFnError::Request(e.to_string()); + value.try_set(Some(Err(e.clone()))); + if let Some(error) = error { + error.try_set(Some(Box::new(e))); + } + }); + }); + let on_form_data = Rc::new(move |form_data: &web_sys::FormData| { let data = I::from_form_data(form_data); match data { Ok(data) => { - input.set(Some(data)); - action.set_pending(true); + cx.batch(move || { + input.try_set(Some(data)); + action.set_pending(true); + }); + } + Err(e) => { + error!("{e}"); + let e = ServerFnError::Serialization(e.to_string()); + cx.batch(move || { + value.try_set(Some(Err(e.clone()))); + if let Some(error) = error { + error.try_set(Some(Box::new(e))); + } + }); } - Err(e) => error!("{e}"), } }); @@ -370,7 +417,6 @@ where let resp = resp.clone().expect("couldn't get Response"); spawn_local(async move { let redirected = resp.redirected(); - if !redirected { let body = JsFuture::from( resp.text().expect("couldn't get .text() from Response"), @@ -434,8 +480,10 @@ where } }; } - input.try_set(None); - action.set_pending(false); + cx.batch(move || { + input.try_set(None); + action.set_pending(false); + }); }); }); let class = class.map(|bx| bx.into_attribute_boxed(cx)); @@ -444,6 +492,7 @@ where .version(version) .on_form_data(on_form_data) .on_response(on_response) + .on_error(on_error) .method("post") .class(class) .children(children) @@ -507,7 +556,7 @@ where Err(e) => { error!("{e}"); if let Some(error) = error { - error.set(Some(Box::new(e))); + error.try_set(Some(Box::new(e))); } } Ok(input) => { @@ -515,7 +564,7 @@ where ev.stop_propagation(); multi_action.dispatch(input); if let Some(error) = error { - error.set(None); + error.try_set(None); } } }