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

Refactor: don't log the stack trace of expected errors #1203

Merged
merged 1 commit into from
Jul 8, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,13 @@
import ArgumentsParser from './src/modules/argumentsParser.js';
import EndOfLifeChecker from './src/modules/endOfLifeChecker.js';
import UpdateChecker from './src/modules/updateChecker.js';
import ExpectedError from './src/types/expectedError.js';
import Options from './src/types/options.js';

// Monkey-patch 'fs' to help prevent Windows EMFILE errors
gracefulFs.gracefulify(realFs);

(async (): Promise<void> => {

Check warning on line 21 in index.ts

View workflow job for this annotation

GitHub Actions / node-lint

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
const logger = new Logger();
logger.printHeader();

Expand Down Expand Up @@ -63,13 +64,15 @@
// Start the main process
try {
new EndOfLifeChecker(logger).check(process.version);
new UpdateChecker(logger).check();

Check warning on line 67 in index.ts

View workflow job for this annotation

GitHub Actions / node-lint

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator

await new Igir(options, logger).main();
await ProgressBarCLI.stop();
} catch (error) {
await ProgressBarCLI.stop();
if (error instanceof Error && error.stack) {
if (error instanceof ExpectedError) {
logger.error(error);
} else if (error instanceof Error && error.stack) {
// Log the stack trace to help with bug reports
logger.error(error.stack);
} else {
Expand Down
9 changes: 5 additions & 4 deletions package.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import LogLevel from './src/console/logLevel.js';
import Package from './src/globals/package.js';
import FsPoly from './src/polyfill/fsPoly.js';
import ExpectedError from './src/types/expectedError.js';

interface FileFilter extends GlobOptions {
include?: string,
Expand All @@ -25,15 +26,15 @@
const include = fg.globSync(filter.include.replace(/\\/g, '/'), filter)
.map((file) => path.resolve(file));
if (include.length === 0) {
throw new Error(`glob pattern '${filter.include}' returned no paths`);
throw new ExpectedError(`glob pattern '${filter.include}' returned no paths`);
}
results = [...results, ...include];
}
if (filter.exclude) {
const exclude = new Set(fg.globSync(filter.exclude.replace(/\\/g, '/'), filter)
.map((file) => path.resolve(file)));
if (exclude.size === 0) {
throw new Error(`glob pattern '${filter.exclude}' returned no paths`);
throw new ExpectedError(`glob pattern '${filter.exclude}' returned no paths`);
}
results = results.filter((result) => !exclude.has(result));
}
Expand All @@ -41,7 +42,7 @@
return results;
};

(async (): Promise<void> => {

Check warning on line 45 in package.ts

View workflow job for this annotation

GitHub Actions / node-lint

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
const logger = new Logger(LogLevel.TRACE);

const argv = await yargs(process.argv.slice(2))
Expand All @@ -54,7 +55,7 @@
})
.check((_argv) => {
if (!_argv.input || !fs.existsSync(_argv.input)) {
throw new Error(`input directory '${_argv.input}' doesn't exist`);
throw new ExpectedError(`input directory '${_argv.input}' doesn't exist`);
}
return true;
})
Expand Down Expand Up @@ -121,7 +122,7 @@
});

if (!await FsPoly.exists(output)) {
throw new Error(`output file '${output}' doesn't exist`);
throw new ExpectedError(`output file '${output}' doesn't exist`);
}
logger.info(`Output: ${FsPoly.sizeReadable(await FsPoly.size(output))}`);

Expand Down
7 changes: 4 additions & 3 deletions src/igir.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import Timer from './timer.js';
import DAT from './types/dats/dat.js';
import Parent from './types/dats/parent.js';
import DATStatus from './types/datStatus.js';
import ExpectedError from './types/expectedError.js';
import File from './types/files/file.js';
import FileCache from './types/files/fileCache.js';
import { ChecksumBitmask } from './types/files/fileChecksums.js';
Expand Down Expand Up @@ -78,9 +79,9 @@ export default class Igir {
this.logger.trace('checking Windows for symlink permissions');
if (!await FsPoly.canSymlink(Temp.getTempDir())) {
if (!await isAdmin()) {
throw new Error(`${Package.NAME} does not have permissions to create symlinks, please try running as administrator`);
throw new ExpectedError(`${Package.NAME} does not have permissions to create symlinks, please try running as administrator`);
}
throw new Error(`${Package.NAME} does not have permissions to create symlinks`);
throw new ExpectedError(`${Package.NAME} does not have permissions to create symlinks`);
}
this.logger.trace('Windows has symlink permissions');
}
Expand Down Expand Up @@ -257,7 +258,7 @@ export default class Igir {
const progressBar = await this.logger.addProgressBar('Scanning for DATs');
let dats = await new DATScanner(this.options, progressBar).scan();
if (dats.length === 0) {
throw new Error('No valid DAT files found!');
throw new ExpectedError('No valid DAT files found!');
}

if (dats.length === 1) {
Expand Down
30 changes: 16 additions & 14 deletions src/modules/argumentsParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import Defaults from '../globals/defaults.js';
import Package from '../globals/package.js';
import ArrayPoly from '../polyfill/arrayPoly.js';
import ConsolePoly from '../polyfill/consolePoly.js';
import ExpectedError from '../types/expectedError.js';
import { ChecksumBitmask } from '../types/files/fileChecksums.js';
import ROMHeader from '../types/files/romHeader.js';
import Internationalization from '../types/internationalization.js';
Expand Down Expand Up @@ -147,13 +148,13 @@ export default class ArgumentsParser {

['extract', 'zip'].forEach((command) => {
if (checkArgv._.includes(command) && ['copy', 'move'].every((write) => !checkArgv._.includes(write))) {
throw new Error(`Command "${command}" also requires the commands copy or move`);
throw new ExpectedError(`Command "${command}" also requires the commands copy or move`);
}
});

['test', 'clean'].forEach((command) => {
if (checkArgv._.includes(command) && ['copy', 'move', 'link', 'symlink'].every((write) => !checkArgv._.includes(write))) {
throw new Error(`Command "${command}" requires one of the commands: copy, move, or link`);
throw new ExpectedError(`Command "${command}" requires one of the commands: copy, move, or link`);
}
});

Expand Down Expand Up @@ -187,7 +188,7 @@ export default class ArgumentsParser {
const needInput = ['copy', 'move', 'link', 'symlink', 'extract', 'zip', 'test', 'dir2dat', 'fixdat'].filter((command) => checkArgv._.includes(command));
if (!checkArgv.input && needInput.length > 0) {
// TODO(cememr): print help message
throw new Error(`Missing required argument for command${needInput.length !== 1 ? 's' : ''} ${needInput.join(', ')}: --input <path>`);
throw new ExpectedError(`Missing required argument for command${needInput.length !== 1 ? 's' : ''} ${needInput.join(', ')}: --input <path>`);
}
return true;
})
Expand Down Expand Up @@ -227,6 +228,7 @@ export default class ArgumentsParser {
type: 'array',
requiresArg: true,
})
// TODO(cemmer): don't allow dir2dat & --dat
.option('dat-exclude', {
group: groupDatInput,
description: 'Path(s) to DAT files or archives to exclude from processing (supports globbing)',
Expand Down Expand Up @@ -296,7 +298,7 @@ export default class ArgumentsParser {
}
const needDat = ['report'].filter((command) => checkArgv._.includes(command));
if ((!checkArgv.dat || checkArgv.dat.length === 0) && needDat.length > 0) {
throw new Error(`Missing required argument for commands ${needDat.join(', ')}: --dat`);
throw new ExpectedError(`Missing required argument for commands ${needDat.join(', ')}: --dat`);
}
return true;
})
Expand Down Expand Up @@ -369,7 +371,7 @@ export default class ArgumentsParser {
.check((checkArgv) => {
// Re-implement `implies: 'dir-letter'`, which isn't possible with a default value
if (checkArgv['dir-letter-count'] > 1 && !checkArgv['dir-letter']) {
throw new Error('Missing dependent arguments:\n dir-letter-count -> dir-letter');
throw new ExpectedError('Missing dependent arguments:\n dir-letter-count -> dir-letter');
}
return true;
})
Expand Down Expand Up @@ -427,12 +429,12 @@ export default class ArgumentsParser {
const needOutput = ['copy', 'move', 'link', 'symlink', 'extract', 'zip', 'clean'].filter((command) => checkArgv._.includes(command));
if (!checkArgv.output && needOutput.length > 0) {
// TODO(cememr): print help message
throw new Error(`Missing required argument for command${needOutput.length !== 1 ? 's' : ''} ${needOutput.join(', ')}: --output <path>`);
throw new ExpectedError(`Missing required argument for command${needOutput.length !== 1 ? 's' : ''} ${needOutput.join(', ')}: --output <path>`);
}
const needClean = ['clean-exclude', 'clean-dry-run'].filter((option) => checkArgv[option]);
if (!checkArgv._.includes('clean') && needClean.length > 0) {
// TODO(cememr): print help message
throw new Error(`Missing required command for option${needClean.length !== 1 ? 's' : ''} ${needClean.join(', ')}: clean`);
throw new ExpectedError(`Missing required command for option${needClean.length !== 1 ? 's' : ''} ${needClean.join(', ')}: clean`);
}
return true;
})
Expand All @@ -456,7 +458,7 @@ export default class ArgumentsParser {
}
const needZip = ['zip-exclude', 'zip-dat-name'].filter((option) => checkArgv[option]);
if (!checkArgv._.includes('zip') && needZip.length > 0) {
throw new Error(`Missing required command for option${needZip.length !== 1 ? 's' : ''} ${needZip.join(', ')}: zip`);
throw new ExpectedError(`Missing required command for option${needZip.length !== 1 ? 's' : ''} ${needZip.join(', ')}: zip`);
}
return true;
})
Expand Down Expand Up @@ -487,7 +489,7 @@ export default class ArgumentsParser {
}
const needLinkCommand = ['symlink'].filter((option) => checkArgv[option]);
if (!checkArgv._.includes('link') && !checkArgv._.includes('symlink') && needLinkCommand.length > 0) {
throw new Error(`Missing required command for option${needLinkCommand.length !== 1 ? 's' : ''} ${needLinkCommand.join(', ')}: link`);
throw new ExpectedError(`Missing required command for option${needLinkCommand.length !== 1 ? 's' : ''} ${needLinkCommand.join(', ')}: link`);
}
return true;
})
Expand Down Expand Up @@ -558,7 +560,7 @@ export default class ArgumentsParser {
.check((checkArgv) => {
const invalidLangs = checkArgv['filter-language']?.filter((lang) => !Internationalization.LANGUAGES.includes(lang));
if (invalidLangs !== undefined && invalidLangs.length > 0) {
throw new Error(`Invalid --filter-language language${invalidLangs.length !== 1 ? 's' : ''}: ${invalidLangs.join(', ')}`);
throw new ExpectedError(`Invalid --filter-language language${invalidLangs.length !== 1 ? 's' : ''}: ${invalidLangs.join(', ')}`);
}
return true;
})
Expand All @@ -583,7 +585,7 @@ export default class ArgumentsParser {
.check((checkArgv) => {
const invalidRegions = checkArgv['filter-region']?.filter((lang) => !Internationalization.REGION_CODES.includes(lang));
if (invalidRegions !== undefined && invalidRegions.length > 0) {
throw new Error(`Invalid --filter-region region${invalidRegions.length !== 1 ? 's' : ''}: ${invalidRegions.join(', ')}`);
throw new ExpectedError(`Invalid --filter-region region${invalidRegions.length !== 1 ? 's' : ''}: ${invalidRegions.join(', ')}`);
}
return true;
})
Expand Down Expand Up @@ -712,7 +714,7 @@ export default class ArgumentsParser {
.check((checkArgv) => {
const invalidLangs = checkArgv['prefer-language']?.filter((lang) => !Internationalization.LANGUAGES.includes(lang));
if (invalidLangs !== undefined && invalidLangs.length > 0) {
throw new Error(`Invalid --prefer-language language${invalidLangs.length !== 1 ? 's' : ''}: ${invalidLangs.join(', ')}`);
throw new ExpectedError(`Invalid --prefer-language language${invalidLangs.length !== 1 ? 's' : ''}: ${invalidLangs.join(', ')}`);
}
return true;
})
Expand All @@ -728,7 +730,7 @@ export default class ArgumentsParser {
.check((checkArgv) => {
const invalidRegions = checkArgv['prefer-region']?.filter((lang) => !Internationalization.REGION_CODES.includes(lang));
if (invalidRegions !== undefined && invalidRegions.length > 0) {
throw new Error(`Invalid --prefer-region region${invalidRegions.length !== 1 ? 's' : ''}: ${invalidRegions.join(', ')}`);
throw new ExpectedError(`Invalid --prefer-region region${invalidRegions.length !== 1 ? 's' : ''}: ${invalidRegions.join(', ')}`);
}
return true;
})
Expand Down Expand Up @@ -953,7 +955,7 @@ Example use cases:
throw err;
}
this.logger.colorizeYargs(`${_yargs.help().toString().trimEnd()}\n`);
throw new Error(msg);
throw new ExpectedError(msg);
});

const yargsArgv = yargsParser
Expand Down
9 changes: 5 additions & 4 deletions src/polyfill/fsPoly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import util from 'node:util';
import { isNotJunk } from 'junk';
import nodeDiskInfo from 'node-disk-info';

import ExpectedError from '../types/expectedError.js';
import ArrayPoly from './arrayPoly.js';

export type FsWalkCallback = (increment: number) => void;
Expand Down Expand Up @@ -103,7 +104,7 @@ export default class FsPoly {
return await fs.promises.link(target, link);
} catch (error) {
if (this.onDifferentDrives(target, link)) {
throw new Error(`can't hard link files on different drives: ${error}`);
throw new ExpectedError(`can't hard link files on different drives: ${error}`);
}
throw error;
}
Expand Down Expand Up @@ -222,7 +223,7 @@ export default class FsPoly {
return filePath;
}
}
throw new Error('failed to generate non-existent temp file');
throw new ExpectedError('failed to generate non-existent temp file');
}

static async mv(oldPath: string, newPath: string, attempt = 1): Promise<void> {
Expand Down Expand Up @@ -278,7 +279,7 @@ export default class FsPoly {

static async readlink(pathLike: PathLike): Promise<string> {
if (!await this.isSymlink(pathLike)) {
throw new Error(`can't readlink of non-symlink: ${pathLike}`);
throw new ExpectedError(`can't readlink of non-symlink: ${pathLike}`);
}
return fs.promises.readlink(pathLike);
}
Expand All @@ -293,7 +294,7 @@ export default class FsPoly {

static async realpath(pathLike: PathLike): Promise<string> {
if (!await this.exists(pathLike)) {
throw new Error(`can't get realpath of non-existent path: ${pathLike}`);
throw new ExpectedError(`can't get realpath of non-existent path: ${pathLike}`);
}
return fs.promises.realpath(pathLike);
}
Expand Down
6 changes: 4 additions & 2 deletions src/types/dats/cmpro/cmProParser.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import ExpectedError from '../../expectedError.js';

export interface DATProps extends CMProObject {
clrmamepro?: ClrMameProProps,
game?: GameProps | GameProps[],
Expand Down Expand Up @@ -172,7 +174,7 @@ export default class CMProParser {

private parseQuotedString(): string {
if (this.contents.charAt(this.pos) !== '"') {
throw new Error('invalid quoted string');
throw new ExpectedError('invalid quoted string');
}
this.pos += 1;

Expand All @@ -193,7 +195,7 @@ export default class CMProParser {
}
}

throw new Error('invalid quoted string');
throw new ExpectedError('invalid quoted string');
}

private parseUnquotedString(): string {
Expand Down
4 changes: 4 additions & 0 deletions src/types/expectedError.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/**
* An {@link Error} thrown by application code due to expected reasons such as invalid inputs.
*/
export default class ExpectedError extends Error {}
3 changes: 2 additions & 1 deletion src/types/files/archives/rar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { Mutex } from 'async-mutex';
import unrar from 'node-unrar-js';

import Defaults from '../../../globals/defaults.js';
import ExpectedError from '../../expectedError.js';
import Archive from './archive.js';
import ArchiveEntry from './archiveEntry.js';

Expand Down Expand Up @@ -73,7 +74,7 @@ export default class Rar extends Archive {
files: [entryPath.replace(/[\\/]/g, '/')],
}).files];
if (extracted.length === 0) {
throw new Error(`didn't find entry '${entryPath}'`);
throw new ExpectedError(`didn't find entry '${entryPath}'`);
}
});
}
Expand Down
5 changes: 3 additions & 2 deletions src/types/files/archives/sevenZip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { Mutex } from 'async-mutex';
import Defaults from '../../../globals/defaults.js';
import Temp from '../../../globals/temp.js';
import fsPoly from '../../../polyfill/fsPoly.js';
import ExpectedError from '../../expectedError.js';
import Archive from './archive.js';
import ArchiveEntry from './archiveEntry.js';

Expand Down Expand Up @@ -124,9 +125,9 @@ export default class SevenZip extends Archive {
if (process.platform === 'win32' && !await fsPoly.exists(tempFile)) {
const files = await fsPoly.walk(tempDir);
if (files.length === 0) {
throw new Error('failed to extract any files');
throw new ExpectedError('failed to extract any files');
} else if (files.length > 1) {
throw new Error('extracted too many files');
throw new ExpectedError('extracted too many files');
}
[tempFile] = files;
}
Expand Down
3 changes: 2 additions & 1 deletion src/types/files/archives/tar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import tar from 'tar';

import Defaults from '../../../globals/defaults.js';
import FsPoly from '../../../polyfill/fsPoly.js';
import ExpectedError from '../../expectedError.js';
import FileChecksums from '../fileChecksums.js';
import Archive from './archive.js';
import ArchiveEntry from './archiveEntry.js';
Expand Down Expand Up @@ -94,7 +95,7 @@ export default class Tar extends Archive {
},
}, [entryPath.replace(/[\\/]/g, '/')]);
if (!await FsPoly.exists(extractedFilePath)) {
throw new Error(`didn't find entry '${entryPath}'`);
throw new ExpectedError(`didn't find extracted file '${entryPath}'`);
}
}
}
3 changes: 2 additions & 1 deletion src/types/files/archives/zip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import unzipper, { Entry } from 'unzipper';
import Defaults from '../../../globals/defaults.js';
import fsPoly from '../../../polyfill/fsPoly.js';
import StreamPoly from '../../../polyfill/streamPoly.js';
import ExpectedError from '../../expectedError.js';
import File from '../file.js';
import FileChecksums, { ChecksumBitmask, ChecksumProps } from '../fileChecksums.js';
import Archive from './archive.js';
Expand Down Expand Up @@ -120,7 +121,7 @@ export default class Zip extends Archive {
.find((entryFile) => entryFile.path === entryPath.replace(/[\\/]/g, '/'));
if (!entry) {
// This should never happen, this likely means the zip file was modified after scanning
throw new Error(`didn't find entry '${entryPath}'`);
throw new ExpectedError(`didn't find entry '${entryPath}'`);
}

let stream: Entry;
Expand Down
5 changes: 3 additions & 2 deletions src/types/files/fileFactory.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import fs from 'node:fs';
import path from 'node:path';

import ExpectedError from '../expectedError.js';
import Archive from './archives/archive.js';
import ArchiveEntry from './archives/archiveEntry.js';
import ArchiveFile from './archives/archiveFile.js';
Expand Down Expand Up @@ -33,7 +34,7 @@ export default class FileFactory {
return await this.entriesFromArchiveExtension(filePath, checksumBitmask);
} catch (error) {
if (error && typeof error === 'object' && 'code' in error && error.code === 'ENOENT') {
throw new Error(`file doesn't exist: ${filePath}`);
throw new ExpectedError(`file doesn't exist: ${filePath}`);
}
if (typeof error === 'string') {
throw new Error(error);
Expand Down Expand Up @@ -87,7 +88,7 @@ export default class FileFactory {
} else if (ZipX.getExtensions().some((ext) => filePath.toLowerCase().endsWith(ext))) {
archive = new ZipX(filePath);
} else {
throw new Error(`unknown archive type: ${path.extname(filePath)}`);
throw new ExpectedError(`unknown archive type: ${path.extname(filePath)}`);
}

return FileCache.getOrComputeEntries(archive, checksumBitmask);
Expand Down
Loading
Loading