feat(middleware): trigger a redirect from loadModuleData (#927)

Co-authored-by: Jonny Adshead <JAdshead@users.noreply.github.com>

.eslintrc.json

@@ -29,6 +29,14 @@
         // console methods are how we log events
         "no-console": 0
       }
+    },
+    {
+      "files": [
+        "**/*.md"
+      ],
+      "rules": {
+        "no-console": 0
+      }
     }
   ]
 }

__tests__/server/config/env/runTime.spec.js

@@ -281,6 +281,7 @@ describe('runTime', () => {
       expect(holocronModuleMapPath.defaultValue()).not.toBeDefined();
     });
 
+    // eslint-disable-next-line jest/expect-expect -- assertion is in nodeUrl
     it('ensures node can reach the URL', nodeUrl(holocronModuleMapPath));
 
     it('should use port numbers specified via HTTP_ONE_APP_DEV_CDN_PORT', () => {
@@ -299,6 +300,7 @@ describe('runTime', () => {
       expect(holocronServerMaxModulesRetry.defaultValue).toBe(undefined);
     });
 
+    // eslint-disable-next-line jest/expect-expect -- assertion is in positiveInteger
     it('validates the value as a positive integer', positiveInteger(holocronServerMaxModulesRetry));
   });
 
@@ -311,6 +313,7 @@ describe('runTime', () => {
       expect(holocronServerMaxSimModulesFetch.defaultValue).toBe(undefined);
     });
 
+    // eslint-disable-next-line jest/expect-expect -- assertion is in positiveInteger
     it(
       'validates the value as a positive integer',
       positiveInteger(holocronServerMaxSimModulesFetch)

__tests__/server/plugins/reactHtml/createRequestHtmlFragment.spec.jsx

@@ -18,8 +18,7 @@ import url from 'url';
 import { browserHistory, matchPromise } from '@americanexpress/one-app-router';
 import { Map as iMap, fromJS } from 'immutable';
 import { composeModules } from 'holocron';
-// getBreaker is only added in the mock
-/* eslint-disable-next-line import/named */
+// eslint-disable-next-line import/named -- getBreaker is only added in the mock
 import { getBreaker } from '../../../../src/server/utils/createCircuitBreaker';
 
 import * as reactRendering from '../../../../src/server/utils/reactRendering';
@@ -37,10 +36,7 @@ jest.mock('holocron', () => ({
 jest.mock('../../../../src/server/utils/createCircuitBreaker', () => {
   const breaker = jest.fn();
   const mockCreateCircuitBreaker = (asyncFunctionThatMightFail) => {
-    breaker.fire = jest.fn((...args) => {
-      asyncFunctionThatMightFail(...args);
-      return false;
-    });
+    breaker.fire = jest.fn(async (...args) => asyncFunctionThatMightFail(...args));
     return breaker;
   };
   mockCreateCircuitBreaker.getBreaker = () => breaker;
@@ -225,7 +221,7 @@ describe('createRequestHtmlFragment', () => {
   it('should fall back when the circuit opens', async () => {
     expect.assertions(4);
     const breaker = getBreaker();
-    breaker.fire.mockReturnValueOnce(true);
+    breaker.fire.mockReturnValueOnce({ fallback: true });
 
     await requireCreateRequestHtmlFragment(req, res, { createRoutes });
 
@@ -235,6 +231,43 @@ describe('createRequestHtmlFragment', () => {
     expect(req.appHtml).toBe('');
   });
 
+  it('should redirect instead of rendering when the circuit breaker returns a redirect', async () => {
+    expect.assertions(4);
+    composeModules.mockImplementationOnce(() => {
+      const error = new Error('An error that redirects');
+      error.abortComposeModules = true;
+      error.redirect = { status: 302, url: 'https://example.com' };
+      throw error;
+    });
+    await requireCreateRequestHtmlFragment(req, res, { createRoutes });
+    expect(res.redirect).toHaveBeenCalledWith(302, 'https://example.com');
+    expect(getBreaker().fire).toHaveBeenCalled();
+    expect(renderForStringSpy).not.toHaveBeenCalled();
+    expect(renderForStaticMarkupSpy).not.toHaveBeenCalled();
+  });
+
+  it('should default to a 302 redirect', async () => {
+    expect.assertions(1);
+    composeModules.mockImplementationOnce(() => {
+      const error = new Error('An error that redirects');
+      error.abortComposeModules = true;
+      error.redirect = { url: 'https://example.com' };
+      throw error;
+    });
+    await requireCreateRequestHtmlFragment(req, res, { createRoutes });
+    expect(res.redirect).toHaveBeenCalledWith(302, 'https://example.com');
+  });
+
+  it('should rethrow if the error does not contain a redirect', async () => {
+    expect.assertions(4);
+    composeModules.mockImplementationOnce(() => { throw new Error('An error that does not redirect'); });
+    await requireCreateRequestHtmlFragment(req, res, { createRoutes });
+    expect(res.redirect).not.toHaveBeenCalled();
+    expect(getBreaker().fire).toHaveBeenCalled();
+    expect(renderForStringSpy).not.toHaveBeenCalled();
+    expect(renderForStaticMarkupSpy).not.toHaveBeenCalled();
+  });
+
   it('should not use the circuit breaker for partials', async () => {
     expect.assertions(5);
 

__tests__/server/utils/createCircuitBreaker.spec.js

@@ -23,7 +23,7 @@ import createCircuitBreaker, {
 
 jest.useFakeTimers();
 
-const asyncFunctionThatMightFail = jest.fn(async () => false);
+const asyncFunctionThatMightFail = jest.fn(async () => ({ fallback: false }));
 const mockCircuitBreaker = createCircuitBreaker(asyncFunctionThatMightFail);
 
 jest.mock('holocron', () => ({
@@ -50,7 +50,7 @@ describe('Circuit breaker', () => {
     const input = 'hello, world';
     const value = await mockCircuitBreaker.fire(input);
     expect(asyncFunctionThatMightFail).toHaveBeenCalledWith(input);
-    expect(value).toBe(false);
+    expect(value).toEqual({ fallback: false });
   });
 
   it('should open the circuit when event loop delay threshold is exceeded', async () => {
@@ -62,7 +62,7 @@ describe('Circuit breaker', () => {
     jest.clearAllMocks();
     const value = await mockCircuitBreaker.fire('hola, mundo');
     expect(asyncFunctionThatMightFail).not.toHaveBeenCalled();
-    expect(value).toBe(true);
+    expect(value).toEqual({ fallback: true });
   });
 
   it('should not open the circuit when in development environment', async () => {
@@ -75,7 +75,7 @@ describe('Circuit breaker', () => {
     jest.clearAllMocks();
     const value = await mockCircuitBreaker.fire('hola, mundo');
     expect(asyncFunctionThatMightFail).toHaveBeenCalled();
-    expect(value).toBe(false);
+    expect(value).toEqual({ fallback: false });
   });
 
   it('should not open the circuit when threshold not exceeded', async () => {
@@ -87,7 +87,7 @@ describe('Circuit breaker', () => {
     jest.clearAllMocks();
     const value = await mockCircuitBreaker.fire('hola, mundo');
     expect(asyncFunctionThatMightFail).toHaveBeenCalled();
-    expect(value).toBe(false);
+    expect(value).toEqual({ fallback: false });
   });
 
   it('should not open the circuit when the root module is not loaded', async () => {
@@ -100,7 +100,7 @@ describe('Circuit breaker', () => {
     jest.clearAllMocks();
     const value = await mockCircuitBreaker.fire('hola, mundo');
     expect(asyncFunctionThatMightFail).toHaveBeenCalled();
-    expect(value).toBe(false);
+    expect(value).toEqual({ fallback: false });
   });
 
   it('should log when the healthcheck fails', async () => {

docs/api/modules/Loading-Data.md

@@ -29,6 +29,8 @@ Please see [`createSsrFetch`](./App-Configuration.md#createssrfetch) in the [App
 
 Please see the [Holocron Module Configuration](https://github.com/americanexpress/holocron/blob/main/packages/holocron/docs/api/README.md#holocron-module-configuration) from the Holocron API Docs for more information about other properties.
 
+Modules can trigger a server side redirect from `loadModuleData` by throwing an error that has the property `abortComposeModules` set to true and a `redirect` property containing an object with an HTTP status code (`status`) and the destination URL (`url`).
+
 ### `Module.holocron.loadModuleData`
 
 **Runs On**
@@ -40,7 +42,16 @@ Please see the [Holocron Module Configuration](https://github.com/americanexpres
 HelloWorldModule.holocron = {
   loadModuleData: async ({
     store, fetchClient, ownProps, module,
-  }) => {},
+  }) => {
+    const res = await fetchClient('https://example-api.com/data');
+    const data = await res.json();
+    if (data.redirect) {
+      const redirectError = new Error(`Redirect user to ${data.redirect.url}`);
+      redirectError.abortComposeModules = true;
+      redirectError.redirect = { status: 302, url: data.redirect.url };
+      throw redirectError;
+    }
+  },
 };
 ```
 

package.json

@@ -108,7 +108,7 @@
     "fastify": "^4.10.2",
     "fastify-plugin": "^4.2.0",
     "helmet": "^6.0.0",
-    "holocron": "^1.4.0",
+    "holocron": "^1.5.0",
     "holocron-module-route": "^1.3.0",
     "if-env": "^1.0.4",
     "immutable": "^4.1.0",

src/server/plugins/reactHtml/createRequestHtmlFragment.jsx

@@ -29,8 +29,13 @@ import {
 } from '../../utils/reactRendering';
 
 const getModuleData = async ({ dispatch, modules }) => {
-  await dispatch(composeModules(modules));
-  return false;
+  try {
+    await dispatch(composeModules(modules));
+  } catch (err) {
+    if (err.abortComposeModules && err.redirect) return { redirect: err.redirect, fallback: false };
+    throw err;
+  }
+  return { fallback: false };
 };
 
 const getModuleDataBreaker = createCircuitBreaker(getModuleData);
@@ -95,11 +100,16 @@ const createRequestHtmlFragment = async (request, reply, { createRoutes }) => {
       if (getRenderMethodName(state) === 'renderForStaticMarkup') {
         await dispatch(composeModules(routeModules));
       } else {
-        const fallback = await getModuleDataBreaker.fire({
+        const { fallback, redirect } = await getModuleDataBreaker.fire({
           dispatch,
           modules: routeModules,
         });
 
+        if (redirect) {
+          reply.redirect(redirect.status || 302, redirect.url);
+          return;
+        }
+
         if (fallback) {
           request.appHtml = '';
           request.helmetInfo = {};

src/server/utils/createCircuitBreaker.js

@@ -55,10 +55,10 @@ const checkMaxEventLoopDelay = async () => {
 };
 
 const createCircuitBreaker = (asyncFunctionThatMightFail) => {
-  // asyncFunctionThatMightFail should return false to indicate fallback is not needed
+  // asyncFunctionThatMightFail should return { fallback: false } to indicate fallback is not needed
   const breaker = new CircuitBreaker(asyncFunctionThatMightFail, options);
   // Fallback returns true to indicate fallback behavior is needed
-  breaker.fallback(() => true);
+  breaker.fallback(() => ({ fallback: true }));
   // Check the max event loop delay every 5 seconds
   breaker.healthCheck(checkMaxEventLoopDelay, 5e3);
   // Log when circuit breaker opens and closes