Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix reporting of config errors #1342

Merged
merged 5 commits into from Feb 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion 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);
});
38 changes: 21 additions & 17 deletions lib/svgo-node.js
Expand Up @@ -10,19 +10,26 @@ const {

const importConfig = async configFile => {
try {
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;
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;
};

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)) {
Expand All @@ -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()) {
return await importConfig(file);
}
} catch {}
const file = path.join(dir, "svgo.config.js");
if (await isFile(file)) {
return await importConfig(file);
}
const parent = path.dirname(dir);
if (dir === parent) {
return null;
Expand Down
31 changes: 8 additions & 23 deletions lib/svgo/coa.js
Expand Up @@ -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.
Expand Down Expand Up @@ -97,18 +96,14 @@ 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);
const loadedConfig = await loadConfig(opts.config);
if (loadedConfig != null) {
config = loadedConfig;
}

// --quiet
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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;
12 changes: 4 additions & 8 deletions test/coa/_index.js
Expand Up @@ -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/);
}
});

Expand Down
21 changes: 21 additions & 0 deletions test/config/_index.js
Expand Up @@ -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'/);
}
});
});
});

Expand Down
1 change: 1 addition & 0 deletions test/config/fixtures/invalid/svgo.config.js
@@ -0,0 +1 @@
module.exports = { plugins };
2 changes: 2 additions & 0 deletions test/config/fixtures/module-not-found.js
@@ -0,0 +1,2 @@
require('unknown-module');
module.exports = {}