From 9761db5ac2efb9a49fbb08a6e40c115cddc0df34 Mon Sep 17 00:00:00 2001 From: Bowen Ni Date: Wed, 30 Nov 2016 13:59:53 -0800 Subject: [PATCH] refactor(compiler): change ngc error handling Do not print stack trace for user errors Print stack trace for compiler internal errors --- modules/@angular/compiler-cli/src/main.ts | 26 +++-- .../@angular/compiler-cli/test/main_spec.ts | 108 ++++++++++++++++++ .../compiler-cli/test/private_import_core.ts | 13 +++ tools/@angular/tsc-wrapped/index.ts | 2 +- tools/@angular/tsc-wrapped/src/main.ts | 2 + tools/@angular/tsc-wrapped/src/tsc.ts | 11 +- 6 files changed, 152 insertions(+), 10 deletions(-) create mode 100644 modules/@angular/compiler-cli/test/main_spec.ts create mode 100644 modules/@angular/compiler-cli/test/private_import_core.ts diff --git a/modules/@angular/compiler-cli/src/main.ts b/modules/@angular/compiler-cli/src/main.ts index 265687c6e0..ffaae93cc6 100644 --- a/modules/@angular/compiler-cli/src/main.ts +++ b/modules/@angular/compiler-cli/src/main.ts @@ -22,14 +22,24 @@ function codegen( return CodeGenerator.create(ngOptions, cliOptions, program, host).codegen(); } +export function main(args: any, consoleError: any): Promise { + const project = args.p || args.project || '.'; + const cliOptions = new tsc.NgcCliOptions(args); + + return tsc.main(project, cliOptions, codegen).then(() => 0).catch(e => { + if (e instanceof tsc.UserError) { + consoleError(e.message); + return Promise.resolve(0); + } else { + consoleError(e.stack); + consoleError('Compilation failed'); + return Promise.resolve(1); + } + }); +} + // CLI entry point if (require.main === module) { const args = require('minimist')(process.argv.slice(2)); - const project = args.p || args.project || '.'; - const cliOptions = new tsc.NgcCliOptions(args); - tsc.main(project, cliOptions, codegen).then(exitCode => process.exit(exitCode)).catch(e => { - console.error(e.stack); - console.error('Compilation failed'); - process.exit(1); - }); -} + main(args, console.error).then((exitCode: number) => process.exit(exitCode)); +} \ No newline at end of file diff --git a/modules/@angular/compiler-cli/test/main_spec.ts b/modules/@angular/compiler-cli/test/main_spec.ts new file mode 100644 index 0000000000..5f4d11a2f3 --- /dev/null +++ b/modules/@angular/compiler-cli/test/main_spec.ts @@ -0,0 +1,108 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {makeTempDir} from '@angular/tsc-wrapped/test/test_support'; +import * as fs from 'fs'; +import * as path from 'path'; + +import {main} from '../src/main'; +import {ReflectionCapabilities, reflector} from './private_import_core'; + + +describe('compiler-cli', () => { + let basePath: string; + let write: (fileName: string, content: string) => void; + + beforeEach(() => { + basePath = makeTempDir(); + write = (fileName: string, content: string) => { + fs.writeFileSync(path.join(basePath, fileName), content, {encoding: 'utf-8'}); + }; + write('tsconfig.json', `{ + "compilerOptions": { + "experimentalDecorators": true, + "types": [], + "outDir": "built", + "declaration": true, + "module": "es2015" + }, + "angularCompilerOptions": { + "annotateForClosureCompiler": true + }, + "files": ["test.ts"] + }`); + }); + + // Restore reflector since AoT compiler will update it with a new static reflector + afterEach(() => { reflector.updateCapabilities(new ReflectionCapabilities()); }); + + it('should compile without errors', (done) => { + write('test.ts', 'export const A = 1;'); + + const mockConsole = {error: {}}; + + spyOn(mockConsole, 'error'); + + main({p: basePath}, mockConsole.error) + .then((exitCode) => { + expect(mockConsole.error).not.toHaveBeenCalled(); + expect(exitCode).toEqual(0); + done(); + }) + .catch(e => done.fail(e)); + }); + + it('should not print the stack trace if user input file does not exist', (done) => { + const mockConsole = {error: {}}; + + spyOn(mockConsole, 'error'); + + main({p: basePath}, mockConsole.error) + .then((exitCode) => { + expect(mockConsole.error).toHaveBeenCalled(); + expect(mockConsole.error).not.toHaveBeenCalledWith('Compilation failed'); + expect(exitCode).toEqual(0); + done(); + }) + .catch(e => done.fail(e)); + }); + + it('should not print the stack trace if user input file is malformed', (done) => { + write('test.ts', 'foo bar'); + + const mockConsole = {error: {}}; + + spyOn(mockConsole, 'error'); + + main({p: basePath}, mockConsole.error) + .then((exitCode) => { + expect(mockConsole.error).toHaveBeenCalled(); + expect(mockConsole.error).not.toHaveBeenCalledWith('Compilation failed'); + expect(exitCode).toEqual(0); + done(); + }) + .catch(e => done.fail(e)); + }); + + it('should print the stack trace on compiler internal errors', (done) => { + write('test.ts', 'export const A = 1;'); + + const mockConsole = {error: {}}; + + spyOn(mockConsole, 'error'); + + main({p: 'not-exist'}, mockConsole.error) + .then((exitCode) => { + expect(mockConsole.error).toHaveBeenCalled(); + expect(mockConsole.error).toHaveBeenCalledWith('Compilation failed'); + expect(exitCode).toEqual(1); + done(); + }) + .catch(e => done.fail(e)); + }); +}); \ No newline at end of file diff --git a/modules/@angular/compiler-cli/test/private_import_core.ts b/modules/@angular/compiler-cli/test/private_import_core.ts new file mode 100644 index 0000000000..4b1eeb34b5 --- /dev/null +++ b/modules/@angular/compiler-cli/test/private_import_core.ts @@ -0,0 +1,13 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {__core_private__ as r} from '@angular/core'; + +export type ReflectionCapabilities = typeof r._ReflectionCapabilities; +export var ReflectionCapabilities: typeof r.ReflectionCapabilities = r.ReflectionCapabilities; +export var reflector: typeof r.reflector = r.reflector; diff --git a/tools/@angular/tsc-wrapped/index.ts b/tools/@angular/tsc-wrapped/index.ts index c219edb0b1..2d6577f155 100644 --- a/tools/@angular/tsc-wrapped/index.ts +++ b/tools/@angular/tsc-wrapped/index.ts @@ -7,7 +7,7 @@ */ export {DecoratorDownlevelCompilerHost, MetadataWriterHost} from './src/compiler_host'; -export {CodegenExtension, main} from './src/main'; +export {CodegenExtension, UserError, main} from './src/main'; export {default as AngularCompilerOptions} from './src/options'; export * from './src/cli_options'; diff --git a/tools/@angular/tsc-wrapped/src/main.ts b/tools/@angular/tsc-wrapped/src/main.ts index c5e1f8ed17..9e2c99eab3 100644 --- a/tools/@angular/tsc-wrapped/src/main.ts +++ b/tools/@angular/tsc-wrapped/src/main.ts @@ -16,6 +16,8 @@ import NgOptions from './options'; import {MetadataWriterHost, DecoratorDownlevelCompilerHost, TsickleCompilerHost} from './compiler_host'; import {CliOptions} from './cli_options'; +export {UserError} from './tsc'; + export type CodegenExtension = (ngOptions: NgOptions, cliOptions: CliOptions, program: ts.Program, host: ts.CompilerHost) => Promise; diff --git a/tools/@angular/tsc-wrapped/src/tsc.ts b/tools/@angular/tsc-wrapped/src/tsc.ts index e68deb6d67..75a2844045 100644 --- a/tools/@angular/tsc-wrapped/src/tsc.ts +++ b/tools/@angular/tsc-wrapped/src/tsc.ts @@ -24,6 +24,15 @@ export interface CompilerInterface { emit(program: ts.Program): number; } +export class UserError extends Error { + constructor(message: string) { + super(message); + this.message = message; + this.name = 'UserError'; + this.stack = new Error().stack; + } +} + const DEBUG = false; function debug(msg: string, ...o: any[]) { @@ -48,7 +57,7 @@ export function formatDiagnostics(diags: ts.Diagnostic[]): string { export function check(diags: ts.Diagnostic[]) { if (diags && diags.length && diags[0]) { - throw new Error(formatDiagnostics(diags)); + throw new UserError(formatDiagnostics(diags)); } }