From 45afa18846556c3486e6922108ed6e8fcf6ec125 Mon Sep 17 00:00:00 2001
From: Vincent Ambo <mail@tazj.in>
Date: Wed, 26 Sep 2018 23:19:34 +0200
Subject: [PATCH] feat(postgres): Compatibility with new associated error type
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Changes the implementation of action execution to deal with the
returned associated errors.

The only sensible constraint on those errors that I could think of for
now is `Debug`, meaning that errors are now persisted as debug
messages.

This is not as nice to work with for a future implementation of
retryable actions as the equivalent in Haskell, but maybe an idea
shows up underway. The main issue is that most of the common error
types will not be implementing Serde traits, so serialization to/from
the same error type is difficult.

Adding an implementation constraint for JSON serialisation on error
types (i.e. `S::Error: Serialize + Deserialize`) would probably cause
headaches for users, especially if they are trying to use an
out-of-the-box error type or an error type wrapping foreign errors.

Det ska'kke være lett ...
---
 .../up.sql                                    |  2 +-
 finito-postgres/src/lib.rs                    | 33 +++++++++++++------
 finito-postgres/src/tests.rs                  |  2 +-
 3 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/finito-postgres/migrations/2018-09-26-160621_bootstrap_finito_schema/up.sql b/finito-postgres/migrations/2018-09-26-160621_bootstrap_finito_schema/up.sql
index 0de1a9e3c..18ace393b 100644
--- a/finito-postgres/migrations/2018-09-26-160621_bootstrap_finito_schema/up.sql
+++ b/finito-postgres/migrations/2018-09-26-160621_bootstrap_finito_schema/up.sql
@@ -30,7 +30,7 @@ CREATE TABLE actions (
   event_id UUID NOT NULL REFERENCES events(id),
   content JSONB NOT NULL,
   status ActionStatus NOT NULL,
-  error JSONB
+  error TEXT
 );
 
 CREATE INDEX idx_actions_machines ON actions(fsm_id);
diff --git a/finito-postgres/src/lib.rs b/finito-postgres/src/lib.rs
index af8013142..ffb2532e5 100644
--- a/finito-postgres/src/lib.rs
+++ b/finito-postgres/src/lib.rs
@@ -110,10 +110,9 @@ struct ActionT {
     /// Current status of the action.
     status: ActionStatus,
 
-    /// Serialised error representation, if an error occured during
-    /// processing. TODO: Use some actual error type. Maybe failure
-    /// has serialisation support?
-    error: Option<Value>,
+    /// Detailed (i.e. Debug-trait formatted) error message, if an
+    /// error occured during action processing.
+    error: Option<String>,
 }
 
 // The following functions implement the public interface of
@@ -292,7 +291,7 @@ fn get_action<C, S>(conn: &C, id: Uuid) -> Result<(ActionStatus, S::Action)> whe
 fn update_action_status<C, S>(conn: &C,
                               id: Uuid,
                               status: ActionStatus,
-                              error: Option<Value>,
+                              error: Option<String>,
                               _fsm: PhantomData<S>) -> Result<()> where
     C: GenericConnection,
     S: FSM {
@@ -371,12 +370,26 @@ fn run_action<S>(tx: Transaction, id: Uuid, _fsm: PhantomData<S>)
 
     let result = match status {
         ActionStatus::Pending => {
-            let events = <S as FSM>::act(action);
-            update_action_status(
-                &tx, id, ActionStatus::Completed, None, PhantomData::<S>
-            )?;
+            match <S as FSM>::act(action) {
+                // If the action succeeded, update its status to
+                // completed and return the created events.
+                Ok(events) => {
+                    update_action_status(
+                        &tx, id, ActionStatus::Completed, None, PhantomData::<S>
+                    )?;
+                    events
+                },
 
-            events
+                // If the action failed, persist the debug message and
+                // return nothing.
+                Err(err) => {
+                    let msg = Some(format!("{:?}", err));
+                    update_action_status(
+                        &tx, id, ActionStatus::Failed, msg, PhantomData::<S>
+                    )?;
+                    vec![]
+                },
+            }
         },
 
         _ => {
diff --git a/finito-postgres/src/tests.rs b/finito-postgres/src/tests.rs
index 55ada1329..b1b5821be 100644
--- a/finito-postgres/src/tests.rs
+++ b/finito-postgres/src/tests.rs
@@ -1,5 +1,5 @@
 use super::*;
-use finito;
+
 use finito_door::*;
 use postgres::{Connection, TlsMode};