Skip to content

Commit 7431d1c

Browse files
clydinalan-agius4
authored andcommitted
refactor(@angular-devkit/build-angular): assert catch clause variable type before usage
Prepares the `@angular-devkit/build-angular` 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 2636262 commit 7431d1c

File tree

14 files changed

+49
-13
lines changed

14 files changed

+49
-13
lines changed

‎packages/angular_devkit/build_angular/src/builders/app-shell/index.ts

+2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { JsonObject } from '@angular-devkit/core';
1616
import * as fs from 'fs';
1717
import * as path from 'path';
1818
import { normalizeOptimization } from '../../utils';
19+
import { assertIsError } from '../../utils/error';
1920
import { InlineCriticalCssProcessor } from '../../utils/index-file/inline-critical-css';
2021
import { augmentAppWithServiceWorker } from '../../utils/service-worker';
2122
import { Spinner } from '../../utils/spinner';
@@ -199,6 +200,7 @@ async function _appShellBuilder(
199200
return result;
200201
} catch (err) {
201202
spinner?.fail('Application shell generation failed.');
203+
assertIsError(err);
202204

203205
return { success: false, error: err.message };
204206
} finally {

‎packages/angular_devkit/build_angular/src/builders/browser-esbuild/index.ts

+3-2
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { promises as fs } from 'fs';
1313
import * as path from 'path';
1414
import { NormalizedOptimizationOptions, deleteOutputDir } from '../../utils';
1515
import { copyAssets } from '../../utils/copy-assets';
16+
import { assertIsError } from '../../utils/error';
1617
import { FileInfo } from '../../utils/index-file/augment-index-html';
1718
import { IndexHtmlGenerator } from '../../utils/index-file/index-html-generator';
1819
import { generateEntryPoints } from '../../utils/package-chunk-sort';
@@ -134,8 +135,8 @@ export async function execute(
134135
try {
135136
await fs.mkdir(outputPath, { recursive: true });
136137
} catch (e) {
137-
const reason = 'message' in e ? e.message : 'Unknown error';
138-
context.logger.error('Unable to create output directory: ' + reason);
138+
assertIsError(e);
139+
context.logger.error('Unable to create output directory: ' + e.message);
139140

140141
return { success: false };
141142
}

‎packages/angular_devkit/build_angular/src/builders/browser/index.ts

+2
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
} from '../../utils/bundle-calculator';
3030
import { colors } from '../../utils/color';
3131
import { copyAssets } from '../../utils/copy-assets';
32+
import { assertIsError } from '../../utils/error';
3233
import { i18nInlineEmittedFiles } from '../../utils/i18n-inlining';
3334
import { I18nOptions } from '../../utils/i18n-options';
3435
import { FileInfo } from '../../utils/index-file/augment-index-html';
@@ -278,6 +279,7 @@ export function buildWebpackBrowser(
278279
spinner.succeed('Copying assets complete.');
279280
} catch (err) {
280281
spinner.fail(colors.redBright('Copying of assets failed.'));
282+
assertIsError(err);
281283

282284
return { success: false, error: 'Unable to copy assets: ' + err.message };
283285
}

‎packages/angular_devkit/build_angular/src/builders/protractor/index.ts

+2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { json, tags } from '@angular-devkit/core';
1616
import { resolve } from 'path';
1717
import * as url from 'url';
1818
import { runModuleAsObservableFork } from '../../utils';
19+
import { assertIsError } from '../../utils/error';
1920
import { DevServerBuilderOptions } from '../dev-server/index';
2021
import { Schema as ProtractorBuilderOptions } from './schema';
2122

@@ -56,6 +57,7 @@ async function updateWebdriver() {
5657

5758
path = require.resolve(webdriverDeepImport, { paths: [protractorPath] });
5859
} catch (error) {
60+
assertIsError(error);
5961
if (error.code !== 'MODULE_NOT_FOUND') {
6062
throw error;
6163
}

‎packages/angular_devkit/build_angular/src/sass/worker.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@ parentPort.on('message', ({ id, hasImporter, options }: RenderRequestMessage) =>
6060
const result = renderSync(options);
6161

6262
parentPort?.postMessage({ id, result });
63-
} catch (error) {
63+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
64+
} catch (error: any) {
6465
// Needed because V8 will only serialize the message and stack properties of an Error instance.
6566
const { formatted, file, line, column, message, stack } = error;
6667
parentPort?.postMessage({ id, error: { formatted, file, line, column, message, stack } });

‎packages/angular_devkit/build_angular/src/testing/jasmine-helpers.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ export function expectFile<T>(path: string, harness: BuilderHarness<T>): Harness
9595
try {
9696
return expect(harness.readFile(path)).withContext(`With file content for '${path}'`);
9797
} catch (e) {
98-
if (e.code !== 'ENOENT') {
98+
if ((e as NodeJS.ErrnoException).code !== 'ENOENT') {
9999
throw e;
100100
}
101101

@@ -112,7 +112,7 @@ export function expectFile<T>(path: string, harness: BuilderHarness<T>): Harness
112112
`With file size for '${path}'`,
113113
);
114114
} catch (e) {
115-
if (e.code !== 'ENOENT') {
115+
if ((e as NodeJS.ErrnoException).code !== 'ENOENT') {
116116
throw e;
117117
}
118118

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+
}

‎packages/angular_devkit/build_angular/src/utils/i18n-inlining.ts

+3
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import * as fs from 'fs';
1212
import * as path from 'path';
1313
import { BundleActionExecutor } from './action-executor';
1414
import { copyAssets } from './copy-assets';
15+
import { assertIsError } from './error';
1516
import { I18nOptions } from './i18n-options';
1617
import { InlineOptions } from './process-bundle';
1718
import { Spinner } from './spinner';
@@ -52,6 +53,7 @@ function emittedFilesToInlineOptions(
5253
action.map = fs.readFileSync(originalMapPath, 'utf8');
5354
originalFiles.push(originalMapPath);
5455
} catch (err) {
56+
assertIsError(err);
5557
if (err.code !== 'ENOENT') {
5658
throw err;
5759
}
@@ -121,6 +123,7 @@ export async function i18nInlineEmittedFiles(
121123
'',
122124
);
123125
} catch (err) {
126+
assertIsError(err);
124127
spinner.fail('Localized bundle generation failed: ' + err.message);
125128

126129
return false;

‎packages/angular_devkit/build_angular/src/utils/process-bundle.ts

+9-8
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import * as fs from 'fs';
2121
import * as path from 'path';
2222
import { workerData } from 'worker_threads';
2323
import { allowMinify, shouldBeautify } from './environment-options';
24+
import { assertIsError } from './error';
2425
import { I18nOptions } from './i18n-options';
2526
import { loadEsmModule } from './load-esm';
2627

@@ -151,14 +152,14 @@ export async function inlineLocales(options: InlineOptions) {
151152
filename: options.filename,
152153
});
153154
} catch (error) {
154-
if (error.message) {
155-
// Make the error more readable.
156-
// Same errors will contain the full content of the file as the error message
157-
// Which makes it hard to find the actual error message.
158-
const index = error.message.indexOf(')\n');
159-
const msg = index !== -1 ? error.message.slice(0, index + 1) : error.message;
160-
throw new Error(`${msg}\nAn error occurred inlining file "${options.filename}"`);
161-
}
155+
assertIsError(error);
156+
157+
// Make the error more readable.
158+
// Same errors will contain the full content of the file as the error message
159+
// Which makes it hard to find the actual error message.
160+
const index = error.message.indexOf(')\n');
161+
const msg = index !== -1 ? error.message.slice(0, index + 1) : error.message;
162+
throw new Error(`${msg}\nAn error occurred inlining file "${options.filename}"`);
162163
}
163164

164165
if (!ast) {

‎packages/angular_devkit/build_angular/src/utils/service-worker.ts

+3
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import * as crypto from 'crypto';
1111
import { createReadStream, promises as fs, constants as fsConstants } from 'fs';
1212
import * as path from 'path';
1313
import { pipeline } from 'stream';
14+
import { assertIsError } from './error';
1415
import { loadEsmModule } from './load-esm';
1516

1617
class CliFilesystem implements Filesystem {
@@ -78,6 +79,7 @@ export async function augmentAppWithServiceWorker(
7879
const configurationData = await fs.readFile(configPath, 'utf-8');
7980
config = JSON.parse(configurationData) as Config;
8081
} catch (error) {
82+
assertIsError(error);
8183
if (error.code === 'ENOENT') {
8284
throw new Error(
8385
'Error: Expected to find an ngsw-config.json configuration file' +
@@ -130,6 +132,7 @@ export async function augmentAppWithServiceWorker(
130132
fsConstants.COPYFILE_FICLONE,
131133
);
132134
} catch (error) {
135+
assertIsError(error);
133136
if (error.code !== 'ENOENT') {
134137
throw error;
135138
}

‎packages/angular_devkit/build_angular/src/webpack/configs/dev-server.ts

+2
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { URL, pathToFileURL } from 'url';
1313
import { Configuration, RuleSetRule } from 'webpack';
1414
import { Configuration as DevServerConfiguration } from 'webpack-dev-server';
1515
import { WebpackConfigOptions, WebpackDevServerOptions } from '../../utils/build-options';
16+
import { assertIsError } from '../../utils/error';
1617
import { loadEsmModule } from '../../utils/load-esm';
1718
import { getIndexOutputFile } from '../../utils/webpack-browser-config';
1819
import { HmrLoader } from '../plugins/hmr/hmr-loader';
@@ -193,6 +194,7 @@ async function addProxyConfig(root: string, proxyConfig: string | undefined) {
193194
try {
194195
return require(proxyPath);
195196
} catch (e) {
197+
assertIsError(e);
196198
if (e.code === 'ERR_REQUIRE_ESM') {
197199
// Load the ESM configuration file using the TypeScript dynamic import workaround.
198200
// Once TypeScript provides support for keeping the dynamic import this workaround can be

‎packages/angular_devkit/build_angular/src/webpack/plugins/index-html-webpack-plugin.ts

+2
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import { basename, dirname, extname } from 'path';
1010
import { Compilation, Compiler, sources } from 'webpack';
11+
import { assertIsError } from '../../utils/error';
1112
import { FileInfo } from '../../utils/index-file/augment-index-html';
1213
import {
1314
IndexHtmlGenerator,
@@ -77,6 +78,7 @@ export class IndexHtmlWebpackPlugin extends IndexHtmlGenerator {
7778
warnings.forEach((msg) => addWarning(this.compilation, msg));
7879
errors.forEach((msg) => addError(this.compilation, msg));
7980
} catch (error) {
81+
assertIsError(error);
8082
addError(this.compilation, error.message);
8183
}
8284
};

‎packages/angular_devkit/build_angular/src/webpack/plugins/json-stats-plugin.ts

+2
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import { createWriteStream, promises as fsPromises } from 'fs';
1010
import { dirname } from 'path';
1111
import { Compiler } from 'webpack';
12+
import { assertIsError } from '../../utils/error';
1213

1314
import { addError } from '../../utils/webpack-diagnostics';
1415

@@ -29,6 +30,7 @@ export class JsonStatsPlugin {
2930
.on('error', reject),
3031
);
3132
} catch (error) {
33+
assertIsError(error);
3234
addError(
3335
stats.compilation,
3436
`Unable to write stats file: ${error.message || 'unknown error'}`,

‎packages/angular_devkit/build_angular/src/webpack/plugins/postcss-cli-resources.ts

+2
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { interpolateName } from 'loader-utils';
1010
import * as path from 'path';
1111
import { Declaration, Plugin } from 'postcss';
1212
import * as url from 'url';
13+
import { assertIsError } from '../../utils/error';
1314

1415
function wrapUrl(url: string): string {
1516
let wrappedUrl;
@@ -172,6 +173,7 @@ export default function (options?: PostcssCliResourcesOptions): Plugin {
172173
try {
173174
processedUrl = await process(originalUrl, context, resourceCache);
174175
} catch (err) {
176+
assertIsError(err);
175177
loader.emitError(decl.error(err.message, { word: originalUrl }));
176178
continue;
177179
}

0 commit comments

Comments
 (0)