Repository: boardgameio/boardgame.io
Pull request #940 (2021-05-18 21:52:39)

feature(#723): Improve client error handling [Part 1]
This PR sets up the initial reducer/master plumbing for error handling
by introducing a "transients" option bag containing errors.

The goal of this change is to be as backwards-compatible as possible,
with subsequent changes building on top of the API extensions added
here.

Notes:

- Previously, there was an assumption that the state resulting from a
reducer processing an action is suitable for client consumption.
- Now, transient artifacts might be appended onto the state and need to
be stripped before being sent to the client.
- To work around this change, I added some new types to help signal
the "transient" nature of State that enters and exits the reducer.
- I'm assuming the master is the single caller of the core reducer and
leaving it up to the master to do this stripping..

#### Checklist

* [x] Use a separate branch in your local repo (not `master`).
* [x] Test coverage is 100% (or you have a story for why it's ok).


shaoster: > Thanks @shaoster! Good job separating out this chunk of the work. Here are a few initial comments.
> 
> Regarding the general architecture, it’s a little more complicated than reducer → master → client. If you search for `CreateGameReducer` in the repo, you’ll find all the places it’s used, but here’s a quick overview:
> 
> 1. The reducer runs [directly inside the client](https://github.com/boardgameio/boardgame.io/blob/dc6cd451d85b4cb8a857d21af06c66e68a217815/src/client/client.ts#L314), this allows the client to run without a server/local master and allows the client to update optimistically even before receiving an update from the server when running a multiplayer game.
> 2. When in use, the reducer runs in the master either on the server or inside `Local` (this is the scenario you already considered).
> 3. AI code [uses the reducer](https://github.com/boardgameio/boardgame.io/blob/dc6cd451d85b4cb8a857d21af06c66e68a217815/src/ai/ai.ts#L61) to run simulations. (The same is true in [the MCTS bot implementation](https://github.com/boardgameio/boardgame.io/blob/dc6cd451d85b4cb8a857d21af06c66e68a217815/src/ai/mcts-bot.ts#L88).)
> 4. Finally, we also currently run the reducer when stepping through items [in the Log pane of the debug panel](https://github.com/boardgameio/boardgame.io/blob/dc6cd451d85b4cb8a857d21af06c66e68a217815/src/client/debug/log/Log.svelte#L14).
> 
> #### Implications
> * In the client we need to consider [the middleware added to the store](https://github.com/boardgameio/boardgame.io/blob/dc6cd451d85b4cb8a857d21af06c66e68a217815/src/client/client.ts#L219-L314) to see how exactly to handle the transient state and later surface `transients.error` when produced by the local reducer. As elsewhere, adding `transients` doen’t cause any actual errors, but presumably we’d want to strip it as in the master and use the error somehow.
> * 3 & 4 can perhaps wait — adding the `transients` field shouldn’t actually cause problems here. It may later be possible to use `transients.error` to improve the logic in AI simulations and log stepping (this latter could perhaps be done as part of #892)

The latest version has dealt with 1 & 2 as quoted above.

My implementation strategy of creating a new shared middleware between the client and master, however, suggests that a more fundamental unification of those code paths might be beneficial at some point. Having some kind of shared component that deals with all dispatches, and adding client/master specializations in an outer-most middleware might be easier to maintain.

Don't want to bite off more than I can chew though!
shaoster: Review feedback addressed. PTAL
delucis: @shaoster I’ve merged your fix PR. Let me know if you need a hand resolving the conflict with this branch.
shaoster: De-conflicted against #942 and local change for the additional error cases rebased on top.

Patch:
diff --git a/src/client/client.ts b/src/client/client.ts
index 521af20..5f07265 100644
--- a/src/client/client.ts
+++ b/src/client/client.ts
@@ -14,7 +14,10 @@ import * as Actions from '../core/action-types';
 import * as ActionCreators from '../core/action-creators';
 import { ProcessGameConfig } from '../core/game';
 import type Debug from './debug/Debug.svelte';
-import { CreateGameReducer } from '../core/reducer';
+import {
+  CreateGameReducer,
+  TransientHandlingMiddleware,
+} from '../core/reducer';
 import { InitializeGame } from '../core/initialize';
 import { PlayerView } from '../plugins/main';
 import type { Transport, TransportOpts } from './transport/transport';
@@ -303,6 +306,7 @@ export class _ClientImpl<G extends any = any> {
     };
 
     const middleware = applyMiddleware(
+      TransientHandlingMiddleware,
       SubscriptionMiddleware,
       TransportMiddleware,
       LogMiddleware
diff --git a/src/core/action-creators.ts b/src/core/action-creators.ts
index 1834118..48d479c 100644
--- a/src/core/action-creators.ts
+++ b/src/core/action-creators.ts
@@ -149,3 +149,11 @@ export const plugin = (
   type: Actions.PLUGIN as typeof Actions.PLUGIN,
   payload: { type, args, playerID, credentials },
 });
+
+/**
+ * Private action used to strip transient metadata (e.g. errors) from the game
+ * state.
+ */
+export const stripTransients = () => ({
+  type: Actions.STRIP_TRANSIENTS as typeof Actions.STRIP_TRANSIENTS,
+});
diff --git a/src/core/action-types.ts b/src/core/action-types.ts
index d282c39..dbc83a6 100644
--- a/src/core/action-types.ts
+++ b/src/core/action-types.ts
@@ -15,3 +15,4 @@ export const UNDO = 'UNDO';
 export const UPDATE = 'UPDATE';
 export const PATCH = 'PATCH';
 export const PLUGIN = 'PLUGIN';
+export const STRIP_TRANSIENTS = 'STRIP_TRANSIENTS';
diff --git a/src/core/errors.ts b/src/core/errors.ts
new file mode 100644
index 0000000..267c862
--- /dev/null
+++ b/src/core/errors.ts
@@ -0,0 +1,33 @@
+/*
+ * Copyright 2017 The boardgame.io Authors
+ *
+ * Use of this source code is governed by a MIT-style
+ * license that can be found in the LICENSE file or at
+ * https://opensource.org/licenses/MIT.
+ */
+
+export enum UpdateErrorType {
+  // The action’s credentials were missing or invalid
+  UnauthorizedAction = 'update/unauthorized_action',
+  // The action’s matchID was not found
+  MatchNotFound = 'update/match_not_found',
+  // Could not apply Patch operation (rfc6902).
+  PatchFailed = 'update/patch_failed',
+}
+
+export enum ActionErrorType {
+  // The action contained a stale state ID
+  StaleStateId = 'action/stale_state_id',
+  // The requested move is unknown or not currently available
+  UnavailableMove = 'action/unavailable_move',
+  // The move declared it was invalid (INVALID_MOVE constant)
+  InvalidMove = 'action/invalid_move',
+  // The player making the action is not currently active
+  InactivePlayer = 'action/inactive_player',
+  // The game has finished
+  GameOver = 'action/gameover',
+  // The requested action is disabled (e.g. undo/redo, events)
+  ActionDisabled = 'action/action_disabled',
+  // The requested action is not currently possible
+  ActionInvalid = 'action/action_invalid',
+}
diff --git a/src/core/reducer.ts b/src/core/reducer.ts
index b10ae54..beba12b 100644
--- a/src/core/reducer.ts
+++ b/src/core/reducer.ts
@@ -11,16 +11,23 @@ import * as plugins from '../plugins/main';
 import { ProcessGameConfig } from './game';
 import { error } from './logger';
 import { INVALID_MOVE } from './constants';
+import type { Dispatch } from 'redux';
 import type {
   ActionShape,
   Ctx,
+  ErrorType,
   Game,
   LogEntry,
-  State,
-  Move,
   LongFormMove,
+  Move,
+  State,
+  Store,
+  TransientMetadata,
+  TransientState,
   Undo,
 } from '../types';
+import { stripTransients } from './action-creators';
+import { ActionErrorType, UpdateErrorType } from './errors';
 import { applyPatch } from 'rfc6902';
 
 /**
@@ -97,7 +104,7 @@ function initializeDeltalog(
   state: State,
   action: ActionShape.MakeMove | ActionShape.Undo | ActionShape.Redo,
   move?: Move
-): State {
+): TransientState {
   // Create a log entry for this action.
   const logEntry: LogEntry = {
     action,
@@ -121,6 +128,79 @@ function initializeDeltalog(
   };
 }
 
+/**
+ * ExtractTransientsFromState
+ *
+ * Split out transients from the a TransientState
+ */
+function ExtractTransients(
+  transientState: TransientState | null
+): [State | null, TransientMetadata | undefined] {
+  if (!transientState) {
+    // We preserve null for the state for legacy callers, but the transient
+    // field should be undefined if not present to be consistent with the
+    // code path below.
+    return [null, undefined];
+  }
+  const { transients, ...state } = transientState;
+  return [state as State, transients as TransientMetadata];
+}
+
+/**
+ * WithError
+ *
+ * Augment a State instance with transient error information.
+ */
+function WithError<PT extends any = any>(
+  state: State,
+  errorType: ErrorType,
+  payload?: PT
+): TransientState {
+  const error = {
+    type: errorType,
+    payload,
+  };
+  return {
+    ...state,
+    transients: {
+      error,
+    },
+  };
+}
+
+/**
+ * Middleware for processing TransientState associated with the reducer
+ * returned by CreateGameReducer.
+ * This should pretty much be used everywhere you want realistic state
+ * transitions and error handling.
+ */
+export const TransientHandlingMiddleware = (store: Store) => (
+  next: Dispatch<ActionShape.Any>
+) => (action: ActionShape.Any) => {
+  const result = next(action);
+  switch (action.type) {
+    case Actions.STRIP_TRANSIENTS: {
+      return result;
+    }
+    default: {
+      const [, transients] = ExtractTransients(store.getState());
+      if (typeof transients !== 'undefined') {
+        store.dispatch(stripTransients());
+        // Dev Note: If parent middleware needs to correlate the spawned
+        // StripTransients action to the triggering action, instrument here.
+        //
+        // This is a bit tricky; for more details, see:
+        //   https://github.com/boardgameio/boardgame.io/pull/940#discussion_r636200648
+        return {
+          ...result,
+          transients,
+        };
+      }
+      return result;
+    }
+  }
+};
+
 /**
  * CreateGameReducer
  *
@@ -142,8 +222,18 @@ export function CreateGameReducer({
    * @param {object} state - The state before the action.
    * @param {object} action - A Redux action.
    */
-  return (state: State | null = null, action: ActionShape.Any): State => {
+  return (
+    stateWithTransients: TransientState | null = null,
+    action: ActionShape.Any
+  ): TransientState => {
+    let [state /*, transients */] = ExtractTransients(stateWithTransients);
     switch (action.type) {
+      case Actions.STRIP_TRANSIENTS: {
+        // This action indicates that transient metadata in the state has been
+        // consumed and should now be stripped from the state..
+        return state;
+      }
+
       case Actions.GAME_EVENT: {
         state = { ...state, deltalog: [] };
 
@@ -158,7 +248,7 @@ export function CreateGameReducer({
         // Disallow events once the game is over.
         if (state.ctx.gameover !== undefined) {
           error(`cannot call event after game end`);
-          return state;
+          return WithError(state, ActionErrorType.GameOver);
         }
 
         // Ignore the event if the player isn't active.
@@ -167,7 +257,7 @@ export function CreateGameReducer({
           !game.flow.isPlayerActive(state.G, state.ctx, action.payload.playerID)
         ) {
           error(`disallowed event: ${action.payload.type}`);
-          return state;
+          return WithError(state, ActionErrorType.InactivePlayer);
         }
 
         // Execute plugins.
@@ -200,7 +290,7 @@ export function CreateGameReducer({
         );
         if (move === null) {
           error(`disallowed move: ${action.payload.type}`);
-          return state;
+          return WithError(state, ActionErrorType.UnavailableMove);
         }
 
         // Don't run move on client if move says so.
@@ -211,7 +301,7 @@ export function CreateGameReducer({
         // Disallow moves once the game is over.
         if (state.ctx.gameover !== undefined) {
           error(`cannot make move after game end`);
-          return state;
+          return WithError(state, ActionErrorType.GameOver);
         }
 
         // Ignore the move if the player isn't active.
@@ -220,7 +310,7 @@ export function CreateGameReducer({
           !game.flow.isPlayerActive(state.G, state.ctx, action.payload.playerID)
         ) {
           error(`disallowed move: ${action.payload.type}`);
-          return state;
+          return WithError(state, ActionErrorType.InactivePlayer);
         }
 
         // Execute plugins.
@@ -238,7 +328,8 @@ export function CreateGameReducer({
           error(
             `invalid move: ${action.payload.type} args: ${action.payload.args}`
           );
-          return state;
+          // TODO(#723): Marshal a nice error payload with the processed move.
+          return WithError(state, ActionErrorType.InvalidMove);
         }
 
         const newState = { ...state, G };
@@ -294,13 +385,14 @@ export function CreateGameReducer({
 
         if (game.disableUndo) {
           error('Undo is not enabled');
-          return state;
+          return WithError(state, ActionErrorType.ActionDisabled);
         }
 
         const { _undo, _redo } = state;
 
         if (_undo.length < 2) {
-          return state;
+          error(`No moves to undo`);
+          return WithError(state, ActionErrorType.ActionInvalid);
         }
 
         const last = _undo[_undo.length - 1];
@@ -311,7 +403,8 @@ export function CreateGameReducer({
           actionHasPlayerID(action) &&
           action.payload.playerID !== last.playerID
         ) {
-          return state;
+          error(`Cannot undo other players' moves`);
+          return WithError(state, ActionErrorType.ActionInvalid);
         }
 
         // If undoing a move, check it is undoable.
@@ -322,7 +415,8 @@ export function CreateGameReducer({
             last.playerID
           );
           if (!CanUndoMove(state.G, state.ctx, lastMove)) {
-            return state;
+            error(`Move cannot be undone`);
+            return WithError(state, ActionErrorType.ActionInvalid);
           }
         }
 
@@ -344,13 +438,14 @@ export function CreateGameReducer({
 
         if (game.disableUndo) {
           error('Redo is not enabled');
-          return state;
+          return WithError(state, ActionErrorType.ActionDisabled);
         }
 
         const { _undo, _redo } = state;
 
         if (_redo.length == 0) {
-          return state;
+          error(`No moves to redo`);
+          return WithError(state, ActionErrorType.ActionInvalid);
         }
 
         const first = _redo[0];
@@ -360,7 +455,8 @@ export function CreateGameReducer({
           actionHasPlayerID(action) &&
           action.payload.playerID !== first.playerID
         ) {
-          return state;
+          error(`Cannot redo other players' moves`);
+          return WithError(state, ActionErrorType.ActionInvalid);
         }
 
         state = initializeDeltalog(state, action);
@@ -377,6 +473,7 @@ export function CreateGameReducer({
       }
 
       case Actions.PLUGIN: {
+        // TODO(#723): Expose error semantics to plugin processing.
         return plugins.ProcessAction(state, action, { game });
       }
 
@@ -387,7 +484,7 @@ export function CreateGameReducer({
         const hasError = patchError.some((entry) => entry !== null);
         if (hasError) {
           error(`Patch ${JSON.stringify(action.patch)} apply failed`);
-          return oldState;
+          return WithError(oldState, UpdateErrorType.PatchFailed, patchError);
         } else {
           return newState;
         }
diff --git a/src/master/master.ts b/src/master/master.ts
index 4a0534f..ddb5960 100644
--- a/src/master/master.ts
+++ b/src/master/master.ts
@@ -6,10 +6,13 @@
  * https://opensource.org/licenses/MIT.
  */
 
-import { CreateGameReducer } from '../core/reducer';
+import {
+  CreateGameReducer,
+  TransientHandlingMiddleware,
+} from '../core/reducer';
 import { ProcessGameConfig, IsLongFormMove } from '../core/game';
 import { UNDO, REDO, MAKE_MOVE } from '../core/action-types';
-import { createStore } from 'redux';
+import { createStore, applyMiddleware } from 'redux';
 import * as logging from '../core/logger';
 import { PlayerView } from '../plugins/main';
 import type {
@@ -202,7 +205,8 @@ export class Master {
     const reducer = CreateGameReducer({
       game: this.game,
     });
-    const store = createStore(reducer, state);
+    const middleware = applyMiddleware(TransientHandlingMiddleware);
+    const store = createStore(reducer, state, middleware);
 
     // Only allow UNDO / REDO if there is exactly one player
     // that can make moves right now and the person doing the
diff --git a/src/plugins/main.ts b/src/plugins/main.ts
index ddc3678..23bcc96 100644
--- a/src/plugins/main.ts
+++ b/src/plugins/main.ts
@@ -41,6 +41,7 @@ export const ProcessAction = (
   action: ActionShape.Plugin,
   opts: PluginOpts
 ): State => {
+  // TODO(#723): Extend error handling to plugins.
   opts.game.plugins
     .filter((plugin) => plugin.action !== undefined)
     .filter((plugin) => plugin.name === action.payload.type)
diff --git a/src/types.ts b/src/types.ts
index b0e37da..7c0b306 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -2,6 +2,7 @@ import type { Object } from 'ts-toolbelt';
 import type Koa from 'koa';
 import type { Store as ReduxStore } from 'redux';
 import type * as ActionCreators from './core/action-creators';
+import type { ActionErrorType, UpdateErrorType } from './core/errors';
 import type { Flow } from './core/flow';
 import type { CreateGameReducer } from './core/reducer';
 import type { INVALID_MOVE } from './core/constants';
@@ -15,6 +16,7 @@ export type { StorageAPI };
 
 export type AnyFn = (...args: any[]) => any;
 
+// "Public" state to be communicated to clients.
 export interface State<G extends any = any, CtxWithPlugins extends Ctx = Ctx> {
   G: G;
   ctx: Ctx | CtxWithPlugins;
@@ -27,6 +29,30 @@ export interface State<G extends any = any, CtxWithPlugins extends Ctx = Ctx> {
   _stateID: number;
 }
 
+export type ErrorType = UpdateErrorType | ActionErrorType;
+
+export interface ActionError {
+  type: ErrorType;
+  // TODO(#723): Figure out if we want to strongly type payloads.
+  payload?: any;
+}
+
+export interface TransientMetadata {
+  error?: ActionError;
+}
+
+// TODO(#732): Actually define a schema for the action dispatch results.
+export type ActionResult = any;
+
+// "Private" state that may include garbage that should be stripped before
+// being handed back to a client.
+export interface TransientState<
+  G extends any = any,
+  CtxWithPlugins extends Ctx = Ctx
+> extends State<G, CtxWithPlugins> {
+  transients?: TransientMetadata;
+}
+
 export type PartialGameState = Pick<State, 'G' | 'ctx' | 'plugins'>;
 
 export type StageName = string;
@@ -403,6 +429,9 @@ export namespace ActionShape {
   export type Reset = ReturnType<typeof ActionCreators.reset>;
   export type Undo = StripCredentials<CredentialedActionShape.Undo>;
   export type Redo = StripCredentials<CredentialedActionShape.Redo>;
+  // Private type used only for internal error processing.
+  // Included here to preserve type-checking of reducer inputs.
+  type _StripTransients = ReturnType<typeof ActionCreators.stripTransients>;
   export type Any =
     | MakeMove
     | GameEvent
@@ -413,7 +442,8 @@ export namespace ActionShape {
     | Reset
     | Undo
     | Redo
-    | Plugin;
+    | Plugin
+    | _StripTransients;
 }
 
 export namespace ActionPayload {


Test patch:
diff --git a/src/client/client.test.ts b/src/client/client.test.ts
index 9fca283..7c5b54e 100644
--- a/src/client/client.test.ts
+++ b/src/client/client.test.ts
@@ -6,6 +6,7 @@
  * https://opensource.org/licenses/MIT.
  */
 
+import { INVALID_MOVE } from '../core/constants';
 import { createStore } from 'redux';
 import { CreateGameReducer } from '../core/reducer';
 import { InitializeGame } from '../core/initialize';
@@ -498,6 +499,45 @@ describe('move dispatchers', () => {
   });
 });
 
+describe('transient handling', () => {
+  let client = null;
+
+  beforeEach(() => {
+    client = Client({
+      game: {
+        moves: {
+          A: () => ({}),
+          Invalid: () => INVALID_MOVE,
+        },
+      },
+    });
+  });
+
+  test('regular move', () => {
+    const result = client.moves.A();
+    // TODO(#723): Check against a successful ActionResult.
+    expect(result).toBeUndefined();
+    const state = client.store.getState();
+    // Slightly paranoid check to ensure we don't erroneously add transients.
+    expect(state).toEqual(
+      expect.not.objectContaining({ transients: expect.anything() })
+    );
+  });
+
+  test('invalid move', () => {
+    const result = client.moves.Invalid();
+    // TODO(#723): Check against an errored ActionResult.
+    expect(result).toBeUndefined();
+    const state = client.store.getState();
+    // Ensure we've stripped the transients automagically.
+    // At the time this test was written, this effectively ensures that Client
+    // hooks up the TransientHandlingMiddleware correctly.
+    expect(state).toEqual(
+      expect.not.objectContaining({ transients: expect.anything() })
+    );
+  });
+});
+
 describe('log handling', () => {
   let client = null;
 
@@ -769,20 +809,30 @@ test('override game state', () => {
   expect(client.getState().G).toEqual({ moved: true });
 });
 
+// TODO(#941): These tests should validate DOM mounting/unmounting.
 describe('start / stop', () => {
+  beforeEach(() => {
+    // Don't let other calls to `error` pollute this state.
+    jest.resetAllMocks();
+  });
+
   test('mount on custom element', () => {
     const el = document.createElement('div');
     const client = Client({ game: {}, debug: { target: el } });
-    client.start();
-    client.stop();
-    expect(error).toHaveBeenCalledTimes(3);
-    expect(error).toHaveBeenCalledWith('disallowed move: B');
-    expect(error).toHaveBeenCalledWith('disallowed move: C');
-    expect(error).toHaveBeenLastCalledWith('disallowed move: A');
+    expect(() => {
+      client.start();
+      client.stop();
+    }).not.toThrow();
+    expect(error).not.toHaveBeenCalled();
   });
 
   test('no error when mounting on null element', () => {
     const client = Client({ game: {}, debug: { target: null } }) as any;
+    expect(() => {
+      client.start();
+      client.stop();
+    }).not.toThrow();
+
     client.start();
     client.stop();
     expect(client.manager.debugPanel).toBe(null);
@@ -790,31 +840,31 @@ describe('start / stop', () => {
 
   test('override debug implementation', () => {
     const client = Client({ game: {}, debug: { impl: Debug } });
+    expect(() => {
+      client.start();
+      client.stop();
+    }).not.toThrow();
+
     client.start();
     client.stop();
-    expect(error).toHaveBeenCalledTimes(3);
-    expect(error).toHaveBeenCalledWith('disallowed move: B');
-    expect(error).toHaveBeenCalledWith('disallowed move: C');
-    expect(error).toHaveBeenLastCalledWith('disallowed move: A');
+    expect(error).not.toHaveBeenCalled();
   });
 
   test('production mode', () => {
     process.env.NODE_ENV = 'production';
     const client = Client({ game: {} });
-    client.start();
-    client.stop();
-    expect(error).toHaveBeenCalledTimes(3);
-    expect(error).toHaveBeenCalledWith('disallowed move: B');
-    expect(error).toHaveBeenCalledWith('disallowed move: C');
-    expect(error).toHaveBeenLastCalledWith('disallowed move: A');
+    expect(() => {
+      client.start();
+      client.stop();
+    }).not.toThrow();
+    expect(error).not.toHaveBeenCalled();
   });
 
   test('try to stop without starting', () => {
     const client = Client({ game: {} });
-    client.stop();
-    expect(error).toHaveBeenCalledTimes(3);
-    expect(error).toHaveBeenCalledWith('disallowed move: B');
-    expect(error).toHaveBeenCalledWith('disallowed move: C');
-    expect(error).toHaveBeenLastCalledWith('disallowed move: A');
+    expect(() => {
+      client.stop();
+    }).not.toThrow();
+    expect(error).not.toHaveBeenCalled();
   });
 });
diff --git a/src/core/reducer.test.ts b/src/core/reducer.test.ts
index 02d9461..0d2c839 100644
--- a/src/core/reducer.test.ts
+++ b/src/core/reducer.test.ts
@@ -7,7 +7,8 @@
  */
 
 import { INVALID_MOVE } from './constants';
-import { CreateGameReducer } from './reducer';
+import { applyMiddleware, createStore } from 'redux';
+import { CreateGameReducer, TransientHandlingMiddleware } from './reducer';
 import { InitializeGame } from './initialize';
 import {
   makeMove,
@@ -32,6 +33,7 @@ const game: Game = {
     A: (G) => G,
     B: () => ({ moved: true }),
     C: () => ({ victory: true }),
+    Invalid: () => INVALID_MOVE,
   },
   endIf: (G, ctx) => (G.victory ? ctx.currentPlayer : undefined),
 };
@@ -131,11 +133,16 @@ test('valid patch', () => {
 
 test('invalid patch', () => {
   const originalState = { _stateID: 0, G: 'patch' } as State;
-  const state = reducer(
+  const { transients, ...state } = reducer(
     originalState,
     patch(0, 1, [{ op: 'replace', path: '/_stateIDD', value: 1 }], [])
   );
   expect(state).toEqual(originalState);
+  expect(transients.error.type).toEqual('update/patch_failed');
+  // It's an array.
+  expect(transients.error.payload.length).toEqual(1);
+  // It looks like the standard rfc6902 error language.
+  expect(transients.error.payload[0].toString()).toContain('/_stateIDD');
 });
 
 test('reset', () => {
@@ -431,7 +438,15 @@ describe('undo / redo', () => {
 
   test('redo only resets deltalog if nothing to redo', () => {
     const state = reducer(initialState, makeMove('move', 'A', '0'));
-    expect(reducer(state, redo())).toEqual({ ...state, deltalog: [] });
+    expect(reducer(state, redo())).toMatchObject({
+      ...state,
+      deltalog: [],
+      transients: {
+        error: {
+          type: 'action/action_invalid',
+        },
+      },
+    });
   });
 });
 
@@ -526,13 +541,29 @@ describe('undo stack', () => {
 
   test('can’t undo at the start of a turn', () => {
     const newState = reducer(state, undo());
-    expect(newState).toEqual({ ...state, deltalog: [] });
+    expect(newState).toMatchObject({
+      ...state,
+      deltalog: [],
+      transients: {
+        error: {
+          type: 'action/action_invalid',
+        },
+      },
+    });
   });
 
   test('can’t undo another player’s move', () => {
     state = reducer(state, makeMove('basic', null, '1'));
     const newState = reducer(state, undo('0'));
-    expect(newState).toEqual({ ...state, deltalog: [] });
+    expect(newState).toMatchObject({
+      ...state,
+      deltalog: [],
+      transients: {
+        error: {
+          type: 'action/action_invalid',
+        },
+      },
+    });
   });
 });
 
@@ -588,7 +619,15 @@ describe('redo stack', () => {
     expect(state._redo).toHaveLength(1);
     const newState = reducer(state, redo('0'));
     expect(state._redo).toHaveLength(1);
-    expect(newState).toEqual({ ...state, deltalog: [] });
+    expect(newState).toMatchObject({
+      ...state,
+      deltalog: [],
+      transients: {
+        error: {
+          type: 'action/action_invalid',
+        },
+      },
+    });
   });
 });
 
@@ -772,3 +811,33 @@ describe('undo / redo with stages', () => {
     expect(state.ctx.activePlayers['0']).toBe('B');
   });
 });
+
+describe('TransientHandlingMiddleware', () => {
+  const middleware = applyMiddleware(TransientHandlingMiddleware);
+  let store = null;
+
+  beforeEach(() => {
+    store = createStore(reducer, initialState, middleware);
+  });
+
+  test('regular dispatch result has no transients', () => {
+    const result = store.dispatch(makeMove('A'));
+    expect(result).toEqual(
+      expect.not.objectContaining({ transients: expect.anything() })
+    );
+    expect(result).toEqual(
+      expect.not.objectContaining({ stripTransientsResult: expect.anything() })
+    );
+  });
+
+  test('failing dispatch result contains transients', () => {
+    const result = store.dispatch(makeMove('Invalid'));
+    expect(result).toMatchObject({
+      transients: {
+        error: {
+          type: 'action/invalid_move',
+        },
+      },
+    });
+  });
+});
diff --git a/src/master/master.test.ts b/src/master/master.test.ts
index 6d80951..fb2021b 100644
--- a/src/master/master.test.ts
+++ b/src/master/master.test.ts
@@ -16,6 +16,7 @@ import { Auth } from '../server/auth';
 import * as StorageAPI from '../server/db/base';
 import * as dateMock from 'jest-date-mock';
 import { PlayerView } from '../core/player-view';
+import { INVALID_MOVE } from '../core/constants';
 
 jest.mock('../core/logger', () => ({
   info: jest.fn(),
@@ -83,6 +84,12 @@ function TransportAPI(send = jest.fn(), sendAll = jest.fn()) {
   return { send, sendAll };
 }
 
+function validateNotTransientState(state: any) {
+  expect(state).toEqual(
+    expect.not.objectContaining({ transients: expect.anything() })
+  );
+}
+
 describe('sync', () => {
   const send = jest.fn();
   const db = new InMemory();
@@ -170,15 +177,19 @@ describe('update', () => {
   const sendAll = jest.fn((arg) => {
     sendAllReturn = arg;
   });
-  const db = new InMemory();
-  const master = new Master(game, db, TransportAPI(send, sendAll));
+  const game = {
+    moves: {
+      A: (G) => G,
+    },
+  };
+  let db;
+  let master;
   const action = ActionCreators.gameEvent('endTurn');
 
-  beforeAll(async () => {
+  beforeEach(async () => {
+    db = new InMemory();
+    master = new Master(game, db, TransportAPI(send, sendAll));
     await master.onSync('matchID', '0', undefined, 2);
-  });
-
-  beforeEach(() => {
     sendAllReturn = undefined;
     jest.clearAllMocks();
   });
@@ -223,7 +234,7 @@ describe('update', () => {
   });
 
   test('invalid matchID', async () => {
-    await master.onUpdate(action, 1, 'default:unknown', '1');
+    await master.onUpdate(action, 0, 'default:unknown', '1');
     expect(sendAll).not.toHaveBeenCalled();
     expect(error).toHaveBeenCalledWith(
       `game not found, matchID=[default:unknown]`
@@ -231,15 +242,15 @@ describe('update', () => {
   });
 
   test('invalid stateID', async () => {
-    await master.onUpdate(action, 100, 'matchID', '1');
+    await master.onUpdate(action, 100, 'matchID', '0');
     expect(sendAll).not.toHaveBeenCalled();
     expect(error).toHaveBeenCalledWith(
-      `invalid stateID, was=[100], expected=[1] - playerID=[1] - action[endTurn]`
+      `invalid stateID, was=[100], expected=[0] - playerID=[0] - action[endTurn]`
     );
   });
 
   test('invalid playerID', async () => {
-    await master.onUpdate(action, 1, 'matchID', '100');
+    await master.onUpdate(action, 0, 'matchID', '100');
     await master.onUpdate(ActionCreators.makeMove('move'), 1, 'matchID', '100');
     expect(sendAll).not.toHaveBeenCalled();
     expect(error).toHaveBeenCalledWith(
@@ -248,15 +259,15 @@ describe('update', () => {
   });
 
   test('invalid move', async () => {
-    await master.onUpdate(ActionCreators.makeMove('move'), 1, 'matchID', '1');
+    await master.onUpdate(ActionCreators.makeMove('move'), 0, 'matchID', '0');
     expect(sendAll).not.toHaveBeenCalled();
     expect(error).toHaveBeenCalledWith(
-      `move not processed - canPlayerMakeMove=false - playerID=[1] - action[move]`
+      `move not processed - canPlayerMakeMove=false - playerID=[0] - action[move]`
     );
   });
 
   test('valid matchID / stateID / playerID', async () => {
-    await master.onUpdate(action, 1, 'matchID', '1');
+    await master.onUpdate(action, 0, 'matchID', '0');
     expect(sendAll).toHaveBeenCalled();
   });
 
@@ -354,8 +365,15 @@ describe('update', () => {
 
   describe('undo / redo', () => {
     test('player 0 can undo', async () => {
+      const move = ActionCreators.makeMove('A', null, '0');
+      await master.onUpdate(move, 0, 'matchID', '0');
+      expect(error).not.toHaveBeenCalled();
+      await master.onUpdate(ActionCreators.undo(), 1, 'matchID', '0');
+      expect(error).not.toHaveBeenCalled();
+
+      // Negative case: All moves already undone.
       await master.onUpdate(ActionCreators.undo(), 2, 'matchID', '0');
-      expect(error).not.toBeCalled();
+      expect(error).toHaveBeenCalledWith(`No moves to undo`);
     });
 
     test('player 1 can’t undo', async () => {
@@ -371,30 +389,34 @@ describe('update', () => {
         [{ all: 'A' }],
         '0'
       );
-      await master.onUpdate(setActivePlayers, 2, 'matchID', '0');
-      await master.onUpdate(ActionCreators.undo('0'), 3, 'matchID', '0');
+      await master.onUpdate(setActivePlayers, 0, 'matchID', '0');
+      await master.onUpdate(ActionCreators.undo('0'), 1, 'matchID', '0');
       expect(error).toHaveBeenCalledWith(
         `playerID=[0] cannot undo / redo right now`
       );
     });
 
     test('player can undo if they are the only active player', async () => {
+      const move = ActionCreators.makeMove('A', null, '0');
+      await master.onUpdate(move, 0, 'matchID', '0');
+      expect(error).not.toHaveBeenCalled();
       const endStage = ActionCreators.gameEvent('endStage', undefined, '0');
-      await master.onUpdate(endStage, 3, 'matchID', '0');
-      await master.onUpdate(ActionCreators.undo('1'), 4, 'matchID', '1');
+      await master.onUpdate(endStage, 1, 'matchID', '0');
+      expect(error).not.toBeCalled();
+      await master.onUpdate(ActionCreators.undo(), 2, 'matchID', '0');
       expect(error).not.toBeCalled();
 
       // Clean-up active players.
       const endStage2 = ActionCreators.gameEvent('endStage', undefined, '1');
-      await master.onUpdate(endStage2, 4, 'matchID', '1');
+      await master.onUpdate(endStage2, 3, 'matchID', '1');
     });
   });
 
   test('game over', async () => {
     let event = ActionCreators.gameEvent('endGame');
-    await master.onUpdate(event, 5, 'matchID', '0');
+    await master.onUpdate(event, 0, 'matchID', '0');
     event = ActionCreators.gameEvent('endTurn');
-    await master.onUpdate(event, 6, 'matchID', '0');
+    await master.onUpdate(event, 1, 'matchID', '0');
     expect(error).toHaveBeenCalledWith(
       `game over - matchID=[matchID] - playerID=[0] - action[endTurn]`
     );
@@ -533,6 +555,9 @@ describe('patch', () => {
         stages: {
           A: {
             moves: {
+              Invalid: () => {
+                return INVALID_MOVE;
+              },
               A: {
                 client: false,
                 move: (G, ctx: Ctx) => {
@@ -560,6 +585,9 @@ describe('patch', () => {
   const action = ActionCreators.gameEvent('endTurn');
 
   beforeAll(async () => {
+    master.subscribe(({ state }) => {
+      validateNotTransientState(state);
+    });
     await master.onSync('matchID', '0', undefined, 2);
   });
 
@@ -624,7 +652,7 @@ describe('patch', () => {
     );
   });
 
-  test('invalid move', async () => {
+  test('disallowed move', async () => {
     await master.onUpdate(ActionCreators.makeMove('move'), 1, 'matchID', '0');
     expect(sendAll).not.toHaveBeenCalled();
     expect(error).toHaveBeenCalledWith(
@@ -632,6 +660,17 @@ describe('patch', () => {
     );
   });
 
+  test('invalid move', async () => {
+    await master.onUpdate(
+      ActionCreators.makeMove('Invalid', null, '0'),
+      1,
+      'matchID',
+      '0'
+    );
+    expect(sendAll).toHaveBeenCalled();
+    expect(error).toHaveBeenCalledWith('invalid move: Invalid args: null');
+  });
+
   test('valid matchID / stateID / playerID', async () => {
     await master.onUpdate(action, 1, 'matchID', '0');
     expect(sendAll).toHaveBeenCalled();
@@ -640,7 +679,8 @@ describe('patch', () => {
   describe('undo / redo', () => {
     test('player 0 can undo', async () => {
       await master.onUpdate(ActionCreators.undo(), 2, 'matchID', '1');
-      expect(error).not.toBeCalled();
+      // The master allows this, but the reducer does not.
+      expect(error).toHaveBeenCalledWith(`No moves to undo`);
     });
 
     test('player 1 can’t undo', async () => {
@@ -667,7 +707,8 @@ describe('patch', () => {
       const endStage = ActionCreators.gameEvent('endStage', undefined, '1');
       await master.onUpdate(endStage, 2, 'matchID', '1');
       await master.onUpdate(ActionCreators.undo('0'), 3, 'matchID', '1');
-      expect(error).not.toBeCalled();
+      // The master allows this, but the reducer does not.
+      expect(error).toHaveBeenCalledWith(`Cannot undo other players' moves`);
 
       // Clean-up active players.
       const endStage2 = ActionCreators.gameEvent('endStage', undefined, '1');
@@ -720,6 +761,9 @@ describe('connectionChange', () => {
 
   beforeEach(() => {
     sendAllReturn = undefined;
+    master.subscribe(({ state }) => {
+      validateNotTransientState(state);
+    });
     jest.clearAllMocks();
   });