Fix reporting of config errors (#1342)

Errors are swallowed while resolving.

bin/svgo

@@ -1,6 +1,10 @@
 #!/usr/bin/env node
 
+const { red } = require('chalk');
 const { program } = require('commander');
 const makeProgram = require('../lib/svgo/coa');
 makeProgram(program);
-program.parseAsync(process.argv);
+program.parseAsync(process.argv).catch(error => {
+  console.error(red(error.stack));
+  process.exit(1);
+});

lib/svgo-node.js

@@ -10,19 +10,26 @@ const {
 
 const importConfig = async configFile => {
   try {
+    await fs.promises.access(configFile);
+  } catch {
+    return null;
+  }
   const config = require(configFile);
   if (config == null || typeof config !== 'object' || Array.isArray(config)) {
     throw Error(`Invalid config file "${configFile}"`);
   }
   return config;
-  } catch (error) {
-    if (error.code === 'MODULE_NOT_FOUND') {
-      return null;
-    }
-    throw error;
-  }
 };
 
+const isFile = async (file) => {
+  try {
+    const stats = await fs.promises.stat(file);
+    return stats.isFile();
+  } catch {
+    return false;
+  }
+}
+
 const loadConfig = async (configFile, cwd = process.cwd()) => {
   if (configFile != null) {
     if (path.isAbsolute(configFile)) {
@@ -33,13 +40,10 @@ const loadConfig = async (configFile, cwd = process.cwd()) => {
   }
   let dir = cwd;
   while (true) {
-    try {
     const file = path.join(dir, "svgo.config.js");
-      const stats = await fs.promises.stat(file);
-      if (stats.isFile()) {
+    if (await isFile(file)) {
       return await importConfig(file);
     }
-    } catch {}
     const parent = path.dirname(dir);
     if (dir === parent) {
       return null;

lib/svgo/coa.js

@@ -9,7 +9,6 @@ const pluginsMap = require('../../plugins/plugins.js');
 const PKG = require('../../package.json');
 const { encodeSVGDatauri, decodeSVGDatauri } = require('./tools.js');
 const regSVGFile = /\.svg$/i;
-const noop = () => {};
 
 /**
  * Synchronously check if path is a directory. Tolerant to errors like ENOENT.
@@ -97,19 +96,15 @@ async function action(args, opts) {
     if (typeof process == 'object' && process.versions && process.versions.node && PKG && PKG.engines.node) {
         var nodeVersion = String(PKG.engines.node).match(/\d*(\.\d+)*/)[0];
         if (parseFloat(process.versions.node) < parseFloat(nodeVersion)) {
-            return printErrorAndExit(`Error: ${PKG.name} requires Node.js version ${nodeVersion} or higher.`);
+            throw Error(`${PKG.name} requires Node.js version ${nodeVersion} or higher.`);
         }
     }
 
     // --config
-    try {
     const loadedConfig = await loadConfig(opts.config);
     if (loadedConfig != null) {
       config = loadedConfig;
     }
-    } catch (error) {
-      return printErrorAndExit(error.message);
-    }
 
     // --quiet
     if (opts.quiet) {
@@ -166,7 +161,7 @@ async function action(args, opts) {
     // --folder
     if (opts.folder) {
         var ouputFolder = output && output[0] || opts.folder;
-        return optimizeFolder(config, opts.folder, ouputFolder).then(noop, printErrorAndExit);
+        await optimizeFolder(config, opts.folder, ouputFolder);
     }
 
     // --input
@@ -183,8 +178,9 @@ async function action(args, opts) {
             });
         // file
         } else {
-            return Promise.all(input.map((file, n) => optimizeFile(config, file, output[n])))
-                .then(noop, printErrorAndExit);
+            await Promise.all(
+              input.map((file, n) => optimizeFile(config, file, output[n]))
+            );
         }
 
     // --string
@@ -390,15 +386,4 @@ function showAvailablePlugins() {
     console.log('Currently available plugins:\n' + list);
 }
 
-/**
- * Write an error and exit.
- * @param {Error} error
- * @return {Promise} a promise for running tests
- */
-function printErrorAndExit(error) {
-    console.error(chalk.red(error));
-    process.exit(1);
-    return Promise.reject(error); // for tests
-}
-
 module.exports.checkIsDir = checkIsDir;

test/coa/_index.js

@@ -175,22 +175,18 @@ describe('coa', function() {
       if (!fs.existsSync(emptyFolderPath)) {
         fs.mkdirSync(emptyFolderPath);
       }
-      replaceConsoleError();
       try {
         await runProgram(['--folder', emptyFolderPath, '--quiet']);
-      } catch {} finally {
-        restoreConsoleError();
-        expect(output).to.match(/No SVG files/);
+      } catch (error) {
+        expect(error.message).to.match(/No SVG files/);
       }
     });
 
     it('should show message when folder does not consists any svg files', async () => {
-      replaceConsoleError();
       try {
         await runProgram(['--folder', path.resolve(__dirname, 'testFolderWithNoSvg'), '--quiet'])
-      } catch {} finally {
-        restoreConsoleError();
-        expect(output).to.match(/No SVG files have been found/);
+      } catch (error) {
+        expect(error.message).to.match(/No SVG files have been found/);
       }
     });
 

test/config/_index.js

@@ -243,6 +243,27 @@ describe('config', function() {
         expect(error.message).to.match(/Invalid config file/);
       }
     });
+    it('handles config errors properly', async () => {
+      try {
+        await loadConfig(
+          null,
+          path.join(process.cwd(), './test/config/fixtures/invalid/config'),
+        );
+        expect.fail('Config is loaded successfully');
+      } catch (error) {
+        expect(error.message).to.match(/plugins is not defined/);
+      }
+    });
+    it('handles MODULE_NOT_FOUND properly', async () => {
+      try {
+        await loadConfig(
+          path.join(process.cwd(), './test/config/fixtures/module-not-found.js'),
+        );
+        expect.fail('Config is loaded successfully');
+      } catch (error) {
+        expect(error.message).to.match(/Cannot find module 'unknown-module'/);
+      }
+    });
   });
 });
 

test/config/fixtures/invalid/svgo.config.js

@@ -0,0 +1 @@
+module.exports = { plugins };

test/config/fixtures/module-not-found.js

@@ -0,0 +1,2 @@
+require('unknown-module');
+module.exports = {}