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

emmercm · Jul 8, 2024 · ec8d897 · ec8d897
1 parent 20a9ea7
commit ec8d897
Show file tree

Hide file tree

Showing 24 changed files with 99 additions and 68 deletions.
diff --git a/index.ts b/index.ts
@@ -12,6 +12,7 @@ import Igir from './src/igir.js';
 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
@@ -69,7 +70,9 @@ gracefulFs.gracefulify(realFs);
     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 {

diff --git a/package.ts b/package.ts
@@ -12,6 +12,7 @@ import Logger from './src/console/logger.js';
 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,
@@ -25,15 +26,15 @@ const fileFilter = (filters: FileFilter[]): string[] => {
       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));
     }
@@ -54,7 +55,7 @@ const fileFilter = (filters: FileFilter[]): string[] => {
     })
     .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;
     })
@@ -121,7 +122,7 @@ const fileFilter = (filters: FileFilter[]): string[] => {
   });
 
   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))}`);
 

diff --git a/src/igir.ts b/src/igir.ts
@@ -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';
@@ -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');
     }
@@ -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) {

diff --git a/src/modules/argumentsParser.ts b/src/modules/argumentsParser.ts
@@ -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';
@@ -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`);
             }
           });
 
@@ -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;
       })
@@ -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)',
@@ -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;
       })
@@ -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;
       })
@@ -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;
       })
@@ -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;
       })
@@ -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;
       })
@@ -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;
       })
@@ -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;
       })
@@ -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;
       })
@@ -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;
       })
@@ -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

diff --git a/src/polyfill/fsPoly.ts b/src/polyfill/fsPoly.ts
@@ -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;
@@ -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;
     }
@@ -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> {
@@ -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);
   }
@@ -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);
   }

diff --git a/src/types/dats/cmpro/cmProParser.ts b/src/types/dats/cmpro/cmProParser.ts
@@ -1,3 +1,5 @@
+import ExpectedError from '../../expectedError.js';
+
 export interface DATProps extends CMProObject {
   clrmamepro?: ClrMameProProps,
   game?: GameProps | GameProps[],
@@ -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;
 
@@ -193,7 +195,7 @@ export default class CMProParser {
       }
     }
 
-    throw new Error('invalid quoted string');
+    throw new ExpectedError('invalid quoted string');
   }
 
   private parseUnquotedString(): string {

diff --git a/src/types/expectedError.ts b/src/types/expectedError.ts
@@ -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 {}
diff --git a/src/types/files/archives/rar.ts b/src/types/files/archives/rar.ts
@@ -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';
 
@@ -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}'`);
       }
     });
   }

diff --git a/src/types/files/archives/sevenZip.ts b/src/types/files/archives/sevenZip.ts
@@ -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';
 
@@ -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;
       }

diff --git a/src/types/files/archives/tar.ts b/src/types/files/archives/tar.ts
@@ -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';
@@ -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}'`);
     }
   }
 }
diff --git a/src/types/files/archives/zip.ts b/src/types/files/archives/zip.ts
@@ -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';
@@ -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;

diff --git a/src/types/files/fileFactory.ts b/src/types/files/fileFactory.ts
@@ -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';
@@ -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);
@@ -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);