fix(watchLocalModules): fix watcher crashing due to build errors (#306)

This was caused when a module would throw a linting error or something similar during the building of the module. This was not being caught and therefore was causing the app to crash.
americanexpress · Sep 22, 2020 · 9290d92 · 9290d92
1 parent 4a90b98
commit 9290d92
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 32 deletions.
diff --git a/__tests__/server/utils/watchLocalModules.spec.js b/__tests__/server/utils/watchLocalModules.spec.js
@@ -66,6 +66,7 @@ jest.mock('fs', () => {
     readFileSync: jest.fn(actual.readFileSync),
   };
 });
+jest.spyOn(console, 'error').mockImplementation(() => {});
 
 describe('watchLocalModules', () => {
   beforeEach(() => jest.clearAllMocks());
@@ -123,6 +124,49 @@ describe('watchLocalModules', () => {
     expect(getModules().get(moduleName)).toBe(updatedModule);
   });
 
+  it('handles a rejection properly', async () => {
+    const moduleName = 'some-module';
+    const moduleVersion = '1.0.1';
+    const moduleMapSample = {
+      modules: {
+        [moduleName]: {
+          node: {
+            integrity: '133',
+            url: `https://example.com/cdn/${moduleName}/${moduleVersion}/${moduleName}-node.js`,
+          },
+          browser: {
+            integrity: '234',
+            url: `https://example.com/cdn/${moduleName}/${moduleVersion}/${moduleName}-browser.js`,
+          },
+          legacyBrowser: {
+            integrity: '134633',
+            url: `https://example.com/cdn/${moduleName}/${moduleVersion}/${moduleName}-legacy.browser.js`,
+          },
+        },
+      },
+    };
+    fs.readFileSync.mockImplementationOnce(() => JSON.stringify(moduleMapSample));
+    const modulePath = path.resolve(__dirname, `../../../static/modules/${moduleName}/${moduleVersion}/${moduleName}.node.js`);
+    const originalModule = () => null;
+    const updatedModule = () => null;
+    const modules = fromJS({ [moduleName]: originalModule });
+    const moduleMap = fromJS(moduleMapSample);
+    resetModuleRegistry(modules, moduleMap);
+    watchLocalModules();
+    const changeListener = chokidar.getListeners().change;
+    expect(getModules().get(moduleName)).toBe(originalModule);
+    loadModule.mockReturnValueOnce(Promise.reject(updatedModule));
+    await changeListener(modulePath);
+    expect(loadModule).toHaveBeenCalledWith(
+      moduleName,
+      moduleMapSample.modules[moduleName],
+      require('../../../src/server/utils/onModuleLoad').default
+    );
+    expect(getModules().get(moduleName)).toBe(originalModule);
+    expect(console.error).toHaveBeenCalledTimes(1);
+    expect(console.error).toHaveBeenCalledWith(updatedModule);
+  });
+
   it('should ignore when the regex doesn\'t match', async () => {
     const changedPath = path.resolve(__dirname, '../../../static/modules/dont-match-me-bro.node.js');
     watchLocalModules();

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -91,10 +91,10 @@
     "core-js": "^3.6.5",
     "cors": "^2.8.5",
     "create-shared-react-context": "^1.0.3",
-    "cross-fetch": "^3.0.5",
+    "cross-fetch": "^3.0.6",
     "express": "^4.17.1",
     "helmet": "^3.22.0",
-    "holocron": "^1.1.0",
+    "holocron": "^1.1.1",
     "holocron-module-route": "^1.1.0",
     "if-env": "^1.0.4",
     "immutable": "^4.0.0-rc.12",

diff --git a/src/server/utils/watchLocalModules.js b/src/server/utils/watchLocalModules.js
@@ -39,28 +39,32 @@ export default function watchLocalModules() {
   const watcher = chokidar.watch(moduleDirectory, { awaitWriteFinish: true });
 
   watcher.on('change', async (changedPath) => {
-    if (!changedPath.endsWith('.node.js')) return;
+    try {
+      if (!changedPath.endsWith('.node.js')) return;
 
-    const match = changedPath.substring(moduleDirectory.length).match(/\/([^/]+)\/([^/]+)/);
-    if (!match) return;
-    const [, moduleNameChangeDetectedIn] = match;
+      const match = changedPath.substring(moduleDirectory.length).match(/\/([^/]+)\/([^/]+)/);
+      if (!match) return;
+      const [, moduleNameChangeDetectedIn] = match;
 
-    const moduleMap = JSON.parse(fs.readFileSync(moduleMapPath, 'utf8'));
+      const moduleMap = JSON.parse(fs.readFileSync(moduleMapPath, 'utf8'));
 
-    const moduleData = moduleMap.modules[moduleNameChangeDetectedIn];
-    const oneAppDevCdnAddress = `http://${ip}:${process.env.HTTP_ONE_APP_DEV_CDN_PORT || 3001}`;
+      const moduleData = moduleMap.modules[moduleNameChangeDetectedIn];
+      const oneAppDevCdnAddress = `http://${ip}:${process.env.HTTP_ONE_APP_DEV_CDN_PORT || 3001}`;
 
-    moduleData.browser.url = moduleData.browser.url.replace('[one-app-dev-cdn-url]', oneAppDevCdnAddress);
-    moduleData.legacyBrowser.url = moduleData.legacyBrowser.url.replace('[one-app-dev-cdn-url]', oneAppDevCdnAddress);
-    moduleData.node.url = moduleData.node.url.replace('[one-app-dev-cdn-url]', oneAppDevCdnAddress);
+      moduleData.browser.url = moduleData.browser.url.replace('[one-app-dev-cdn-url]', oneAppDevCdnAddress);
+      moduleData.legacyBrowser.url = moduleData.legacyBrowser.url.replace('[one-app-dev-cdn-url]', oneAppDevCdnAddress);
+      moduleData.node.url = moduleData.node.url.replace('[one-app-dev-cdn-url]', oneAppDevCdnAddress);
 
-    const module = addHigherOrderComponent(await loadModule(
-      moduleNameChangeDetectedIn,
-      moduleData,
-      onModuleLoad
-    ));
+      const module = addHigherOrderComponent(await loadModule(
+        moduleNameChangeDetectedIn,
+        moduleData,
+        onModuleLoad
+      ));
 
-    const modules = getModules().set(moduleNameChangeDetectedIn, module);
-    resetModuleRegistry(modules, getModuleMap());
+      const modules = getModules().set(moduleNameChangeDetectedIn, module);
+      resetModuleRegistry(modules, getModuleMap());
+    } catch (error) {
+      console.error(error);
+    }
   });
 }