Skip to content

Commit e41dc3e

Browse files
committed
refactor(@angular/cli): assert catch clause variable type before usage
Prepares the `@angular/cli` package for the eventual change of enabling the TypeScript `useUnknownInCatchVariables` option. This option provides additional code safety by ensuring that the catch clause variable is the proper type before attempting to access its properties. Similar changes will be needed in the other packages in the repository prior to enabling `useUnknownInCatchVariables`.
1 parent 7ce88c7 commit e41dc3e

File tree

12 files changed

+57
-8
lines changed

12 files changed

+57
-8
lines changed

packages/angular/cli/lib/cli/index.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ export default async function (options: { cliArgs: string[] }) {
9292
} catch (e) {
9393
logger.fatal(
9494
`An unhandled exception occurred: ${err.message}\n` +
95-
`Fatal error writing debug log file: ${e.message}`,
95+
`Fatal error writing debug log file: ${e}`,
9696
);
9797
if (err.stack) {
9898
logger.fatal(err.stack);
@@ -105,9 +105,7 @@ export default async function (options: { cliArgs: string[] }) {
105105
} else if (typeof err === 'number') {
106106
// Log nothing.
107107
} else {
108-
logger.fatal(
109-
`An unexpected error occurred: ${'toString' in err ? err.toString() : JSON.stringify(err)}`,
110-
);
108+
logger.fatal(`An unexpected error occurred: ${err}`);
111109
}
112110

113111
return 1;

packages/angular/cli/src/analytics/analytics.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@ import { analytics, json, tags } from '@angular-devkit/core';
1010
import debug from 'debug';
1111
import { v4 as uuidV4 } from 'uuid';
1212
import { colors } from '../utilities/color';
13-
import { AngularWorkspace, getWorkspace } from '../utilities/config';
13+
import { getWorkspace } from '../utilities/config';
1414
import { analyticsDisabled, analyticsShareDisabled } from '../utilities/environment-options';
15+
import { assertIsError } from '../utilities/error';
1516
import { isTTY } from '../utilities/tty';
1617
import { VERSION } from '../utilities/version';
1718
import { AnalyticsCollector } from './analytics-collector';
@@ -190,6 +191,7 @@ export async function getAnalytics(
190191
return new AnalyticsCollector(AnalyticsProperties.AngularCliDefault, uid);
191192
}
192193
} catch (err) {
194+
assertIsError(err);
193195
analyticsDebug('Error happened during reading of analytics config: %s', err.message);
194196

195197
return undefined;
@@ -222,6 +224,7 @@ export async function getSharedAnalytics(): Promise<AnalyticsCollector | undefin
222224
return new AnalyticsCollector(analyticsConfig.tracking, analyticsConfig.uuid);
223225
}
224226
} catch (err) {
227+
assertIsError(err);
225228
analyticsDebug('Error happened during reading of analytics sharing config: %s', err.message);
226229

227230
return undefined;

packages/angular/cli/src/command-builder/architect-base-command-module.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { spawnSync } from 'child_process';
1616
import { existsSync } from 'fs';
1717
import { resolve } from 'path';
1818
import { isPackageNameSafeForAnalytics } from '../analytics/analytics';
19+
import { assertIsError } from '../utilities/error';
1920
import { askConfirmation, askQuestion } from '../utilities/prompt';
2021
import { isTTY } from '../utilities/tty';
2122
import {
@@ -47,6 +48,8 @@ export abstract class ArchitectBaseCommandModule<T extends object>
4748
try {
4849
builderName = await architectHost.getBuilderNameForTarget(target);
4950
} catch (e) {
51+
assertIsError(e);
52+
5053
return this.onMissingTarget(e.message);
5154
}
5255

@@ -115,6 +118,7 @@ export abstract class ArchitectBaseCommandModule<T extends object>
115118
try {
116119
builderDesc = await architectHost.resolveBuilder(builderConf);
117120
} catch (e) {
121+
assertIsError(e);
118122
if (e.code === 'MODULE_NOT_FOUND') {
119123
this.warnOnMissingNodeModules();
120124
throw new CommandModuleError(`Could not find the '${builderConf}' builder's node package.`);

packages/angular/cli/src/command-builder/command-runner.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import { UpdateCommandModule } from '../commands/update/cli';
3030
import { VersionCommandModule } from '../commands/version/cli';
3131
import { colors } from '../utilities/color';
3232
import { AngularWorkspace, getWorkspace } from '../utilities/config';
33+
import { assertIsError } from '../utilities/error';
3334
import { PackageManagerUtils } from '../utilities/package-manager';
3435
import { CommandContext, CommandModuleError, CommandScope } from './command-module';
3536
import { addCommandModuleToYargs, demandCommandFailureMessage } from './utilities/command';
@@ -84,6 +85,7 @@ export async function runCommand(args: string[], logger: logging.Logger): Promis
8485
getWorkspace('global'),
8586
]);
8687
} catch (e) {
88+
assertIsError(e);
8789
logger.fatal(e.message);
8890

8991
return 1;

packages/angular/cli/src/command-builder/schematics-command-module.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import type { CheckboxQuestion, Question } from 'inquirer';
1717
import { relative, resolve } from 'path';
1818
import { Argv } from 'yargs';
1919
import { getProjectByCwd, getSchematicDefaults } from '../utilities/config';
20+
import { assertIsError } from '../utilities/error';
2021
import { memoize } from '../utilities/memoize';
2122
import { isTTY } from '../utilities/tty';
2223
import {
@@ -364,6 +365,7 @@ export abstract class SchematicsCommandModule
364365
// "See above" because we already printed the error.
365366
logger.fatal('The Schematic workflow failed. See above.');
366367
} else {
368+
assertIsError(err);
367369
logger.fatal(err.message);
368370
}
369371

packages/angular/cli/src/command-builder/utilities/schematic-engine-host.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { parse as parseJson } from 'jsonc-parser';
1313
import nodeModule from 'module';
1414
import { dirname, resolve } from 'path';
1515
import { Script } from 'vm';
16+
import { assertIsError } from '../../utilities/error';
1617

1718
/**
1819
* Environment variable to control schematic package redirection
@@ -153,6 +154,7 @@ function wrap(
153154
try {
154155
return schematicRequire(id);
155156
} catch (e) {
157+
assertIsError(e);
156158
if (e.code !== 'MODULE_NOT_FOUND') {
157159
throw e;
158160
}

packages/angular/cli/src/commands/add/cli.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
SchematicsCommandModule,
2525
} from '../../command-builder/schematics-command-module';
2626
import { colors } from '../../utilities/color';
27+
import { assertIsError } from '../../utilities/error';
2728
import {
2829
NgAddSaveDependency,
2930
PackageManifest,
@@ -113,6 +114,7 @@ export class AddCommandModule
113114
try {
114115
packageIdentifier = npa(collection);
115116
} catch (e) {
117+
assertIsError(e);
116118
logger.error(e.message);
117119

118120
return 1;
@@ -151,6 +153,7 @@ export class AddCommandModule
151153
verbose,
152154
});
153155
} catch (e) {
156+
assertIsError(e);
154157
spinner.fail(`Unable to load package information from registry: ${e.message}`);
155158

156159
return 1;
@@ -237,6 +240,7 @@ export class AddCommandModule
237240
spinner.succeed(`Package information loaded.`);
238241
}
239242
} catch (e) {
243+
assertIsError(e);
240244
spinner.fail(`Unable to fetch package information for '${packageIdentifier}': ${e.message}`);
241245

242246
return 1;
@@ -341,6 +345,7 @@ export class AddCommandModule
341345

342346
return true;
343347
} catch (e) {
348+
assertIsError(e);
344349
if (e.code !== 'MODULE_NOT_FOUND') {
345350
throw e;
346351
}

packages/angular/cli/src/commands/completion/cli.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { CommandModule, CommandModuleImplementation } from '../../command-builde
1212
import { addCommandModuleToYargs } from '../../command-builder/utilities/command';
1313
import { colors } from '../../utilities/color';
1414
import { hasGlobalCliInstall, initializeAutocomplete } from '../../utilities/completion';
15+
import { assertIsError } from '../../utilities/error';
1516

1617
export class CompletionCommandModule extends CommandModule implements CommandModuleImplementation {
1718
command = 'completion';
@@ -27,6 +28,7 @@ export class CompletionCommandModule extends CommandModule implements CommandMod
2728
try {
2829
rcFile = await initializeAutocomplete();
2930
} catch (err) {
31+
assertIsError(err);
3032
this.context.logger.error(err.message);
3133

3234
return 1;

packages/angular/cli/src/commands/update/cli.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import { UnsuccessfulWorkflowExecution } from '@angular-devkit/schematics';
1010
import { NodeWorkflow } from '@angular-devkit/schematics/tools';
11-
import { execSync, spawnSync } from 'child_process';
11+
import { SpawnSyncReturns, execSync, spawnSync } from 'child_process';
1212
import { existsSync, promises as fs } from 'fs';
1313
import npa from 'npm-package-arg';
1414
import pickManifest from 'npm-pick-manifest';
@@ -27,6 +27,7 @@ import { SchematicEngineHost } from '../../command-builder/utilities/schematic-e
2727
import { subscribeToWorkflow } from '../../command-builder/utilities/schematic-workflow';
2828
import { colors } from '../../utilities/color';
2929
import { disableVersionCheck } from '../../utilities/environment-options';
30+
import { assertIsError } from '../../utilities/error';
3031
import { writeErrorToLogFile } from '../../utilities/log-file';
3132
import {
3233
PackageIdentifier,
@@ -211,6 +212,7 @@ export class UpdateCommandModule extends CommandModule<UpdateCommandArgs> {
211212

212213
packages.push(packageIdentifier as PackageIdentifier);
213214
} catch (e) {
215+
assertIsError(e);
214216
logger.error(e.message);
215217

216218
return 1;
@@ -281,6 +283,7 @@ export class UpdateCommandModule extends CommandModule<UpdateCommandArgs> {
281283
if (e instanceof UnsuccessfulWorkflowExecution) {
282284
logger.error(`${colors.symbols.cross} Migration failed. See above for further details.\n`);
283285
} else {
286+
assertIsError(e);
284287
const logPath = writeErrorToLogFile(e);
285288
logger.fatal(
286289
`${colors.symbols.cross} Migration failed: ${e.message}\n` +
@@ -484,6 +487,7 @@ export class UpdateCommandModule extends CommandModule<UpdateCommandArgs> {
484487
try {
485488
migrations = require.resolve(migrations, { paths: [packagePath] });
486489
} catch (e) {
490+
assertIsError(e);
487491
if (e.code === 'MODULE_NOT_FOUND') {
488492
logger.error('Migrations for package were not found.');
489493
} else {
@@ -577,6 +581,7 @@ export class UpdateCommandModule extends CommandModule<UpdateCommandArgs> {
577581
verbose: options.verbose,
578582
});
579583
} catch (e) {
584+
assertIsError(e);
580585
logger.error(`Error fetching metadata for '${packageName}': ` + e.message);
581586

582587
return 1;
@@ -593,6 +598,7 @@ export class UpdateCommandModule extends CommandModule<UpdateCommandArgs> {
593598
try {
594599
manifest = pickManifest(metadata, requestIdentifier.fetchSpec);
595600
} catch (e) {
601+
assertIsError(e);
596602
if (e.code === 'ETARGET') {
597603
// If not found and next was used and user did not provide a specifier, try latest.
598604
// Package may not have a next tag.
@@ -604,6 +610,7 @@ export class UpdateCommandModule extends CommandModule<UpdateCommandArgs> {
604610
try {
605611
manifest = pickManifest(metadata, 'latest');
606612
} catch (e) {
613+
assertIsError(e);
607614
if (e.code !== 'ETARGET' && e.code !== 'ENOVERSIONS') {
608615
throw e;
609616
}
@@ -746,6 +753,7 @@ export class UpdateCommandModule extends CommandModule<UpdateCommandArgs> {
746753
}),
747754
);
748755
} catch (e) {
756+
assertIsError(e);
749757
if (e.code === 'MODULE_NOT_FOUND') {
750758
// Fallback to trying to resolve the package's main entry point
751759
packagePath = require.resolve(migration.package, { paths: [this.context.root] });
@@ -754,6 +762,7 @@ export class UpdateCommandModule extends CommandModule<UpdateCommandArgs> {
754762
}
755763
}
756764
} catch (e) {
765+
assertIsError(e);
757766
if (e.code === 'MODULE_NOT_FOUND') {
758767
logVerbose(e.toString());
759768
logger.error(
@@ -781,6 +790,7 @@ export class UpdateCommandModule extends CommandModule<UpdateCommandArgs> {
781790
try {
782791
migrations = require.resolve(migration.collection, { paths: [packagePath] });
783792
} catch (e) {
793+
assertIsError(e);
784794
if (e.code === 'MODULE_NOT_FOUND') {
785795
logger.error(`Migrations for package (${migration.package}) were not found.`);
786796
} else {
@@ -821,7 +831,7 @@ export class UpdateCommandModule extends CommandModule<UpdateCommandArgs> {
821831
try {
822832
commitNeeded = hasChangesToCommit();
823833
} catch (err) {
824-
logger.error(` Failed to read Git tree:\n${err.stderr}`);
834+
logger.error(` Failed to read Git tree:\n${(err as SpawnSyncReturns<string>).stderr}`);
825835

826836
return false;
827837
}
@@ -836,7 +846,9 @@ export class UpdateCommandModule extends CommandModule<UpdateCommandArgs> {
836846
try {
837847
createCommit(message);
838848
} catch (err) {
839-
logger.error(`Failed to commit update (${message}):\n${err.stderr}`);
849+
logger.error(
850+
`Failed to commit update (${message}):\n${(err as SpawnSyncReturns<string>).stderr}`,
851+
);
840852

841853
return false;
842854
}

packages/angular/cli/src/commands/update/schematic/index.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { Rule, SchematicContext, SchematicsException, Tree } from '@angular-devk
1111
import * as npa from 'npm-package-arg';
1212
import type { Manifest } from 'pacote';
1313
import * as semver from 'semver';
14+
import { assertIsError } from '../../../utilities/error';
1415
import {
1516
NgPackageManifestProperties,
1617
NpmRepositoryPackageJson,
@@ -269,6 +270,7 @@ function _performUpdate(
269270
try {
270271
packageJson = JSON.parse(packageJsonContent.toString()) as JsonSchemaForNpmPackageJsonFiles;
271272
} catch (e) {
273+
assertIsError(e);
272274
throw new SchematicsException('package.json could not be parsed: ' + e.message);
273275
}
274276

@@ -772,6 +774,7 @@ function _getAllDependencies(tree: Tree): Array<readonly [string, VersionRange]>
772774
try {
773775
packageJson = JSON.parse(packageJsonContent.toString()) as JsonSchemaForNpmPackageJsonFiles;
774776
} catch (e) {
777+
assertIsError(e);
775778
throw new SchematicsException('package.json could not be parsed: ' + e.message);
776779
}
777780

packages/angular/cli/src/utilities/completion.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { colors } from '../utilities/color';
1515
import { getWorkspace } from '../utilities/config';
1616
import { forceAutocomplete } from '../utilities/environment-options';
1717
import { isTTY } from '../utilities/tty';
18+
import { assertIsError } from './error';
1819

1920
/** Interface for the autocompletion configuration stored in the global workspace. */
2021
interface CompletionConfig {
@@ -64,6 +65,7 @@ Ok, you won't be prompted again. Should you change your mind, the following comm
6465
try {
6566
rcFile = await initializeAutocomplete();
6667
} catch (err) {
68+
assertIsError(err);
6769
// Failed to set up autocompeletion, log the error and abort.
6870
logger.error(err.message);
6971

@@ -247,6 +249,7 @@ export async function initializeAutocomplete(): Promise<string> {
247249
'\n\n# Load Angular CLI autocompletion.\nsource <(ng completion script)\n',
248250
);
249251
} catch (err) {
252+
assertIsError(err);
250253
throw new Error(`Failed to append autocompletion setup to \`${rcFile}\`:\n${err.message}`);
251254
}
252255

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import assert from 'assert';
10+
11+
export function assertIsError(value: unknown): asserts value is Error & { code?: string } {
12+
assert(value instanceof Error, 'catch clause variable is not an Error instance');
13+
}

0 commit comments

Comments
 (0)