fix(holocron): bad modules could cause crashes and prevent restart

__tests__/integration/helpers/moduleMap.js

@@ -84,4 +84,5 @@ module.exports = {
   retrieveModuleIntegrityDigests,
   writeModuleMap,
   readModuleMap,
+  testCdnUrl,
 };

__tests__/integration/one-app.spec.js

@@ -35,6 +35,8 @@ import {
   writeModuleMap,
   readModuleMap,
   retrieveModuleIntegrityDigests,
+  retrieveGitSha,
+  testCdnUrl,
 } from './helpers/moduleMap';
 import {
   searchForNextLogMatch,
@@ -55,6 +57,51 @@ yargs.array('scanEnvironment');
 jest.setTimeout(95000);
 
 describe('Tests that require Docker setup', () => {
+  describe('one-app startup with bad module module', () => {
+    let originalModuleMap;
+    const oneAppLocalPortToUse = getRandomPortNumber();
+    const oneAppMetricsLocalPortToUse = getRandomPortNumber();
+    let browser;
+    const moduleName = 'unhealthy-frank';
+    const version = '0.0.0';
+    beforeAll(async () => {
+      originalModuleMap = readModuleMap();
+
+      await addModuleToModuleMap({
+        moduleName,
+        version,
+      });
+      ({ browser } = await setUpTestRunner({ oneAppLocalPortToUse, oneAppMetricsLocalPortToUse }));
+    });
+
+    afterAll(async () => {
+      await tearDownTestRunner({ browser });
+      writeModuleMap(originalModuleMap);
+    });
+    test('one-app starts up successfully with a bad module', async () => {
+      const revertErrorMatch = /There was an error loading module (?<moduleName>.*) at (?<url>.*). Ignoring (?<workingModule>.*) until .*/;
+      const requiredExternalsError = searchForNextLogMatch(revertErrorMatch);
+      const loggedError = await requiredExternalsError;
+      const [,
+        problemModule,
+        problemModuleUrl,
+        workingUrl,
+      ] = revertErrorMatch.exec(loggedError);
+      const gitSha = await retrieveGitSha();
+      await expect(requiredExternalsError).resolves.toMatch(revertErrorMatch);
+      expect(problemModule).toBe(moduleName);
+      expect(problemModuleUrl).toBe(`${testCdnUrl}/${gitSha}/${moduleName}/${version}/${moduleName}.node.js`);
+      // eslint-disable-next-line no-useless-escape
+      expect(workingUrl).toBe(moduleName);
+    });
+    test('one-app remains healthy with a bad module at start', async () => {
+      await browser.url('https://one-app:8443/success');
+      const header = await browser.$('.helloMessage');
+      const headerText = await header.getText();
+      expect(headerText).toBe('Hello! One App is successfully rendering its Modules!');
+    });
+  });
+
   describe('one-app successfully started', () => {
     const defaultFetchOptions = createFetchOptions();
     let originalModuleMap;
@@ -71,6 +118,7 @@ describe('Tests that require Docker setup', () => {
 
     beforeAll(async () => {
       removeModuleFromModuleMap('late-frank');
+      removeModuleFromModuleMap('unhealthy-frank');
       originalModuleMap = readModuleMap();
       ({ browser } = await setUpTestRunner({ oneAppLocalPortToUse, oneAppMetricsLocalPortToUse }));
     });
@@ -700,30 +748,71 @@ describe('Tests that require Docker setup', () => {
         });
 
         describe('child module `requiredExternals` invalid usage', () => {
-          const requiredExternalsErrorMatch = /Failed to get external react-intl from root module/;
-
           const moduleName = 'cultured-frankie';
           const version = '0.0.1';
 
-          let requiredExternalsError;
+          afterEach(() => {
+            writeModuleMap(originalModuleMap);
+          });
 
-          beforeAll(async () => {
-            requiredExternalsError = searchForNextLogMatch(requiredExternalsErrorMatch);
+          test('fails to get external `react-intl` for child module as an unsupplied `requiredExternal` - logs failure', async () => {
+            const requiredExternalsErrorMatch = /Failed to get external react-intl from root module/;
+
+            const requiredExternalsError = searchForNextLogMatch(requiredExternalsErrorMatch);
             await addModuleToModuleMap({
               moduleName,
               version,
             });
             // not ideal but need to wait for app to poll;
-
             await waitFor(5000);
-          });
 
-          afterAll(() => {
-            writeModuleMap(originalModuleMap);
+            await expect(requiredExternalsError).resolves.toMatch(requiredExternalsErrorMatch);
           });
 
-          test('fails to get external `react-intl` for child module as an unsupplied `requiredExternal`', async () => {
-            await expect(requiredExternalsError).resolves.toMatch(requiredExternalsErrorMatch);
+          test('fails to get external `react-intl` for child module as an unsupplied `requiredExternal` - Logs reverting message', async () => {
+            const revertErrorMatch = /There was an error loading module (?<moduleName>.*) at (?<url>.*). Reverting back to (?<workingModule>.*)/;
+            const requiredExternalsError = searchForNextLogMatch(revertErrorMatch);
+            await addModuleToModuleMap({
+              moduleName,
+              version,
+            });
+            // not ideal but need to wait for app to poll;
+            await waitFor(5000);
+            const loggedError = await requiredExternalsError;
+            const [,
+              problemModule,
+              problemModuleUrl,
+              workingUrl,
+            ] = revertErrorMatch.exec(loggedError);
+            const gitSha = await retrieveGitSha();
+            await expect(requiredExternalsError).resolves.toMatch(revertErrorMatch);
+            expect(problemModule).toBe('cultured-frankie');
+            expect(problemModuleUrl).toBe(`${testCdnUrl}/${gitSha}/${moduleName}/${version}/${moduleName}.node.js`);
+            // eslint-disable-next-line no-useless-escape
+            expect(workingUrl).toBe(`${testCdnUrl}/${gitSha}/${moduleName}/0.0.0/${moduleName}.node.js\"}`);
+          });
+          test('fails to get external `semver` for child module as an unsupplied `requiredExternal` for new module in mooduleMap', async () => {
+            const revertErrorMatch = /There was an error loading module (?<moduleName>.*) at (?<url>.*). Ignoring (?<ignoredModule>.*) until .*/;
+            const requiredExternalsError = searchForNextLogMatch(revertErrorMatch);
+            const modName = 'unhealthy-frank';
+            const modVersion = '0.0.0';
+            await addModuleToModuleMap({
+              moduleName: modName,
+              version: modVersion,
+            });
+            // not ideal but need to wait for app to poll;
+            await waitFor(5000);
+            const loggedError = await requiredExternalsError;
+            const [,
+              problemModule,
+              problemModuleUrl,
+              ignoredModule,
+            ] = revertErrorMatch.exec(loggedError);
+            const gitSha = await retrieveGitSha();
+            await expect(requiredExternalsError).resolves.toMatch(revertErrorMatch);
+            expect(problemModule).toBe(modName);
+            expect(problemModuleUrl).toBe(`${testCdnUrl}/${gitSha}/${modName}/${modVersion}/${modName}.node.js`);
+            expect(ignoredModule).toBe(modName);
           });
 
           test('does not modify the original version "0.0.0" of the failing module', async () => {

package.json

@@ -95,7 +95,7 @@
     "cross-fetch": "^3.0.6",
     "express": "^4.17.1",
     "helmet": "^3.22.0",
-    "holocron": "^1.1.4",
+    "holocron": "^1.2.0",
     "holocron-module-route": "^1.1.4",
     "if-env": "^1.0.4",
     "immutable": "^4.0.0",

prod-sample/sample-modules/unhealthy-frank/0.0.0/.babelrc

@@ -0,0 +1,3 @@
+{
+  "presets": ["amex"]
+}

prod-sample/sample-modules/unhealthy-frank/0.0.0/.npmrc

@@ -0,0 +1 @@
+registry=https://registry.npmjs.org

prod-sample/sample-modules/unhealthy-frank/0.0.0/README.md

@@ -0,0 +1,3 @@
+# unhealthy-frank
+
+A module that has mismatching requiredExternals.

prod-sample/sample-modules/unhealthy-frank/0.0.0/package.json

@@ -0,0 +1,32 @@
+{
+  "name": "unhealthy-frank",
+  "version": "0.0.0",
+  "description": "A module that works, nothing more, nothing less.",
+  "scripts": {
+    "prebuild": "npm run clean",
+    "build": "bundle-module",
+    "clean": "rimraf build",
+    "prepare": "npm run build"
+  },
+  "dependencies": {
+    "@americanexpress/one-app-router": "^1.1.0",
+    "holocron": "^1.1.4",
+    "holocron-module-route": "^1.1.4",
+    "prop-types": "^15.5.9",
+    "react": "^16.14.0",
+    "react-redux": "^7.2.4",
+    "semver": "7.3.5"
+  },
+  "devDependencies": {
+    "@americanexpress/one-app-bundler": "^6.14.1",
+    "babel-preset-amex": "^3.5.0",
+    "rimraf": "^2.5.2"
+  },
+  "one-amex": {
+    "bundler": {
+      "requiredExternals": [
+        "semver"
+      ]
+    }
+  }
+}

prod-sample/sample-modules/unhealthy-frank/0.0.0/src/components/UnhealthyFrank.jsx

@@ -0,0 +1,26 @@
+/*
+ * Copyright 2019 American Express Travel Related Services Company, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing
+ * permissions and limitations under the License.
+ */
+
+import React from 'react';
+import semver from 'semver';
+
+export default function UnhealthyFrank() {
+  return (
+    <div>
+      <h1 className="helloFrank">Im Frank, and unhealthy. is 1.2.3 valid semver? {semver.valid('1.2.3')}</h1>
+    </div>
+  );
+}

prod-sample/sample-modules/unhealthy-frank/0.0.0/src/index.js

@@ -0,0 +1,19 @@
+/*
+ * Copyright 2019 American Express Travel Related Services Company, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing
+ * permissions and limitations under the License.
+ */
+
+import ModuleContainer from './components/UnhealthyFrank';
+
+export default ModuleContainer;