Repository: boardgameio/boardgame.io (ts)

Issue:
Add API allowing plugins to declare the current action invalid
Currently we support returning `INVALID_MOVE` from moves to bail out of a state update. There are circumstances where a plugin might also want to flag that an action is invalid. For example, a plugin that validates state or — as came up in https://github.com/boardgameio/boardgame.io/pull/957#pullrequestreview-709062516 — in the events plugin if there is an error processing the event queue.

This would probably look something like the current `noClient` plugin method. A plugin could declare an `isInvalid` method that could return any truthy value to invalidate the current action.

```js
const plugin = {
  name: 'validator',
  isInvalid: ({ G }) => {
    if (typeof G.value !== number) {
      return { error: 'G.value must be a number' };
    }
  },
};
```

Once we have something like #723, plugin errors could be added to the possible error types and the return value of `isInvalid` returned to the client for debugging.

Pull request:
feat: Allow plugins to declare an action invalid
Closes #962

This adds support for an `isInvalid` method in boardgame.io plugins. Similar to the current `noClient` plugin method, this allows the reducer to ask plugins if everything is OK or if the current event/move is invalid from their point of view. If one or more plugins returns a truthy value from its `isInvalid` method, the reducer will fall back to its initial state, analogous to what it does when a move returns `INVALID_MOVE`.

Currently this is not in use but the intention would be to use it for the events plugin, following on from #957. It also allows for other custom use cases, like state validators.

#### Notes

- `isInvalid` should return a string if the action is invalid. This could be less strict if that seems desirable.

- The error returned to the client is of the form
    ```js
    {
      type: 'action/plugin_invalid',
      payload: {
        plugin: 'name of plugin that declared action invalid',
        message: 'string returned by plugin’s isInvalid method'
      },
    }
    ```

- If more than one plugin’s `isInvalid` method returns a string, we just error with the first of them, rather than reporting all of them.

Patch:
diff --git a/docs/documentation/plugins.md b/docs/documentation/plugins.md
index a454452..3d03acd 100644
--- a/docs/documentation/plugins.md
+++ b/docs/documentation/plugins.md
@@ -47,6 +47,12 @@ A plugin is an object that contains the following fields.
   // for the master instead.
   noClient: ({ G, ctx, game, data, api }) => boolean,
 
+  // Function that allows the plugin to indicate that the
+  // current action should be declared invalid and cancelled.
+  // If `isInvalid` returns an error message, the whole update
+  // will be abandoned and an error returned to the client.
+  isInvalid: ({ G, ctx, game, data, api }) => false | string,
+
   // Function that can filter `data` to hide secret state
   // before sending it to a specific client.
   // `playerID` could also be null or undefined for spectators.
diff --git a/src/core/errors.ts b/src/core/errors.ts
index 267c862..ad2fbb9 100644
--- a/src/core/errors.ts
+++ b/src/core/errors.ts
@@ -30,4 +30,6 @@ export enum ActionErrorType {
   ActionDisabled = 'action/action_disabled',
   // The requested action is not currently possible
   ActionInvalid = 'action/action_invalid',
+  // The requested action was declared invalid by a plugin
+  PluginActionInvalid = 'action/plugin_invalid',
 }
diff --git a/src/core/reducer.ts b/src/core/reducer.ts
index beba12b..4f51417 100644
--- a/src/core/reducer.ts
+++ b/src/core/reducer.ts
@@ -128,6 +128,30 @@ function initializeDeltalog(
   };
 }
 
+/**
+ * Update plugin state after move/event & check if plugins consider the action to be valid.
+ * @param newState Latest version of state in the reducer.
+ * @param oldState Initial value of state when reducer started its work.
+ * @param pluginOpts Plugin configuration options.
+ * @returns Tuple of the new state updated after flushing plugins and the old
+ * state augmented with an error if a plugin declared the action invalid.
+ */
+function flushAndValidatePlugins(
+  newState: State,
+  oldState: State,
+  pluginOpts: { game: Game; isClient?: boolean }
+): [State, TransientState?] {
+  newState = plugins.Flush(newState, pluginOpts);
+  const isInvalid = plugins.IsInvalid(newState, pluginOpts);
+  if (!isInvalid) return [newState];
+  const { plugin, message } = isInvalid;
+  error(`plugin declared action invalid: ${plugin} - ${message}`);
+  return [
+    newState,
+    WithError(oldState, ActionErrorType.PluginActionInvalid, isInvalid),
+  ];
+}
+
 /**
  * ExtractTransientsFromState
  *
@@ -271,7 +295,12 @@ export function CreateGameReducer({
         let newState = game.flow.processEvent(state, action);
 
         // Execute plugins.
-        newState = plugins.Flush(newState, { game, isClient: false });
+        let stateWithError: TransientState | undefined;
+        [newState, stateWithError] = flushAndValidatePlugins(newState, state, {
+          game,
+          isClient: false,
+        });
+        if (stateWithError) return stateWithError;
 
         // Update undo / redo state.
         newState = updateUndoRedoState(newState, { game, action });
@@ -280,7 +309,7 @@ export function CreateGameReducer({
       }
 
       case Actions.MAKE_MOVE: {
-        state = { ...state, deltalog: [] };
+        const oldState = (state = { ...state, deltalog: [] });
 
         // Check whether the move is allowed at this time.
         const move: Move = game.flow.getMove(
@@ -348,10 +377,12 @@ export function CreateGameReducer({
         // These will be processed on the server, which
         // will send back a state update.
         if (isClient) {
-          state = plugins.Flush(state, {
+          let stateWithError: TransientState | undefined;
+          [state, stateWithError] = flushAndValidatePlugins(state, oldState, {
             game,
             isClient: true,
           });
+          if (stateWithError) return stateWithError;
           return {
             ...state,
             _stateID: state._stateID + 1,
@@ -363,7 +394,11 @@ export function CreateGameReducer({
 
         // Allow the flow reducer to process any triggers that happen after moves.
         state = game.flow.processMove(state, action.payload);
-        state = plugins.Flush(state, { game });
+        let stateWithError: TransientState | undefined;
+        [state, stateWithError] = flushAndValidatePlugins(state, oldState, {
+          game,
+        });
+        if (stateWithError) return stateWithError;
 
         // Update undo / redo state.
         state = updateUndoRedoState(state, { game, action });
diff --git a/src/plugins/main.ts b/src/plugins/main.ts
index 23bcc96..420eab9 100644
--- a/src/plugins/main.ts
+++ b/src/plugins/main.ts
@@ -243,6 +243,34 @@ export const NoClient = (state: State, opts: PluginOpts): boolean => {
     .some((value) => value === true);
 };
 
+/**
+ * Allows plugins to indicate if the entire action should be thrown out
+ * as invalid. This will cancel the entire state update.
+ */
+export const IsInvalid = (
+  state: State,
+  opts: PluginOpts
+): false | { plugin: string; message: string } => {
+  const firstInvalidReturn = [...DEFAULT_PLUGINS, ...opts.game.plugins]
+    .filter((plugin) => plugin.isInvalid !== undefined)
+    .map((plugin) => {
+      const { name } = plugin;
+      const pluginState = state.plugins[name];
+
+      const message = plugin.isInvalid({
+        G: state.G,
+        ctx: state.ctx,
+        game: opts.game,
+        api: pluginState && pluginState.api,
+        data: pluginState && pluginState.data,
+      });
+
+      return message ? { plugin: name, message } : false;
+    })
+    .find((value) => value);
+  return firstInvalidReturn || false;
+};
+
 /**
  * Allows plugins to customize their data for specific players.
  * For example, a plugin may want to share no data with the client, or
diff --git a/src/types.ts b/src/types.ts
index 85257e2..2efb55d 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -144,6 +144,7 @@ export interface Plugin<
 > {
   name: string;
   noClient?: (context: PluginContext<API, Data, G>) => boolean;
+  isInvalid?: (context: PluginContext<API, Data, G>) => false | string;
   setup?: (setupCtx: { G: G; ctx: Ctx; game: Game<G, Ctx> }) => Data;
   action?: (data: Data, payload: ActionShape.Plugin['payload']) => Data;
   api?: (context: {


Test patch:
diff --git a/src/core/reducer.test.ts b/src/core/reducer.test.ts
index 0d2c839..f6ff82e 100644
--- a/src/core/reducer.test.ts
+++ b/src/core/reducer.test.ts
@@ -299,6 +299,96 @@ describe('Events API', () => {
   });
 });
 
+describe('Plugin Invalid Action API', () => {
+  const pluginName = 'validator';
+  const message = 'G.value must divide by 5';
+  const game: Game<{ value: number }> = {
+    setup: () => ({ value: 5 }),
+    plugins: [
+      {
+        name: pluginName,
+        isInvalid: ({ G }) => {
+          if (G.value % 5 !== 0) return message;
+          return false;
+        },
+      },
+    ],
+    moves: {
+      setValue: (G, _ctx, arg) => {
+        G.value = arg;
+      },
+    },
+    phases: {
+      unenterable: {
+        onBegin: () => ({ value: 13 }),
+      },
+      enterable: {
+        onBegin: () => ({ value: 25 }),
+      },
+    },
+  };
+
+  let state: State;
+  beforeEach(() => {
+    state = InitializeGame({ game });
+  });
+
+  describe('multiplayer client', () => {
+    const reducer = CreateGameReducer({ game });
+
+    test('move is cancelled if plugin declares it invalid', () => {
+      state = reducer(state, makeMove('setValue', [6], '0'));
+      expect(state.G).toMatchObject({ value: 5 });
+      expect(state['transients'].error).toEqual({
+        type: 'action/plugin_invalid',
+        payload: { plugin: pluginName, message },
+      });
+    });
+
+    test('move is processed if no plugin declares it invalid', () => {
+      state = reducer(state, makeMove('setValue', [15], '0'));
+      expect(state.G).toMatchObject({ value: 15 });
+      expect(state['transients']).toBeUndefined();
+    });
+
+    test('event is cancelled if plugin declares it invalid', () => {
+      state = reducer(state, gameEvent('setPhase', 'unenterable', '0'));
+      expect(state.G).toMatchObject({ value: 5 });
+      expect(state.ctx.phase).toBe(null);
+      expect(state['transients'].error).toEqual({
+        type: 'action/plugin_invalid',
+        payload: { plugin: pluginName, message },
+      });
+    });
+
+    test('event is processed if no plugin declares it invalid', () => {
+      state = reducer(state, gameEvent('setPhase', 'enterable', '0'));
+      expect(state.G).toMatchObject({ value: 25 });
+      expect(state.ctx.phase).toBe('enterable');
+      expect(state['transients']).toBeUndefined();
+    });
+  });
+
+  describe('local client', () => {
+    const reducer = CreateGameReducer({ game, isClient: true });
+
+    test('move is cancelled if plugin declares it invalid', () => {
+      state = reducer(state, makeMove('setValue', [6], '0'));
+      expect(state.G).toMatchObject({ value: 5 });
+      expect(state['transients'].error).toEqual({
+        type: 'action/plugin_invalid',
+        payload: { plugin: pluginName, message },
+      });
+    });
+
+    test('move is processed if no plugin declares it invalid', () => {
+      state = reducer(state, makeMove('setValue', [15], '0'));
+      expect(state.G).toMatchObject({ value: 15 });
+      expect(state['transients']).toBeUndefined();
+    });
+  });
+});
+
 describe('Random inside setup()', () => {
   const game1: Game = {
     seed: 'seed1',
diff --git a/src/plugins/main.test.ts b/src/plugins/main.test.ts
index 94a7062..bf3ca9f 100644
--- a/src/plugins/main.test.ts
+++ b/src/plugins/main.test.ts
@@ -8,6 +8,7 @@
 
 import { Client } from '../client/client';
 import { Local } from '../client/transport/local';
+import type { Game } from '../types';
 
 describe('basic', () => {
   let client: ReturnType<typeof Client>;
@@ -134,6 +135,69 @@ describe('default values', () => {
   });
 });
 
+describe('isInvalid method', () => {
+  // Silence expected error logging and restore when finished.
+  const stderr = console.error;
+  beforeAll(() => (console.error = () => {}));
+  afterAll(() => (console.error = stderr));
+
+  test('basic plugin', () => {
+    const goodG = { good: 'nice' };
+    const game: Game = {
+      plugins: [
+        {
+          name: 'test',
+          isInvalid: ({ G }) => 'bad' in G && 'not ok',
+        },
+      ],
+      moves: {
+        good: () => goodG,
+        bad: () => ({ bad: 'not ok' }),
+      },
+    };
+
+    const client = Client({ game, playerID: '0' });
+    client.start();
+    client.moves.good();
+    expect(client.getState().G).toEqual(goodG);
+    client.moves.bad();
+    expect(client.getState().G).toEqual(goodG);
+  });
+
+  test('plugin with API and data', () => {
+    const game: Game<any, any> = {
+      plugins: [
+        {
+          name: 'test',
+          setup: () => ({}),
+          api: ({ data }) => ({
+            set: (key, val) => {
+              data[key] = val;
+            },
+          }),
+          isInvalid: ({ data }) => 'bad' in data && 'not ok',
+        },
+      ],
+      moves: {
+        good: (_, ctx) => {
+          ctx.test.set('good', 'nice');
+        },
+        bad: (_, ctx) => {
+          ctx.test.set('bad', 'not ok');
+        },
+      },
+    };
+
+    const client = Client({ game, playerID: '0' });
+    client.start();
+    expect(client.getState().ctx.numMoves).toBe(0);
+    client.moves.good();
+    expect(client.getState().ctx.numMoves).toBe(1);
+    client.moves.bad();
+    expect(client.getState().ctx.numMoves).toBe(1);
+  });
+});
+
 describe('actions', () => {
   let client;