Skip to content

Commit

Permalink
[SDK] Expose reducer error messages as a field in Status::Failed.
Browse files Browse the repository at this point in the history
Prior to this commit, the error messages returned by reducers
were unavailable to clients; `Status::Failed` was a singleton.
With this commit, `Status::Failed` holds a `String`,
the string representation of the `Err` returned by the reducer.

This means that `Status` is no longer `Copy`,
and must be passed by reference to callbacks instead of by value.
  • Loading branch information
gefjon committed Jul 20, 2023
1 parent 92743d2 commit 375764d
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 18 deletions.
4 changes: 2 additions & 2 deletions crates/cli/src/subcommands/generate/rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,7 @@ pub fn autogen_rust_reducer(ctx: &GenCtx, reducer: &ReducerDef) -> String {
writeln!(out, "{}", ALLOW_UNUSED).unwrap();
write!(
out,
"pub fn on_{}(mut __callback: impl FnMut(&Identity, Status",
"pub fn on_{}(mut __callback: impl FnMut(&Identity, &Status",
func_name
)
.unwrap();
Expand Down Expand Up @@ -679,7 +679,7 @@ pub fn autogen_rust_reducer(ctx: &GenCtx, reducer: &ReducerDef) -> String {
writeln!(out, "{}", ALLOW_UNUSED).unwrap();
write!(
out,
"pub fn once_on_{}(__callback: impl FnOnce(&Identity, Status",
"pub fn once_on_{}(__callback: impl FnOnce(&Identity, &Status",
func_name
)
.unwrap();
Expand Down
26 changes: 14 additions & 12 deletions crates/sdk/src/callbacks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,7 @@ impl<Args> std::fmt::Debug for CallbackId<Args> {
// - `Credentials` -> `&Credentials`, for `on_connect`.
// - `()` -> `()`, for `on_subscription_applied`.
// - `(T, T, U)` -> `(&T, &T, U)`, for `TableWithPrimaryKey::on_update`.
// - `(Identity, Status, R)` -> `(&Identity, Status, &R)`, for `Reducer::on_reducer`.
// Note that `Status` is `Copy`.
// - `(Identity, Status, R)` -> `(&Identity, &Status, &R)`, for `Reducer::on_reducer`.

impl OwnedArgs for Credentials {
type Borrowed<'a> = &'a Credentials;
Expand Down Expand Up @@ -182,9 +181,9 @@ impl<R> OwnedArgs for (Identity, Status, R)
where
R: Send + 'static,
{
type Borrowed<'a> = (&'a Identity, Status, &'a R);
fn borrow(&self) -> (&Identity, Status, &R) {
(&self.0, self.1, &self.2)
type Borrowed<'a> = (&'a Identity, &'a Status, &'a R);
fn borrow(&self) -> (&Identity, &Status, &R) {
(&self.0, &self.1, &self.2)
}
}

Expand Down Expand Up @@ -450,8 +449,8 @@ fn uncurry_update_callback<Row, ReducerEvent>(
///
/// This function is intended specifically for `Reducer::on_reducer` callbacks.
fn uncurry_reducer_callback<R>(
mut f: impl for<'a> FnMut(&'a Identity, Status, &'a R) + Send + 'static,
) -> impl for<'a> FnMut((&'a Identity, Status, &'a R)) + Send + 'static {
mut f: impl for<'a> FnMut(&'a Identity, &'a Status, &'a R) + Send + 'static,
) -> impl for<'a> FnMut((&'a Identity, &'a Status, &'a R)) + Send + 'static {
move |(identity, status, reducer)| f(identity, status, reducer)
}

Expand Down Expand Up @@ -659,12 +658,14 @@ pub struct ReducerCallbacks {
// which we must then compare against the enum variants.
// This helper function does that comparison.

fn parse_status(status: i32) -> Option<Status> {
fn parse_status(status: i32, message: String) -> Option<Status> {
if status == client_api_messages::event::Status::Committed as i32 {
debug_assert!(message.is_empty());
Some(Status::Committed)
} else if status == client_api_messages::event::Status::Failed as i32 {
Some(Status::Failed)
Some(Status::Failed(message))
} else if status == client_api_messages::event::Status::OutOfEnergy as i32 {
debug_assert!(message.is_empty());
Some(Status::OutOfEnergy)
} else {
None
Expand Down Expand Up @@ -707,13 +708,14 @@ impl ReducerCallbacks {
caller_identity,
function_call: Some(function_call),
status,
message,
..
} = event else {
log::warn!("Received Event with function_call of None");
return None;
};
let identity = Identity { bytes: caller_identity };
let Some(status) = parse_status(status) else {
let Some(status) = parse_status(status, message) else {
log::warn!("Received Event with unknown status {:?}", status);
return None;
};
Expand All @@ -736,7 +738,7 @@ impl ReducerCallbacks {
// TODO: reduce monomorphization by accepting `Box<Callback>` instead of `impl Callback`
pub(crate) fn register_on_reducer<R: Reducer>(
&mut self,
callback: impl FnMut(&Identity, Status, &R) + Send + 'static,
callback: impl FnMut(&Identity, &Status, &R) + Send + 'static,
) -> CallbackId<(Identity, Status, R)> {
self.find_callbacks::<R>()
.insert(Box::new(uncurry_reducer_callback(callback)))
Expand All @@ -750,7 +752,7 @@ impl ReducerCallbacks {
// since [`CallbackMap::insert_oneshot`] boxes its wrapper callback.
pub(crate) fn register_on_reducer_oneshot<R: Reducer>(
&mut self,
callback: impl FnOnce(&Identity, Status, &R) + Send + 'static,
callback: impl FnOnce(&Identity, &Status, &R) + Send + 'static,
) -> CallbackId<(Identity, Status, R)> {
self.find_callbacks::<R>()
.insert_oneshot(move |(identity, status, args)| callback(identity, status, args))
Expand Down
8 changes: 4 additions & 4 deletions crates/sdk/src/reducer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ use anyhow::Result;
use spacetimedb_sats::{de::DeserializeOwned, ser::Serialize};
use std::any::Any;

#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
#[derive(Clone, PartialEq, Eq, Hash, Debug)]
pub enum Status {
Committed,
Failed,
Failed(String),
OutOfEnergy,
}

Expand Down Expand Up @@ -39,7 +39,7 @@ pub trait Reducer: DeserializeOwned + Serialize + Any + Send + Sync + Clone {
//
/// The returned `ReducerCallbackId` can be passed to `remove_on_reducer` to
/// unregister the callback.
fn on_reducer(callback: impl FnMut(&Identity, Status, &Self) + Send + 'static) -> ReducerCallbackId<Self> {
fn on_reducer(callback: impl FnMut(&Identity, &Status, &Self) + Send + 'static) -> ReducerCallbackId<Self> {
let id = with_reducer_callbacks(|callbacks| callbacks.register_on_reducer::<Self>(callback));
ReducerCallbackId { id }
}
Expand All @@ -49,7 +49,7 @@ pub trait Reducer: DeserializeOwned + Serialize + Any + Send + Sync + Clone {
/// The `callback` will run at most once, then unregister itself.
/// It can also be unregistered by passing the returned `ReducerCallbackId`
/// to `remove_on_reducer`.
fn once_on_reducer(callback: impl FnOnce(&Identity, Status, &Self) + Send + 'static) -> ReducerCallbackId<Self> {
fn once_on_reducer(callback: impl FnOnce(&Identity, &Status, &Self) + Send + 'static) -> ReducerCallbackId<Self> {
let id = with_reducer_callbacks(|callbacks| callbacks.register_on_reducer_oneshot::<Self>(callback));
ReducerCallbackId { id }
}
Expand Down

0 comments on commit 375764d

Please sign in to comment.