Skip to content

Commit

Permalink
fix: error messages are getting lost (backport #1263) (#1264)
Browse files Browse the repository at this point in the history
# Backport

This will backport the following commits from `main` to
`maintenance/v5.3`:
- [fix: error messages are getting lost
(#1263)](#1263)

<!--- Backport version: 9.5.1 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

Co-authored-by: Momo Kornher <kornherm@amazon.co.uk>
  • Loading branch information
aws-cdk-automation and mrgrain authored Aug 21, 2024
1 parent c62433b commit 40cf22e
Show file tree
Hide file tree
Showing 16 changed files with 139 additions and 1,192 deletions.
7 changes: 4 additions & 3 deletions src/assembler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { DeprecatedRemover } from './transforms/deprecated-remover';
import { DeprecationWarningsInjector } from './transforms/deprecation-warnings';
import { RuntimeTypeInfoInjector } from './transforms/runtime-info';
import { combinedTransformers } from './transforms/utils';
import { JsiiError } from './utils';
import { Validator } from './validator';
import { SHORT_VERSION, VERSION } from './version';
import { enabledWarnings } from './warnings';
Expand Down Expand Up @@ -82,7 +83,7 @@ export class Assembler implements Emitter {
let allowlistedDeprecations: Set<string> | undefined;
if (options.stripDeprecatedAllowListFile) {
if (!fs.existsSync(options.stripDeprecatedAllowListFile)) {
throw new Error(`--strip-deprecated file not found: ${options.stripDeprecatedAllowListFile}`);
throw new JsiiError(`--strip-deprecated file not found: ${options.stripDeprecatedAllowListFile}`);
}
allowlistedDeprecations = new Set<string>(
fs.readFileSync(options.stripDeprecatedAllowListFile, 'utf8').split('\n'),
Expand Down Expand Up @@ -1348,7 +1349,7 @@ export class Assembler implements Emitter {
private _getTypeFromTypeNode(t: ts.TypeNode) {
const type = this._typeChecker.getTypeFromTypeNode(t);
if (isErrorType(type)) {
throw new Error(
throw new JsiiError(
`Unable to resolve type: ${t.getFullText()}. This typically happens if something is wrong with your dependency closure.`,
);
}
Expand Down Expand Up @@ -1484,7 +1485,7 @@ export class Assembler implements Emitter {
symbol = getSymbolFromDeclaration(decl, this._typeChecker);
}
if (!decl || !symbol || !ts.isEnumDeclaration(decl)) {
throw new Error(`Unable to resolve enum declaration for ${type.symbol.name}!`);
throw new JsiiError(`Unable to resolve enum declaration for ${type.symbol.name}!`);
}

if (_hasInternalJsDocTag(symbol)) {
Expand Down
3 changes: 2 additions & 1 deletion src/common/find-utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as fs from 'node:fs';
import * as path from 'node:path';
import { JsiiError } from '../utils';

/**
* Find a directory up the tree from a starting directory matching a condition
Expand Down Expand Up @@ -55,7 +56,7 @@ export function findDependencyDirectory(dependencyName: string, searchStart: str
const depPkgJsonPath = findPackageJsonUp(dependencyName, path.dirname(entryPoint));

if (!depPkgJsonPath) {
throw new Error(`Could not find dependency '${dependencyName}' from '${searchStart}'`);
throw new JsiiError(`Could not find dependency '${dependencyName}' from '${searchStart}'`);
}

return depPkgJsonPath;
Expand Down
8 changes: 4 additions & 4 deletions src/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export class Compiler implements Emitter {

public constructor(private readonly options: CompilerOptions) {
if (options.generateTypeScriptConfig != null && options.typeScriptConfig != null) {
throw new Error(
throw new utils.JsiiError(
'Cannot use `generateTypeScriptConfig` and `typeScriptConfig` together. Provide only one of them.',
);
}
Expand Down Expand Up @@ -214,7 +214,7 @@ export class Compiler implements Emitter {
);
}

throw new Error(
throw new utils.JsiiError(
`Failed validation of tsconfig "compilerOptions" in "${configName}" against rule set "${rules}"!`,
);
}
Expand Down Expand Up @@ -404,7 +404,7 @@ export class Compiler implements Emitter {
const { config, error } = ts.readConfigFile(this.configPath, ts.sys.readFile);
if (error) {
utils.logDiagnostic(error, projectRoot);
throw new Error(`Failed to load tsconfig at ${this.configPath}`);
throw new utils.JsiiError(`Failed to load tsconfig at ${this.configPath}`);
}
const extended = ts.parseJsonConfigFileContent(config, ts.sys, projectRoot);
// the tsconfig parser adds this in, but it is not an expected compilerOption
Expand All @@ -431,7 +431,7 @@ export class Compiler implements Emitter {
if (fs.existsSync(this.configPath)) {
const currentConfig = JSON.parse(fs.readFileSync(this.configPath, 'utf-8'));
if (!(commentKey in currentConfig)) {
throw new Error(
throw new utils.JsiiError(
`A '${this.configPath}' file that was not generated by jsii is in ${this.options.projectInfo.projectRoot}. Aborting instead of overwriting.`,
);
}
Expand Down
8 changes: 4 additions & 4 deletions src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { DiagnosticCategory } from 'typescript';

import { Compiler, CompilerOptions } from './compiler';
import { loadProjectInfo, ProjectInfo } from './project-info';
import { formatDiagnostic } from './utils';
import { formatDiagnostic, JsiiError } from './utils';

/**
* A set of source files for `sourceToAssemblyHelper`, at least containing 'index.ts'
Expand Down Expand Up @@ -113,7 +113,7 @@ export function compileJsiiForTest(
// logDiagnostic() doesn't work out of the box, so console.error() it is.
}
if (errors.length > 0 || emitResult.emitSkipped) {
throw new Error('There were compiler errors');
throw new JsiiError('There were compiler errors');
}
const assembly = loadAssemblyFromPath(process.cwd(), false);
const files: Record<string, string> = {};
Expand Down Expand Up @@ -280,7 +280,7 @@ export class TestWorkspace {
*/
public addDependency(dependencyAssembly: HelperCompilationResult) {
if (this.installed.has(dependencyAssembly.assembly.name)) {
throw new Error(
throw new JsiiError(
`A dependency with name '${dependencyAssembly.assembly.name}' was already installed. Give one a different name.`,
);
}
Expand Down Expand Up @@ -314,7 +314,7 @@ export class TestWorkspace {

public dependencyDir(name: string) {
if (!this.installed.has(name)) {
throw new Error(`No dependency with name '${name}' has been installed`);
throw new JsiiError(`No dependency with name '${name}' has been installed`);
}
return path.join(this.rootDirectory, 'node_modules', name);
}
Expand Down
117 changes: 74 additions & 43 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,55 +138,74 @@ const ruleSets: {
global: true,
}),
async (argv) => {
_configureLog4js(argv.verbose);
try {
_configureLog4js(argv.verbose);

if (argv['generate-tsconfig'] != null && argv.tsconfig != null) {
throw new Error('Options --generate-tsconfig and --tsconfig are mutually exclusive');
}
if (argv['generate-tsconfig'] != null && argv.tsconfig != null) {
throw new utils.JsiiError('Options --generate-tsconfig and --tsconfig are mutually exclusive', true);
}

const projectRoot = path.normalize(path.resolve(process.cwd(), argv.PROJECT_ROOT));

const projectRoot = path.normalize(path.resolve(process.cwd(), argv.PROJECT_ROOT));
const { projectInfo, diagnostics: projectInfoDiagnostics } = loadProjectInfo(projectRoot);

const { projectInfo, diagnostics: projectInfoDiagnostics } = loadProjectInfo(projectRoot);
// disable all silenced warnings
for (const key of argv['silence-warnings']) {
if (!(key in enabledWarnings)) {
throw new utils.JsiiError(
`Unknown warning type ${key as any}. Must be one of: ${warningTypes.join(', ')}`,
);
}

// disable all silenced warnings
for (const key of argv['silence-warnings']) {
if (!(key in enabledWarnings)) {
throw new Error(`Unknown warning type ${key as any}. Must be one of: ${warningTypes.join(', ')}`);
enabledWarnings[key] = false;
}

enabledWarnings[key] = false;
}
configureCategories(projectInfo.diagnostics ?? {});

configureCategories(projectInfo.diagnostics ?? {});

const typeScriptConfig = argv.tsconfig ?? projectInfo.packageJson.jsii?.tsconfig;
const validateTypeScriptConfig =
(argv['validate-tsconfig'] as TypeScriptConfigValidationRuleSet) ??
projectInfo.packageJson.jsii?.validateTsconfig ??
TypeScriptConfigValidationRuleSet.STRICT;

const compiler = new Compiler({
projectInfo,
projectReferences: argv['project-references'],
failOnWarnings: argv['fail-on-warnings'],
stripDeprecated: argv['strip-deprecated'] != null,
stripDeprecatedAllowListFile: argv['strip-deprecated'],
addDeprecationWarnings: argv['add-deprecation-warnings'],
generateTypeScriptConfig: argv['generate-tsconfig'],
typeScriptConfig,
validateTypeScriptConfig,
compressAssembly: argv['compress-assembly'],
});

const emitResult = argv.watch ? await compiler.watch() : compiler.emit();

const allDiagnostics = [...projectInfoDiagnostics, ...emitResult.diagnostics];

for (const diagnostic of allDiagnostics) {
utils.logDiagnostic(diagnostic, projectRoot);
}
if (emitResult.emitSkipped) {
process.exitCode = 1;
const typeScriptConfig = argv.tsconfig ?? projectInfo.packageJson.jsii?.tsconfig;
const validateTypeScriptConfig =
(argv['validate-tsconfig'] as TypeScriptConfigValidationRuleSet) ??
projectInfo.packageJson.jsii?.validateTsconfig ??
TypeScriptConfigValidationRuleSet.STRICT;

const compiler = new Compiler({
projectInfo,
projectReferences: argv['project-references'],
failOnWarnings: argv['fail-on-warnings'],
stripDeprecated: argv['strip-deprecated'] != null,
stripDeprecatedAllowListFile: argv['strip-deprecated'],
addDeprecationWarnings: argv['add-deprecation-warnings'],
generateTypeScriptConfig: argv['generate-tsconfig'],
typeScriptConfig,
validateTypeScriptConfig,
compressAssembly: argv['compress-assembly'],
});

const emitResult = argv.watch ? await compiler.watch() : compiler.emit();

const allDiagnostics = [...projectInfoDiagnostics, ...emitResult.diagnostics];

for (const diagnostic of allDiagnostics) {
utils.logDiagnostic(diagnostic, projectRoot);
}
if (emitResult.emitSkipped) {
process.exitCode = 1;
}
} catch (e: unknown) {
if (e instanceof utils.JsiiError) {
if (e.showHelp) {
console.log();
yargs.showHelp();
console.log();
}

const LOG = log4js.getLogger(utils.CLI_LOGGER);
LOG.error(e.message);

process.exitCode = -1;
} else {
throw e;
}
}
},
)
Expand All @@ -212,18 +231,30 @@ function _configureLog4js(verbosity: number) {
type: 'stderr',
layout: { type: stderrColor ? 'colored' : 'basic' },
},

[utils.DIAGNOSTICS]: {
type: 'stdout',
layout: {
type: stdoutColor ? 'messagePassThrough' : ('passThroughNoColor' as any),
},
},
[utils.CLI_LOGGER]: {
type: 'stderr',
layout: {
type: 'pattern',
pattern: stdoutColor ? '%[[%p]%] %m' : '[%p] %m',
},
},
},
categories: {
default: { appenders: ['console'], level: _logLevel() },
[utils.CLI_LOGGER]: {
appenders: [utils.CLI_LOGGER],
level: _logLevel(),
},
// The diagnostics logger must be set to INFO or more verbose, or watch won't show important messages
[utils.DIAGNOSTICS]: {
appenders: ['diagnostics'],
appenders: [utils.DIAGNOSTICS],
level: _logLevel(Math.max(verbosity, 1)),
},
},
Expand Down
Loading

0 comments on commit 40cf22e

Please sign in to comment.