From e2a285b871132f65f429fdbb7d5628a104f68e9a Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Sat, 8 Sep 2018 16:40:16 -0400 Subject: [PATCH] Better NotFound error handling in CodeFetch throwResolutionError was swallowing unrelated errors. --- js/compiler.ts | 52 +++++-------------- src/deno_dir.rs | 22 +++++++- tests/error_004_missing_module.ts.out | 11 ++-- tests/error_005_missing_dynamic_import.ts.out | 11 ++-- 4 files changed, 42 insertions(+), 54 deletions(-) diff --git a/js/compiler.ts b/js/compiler.ts index fdce8c2ed1..624aab201f 100644 --- a/js/compiler.ts +++ b/js/compiler.ts @@ -117,20 +117,6 @@ export class ModuleMetaData implements ts.IScriptSnapshot { } } -/** - * Throw a module resolution error, when a module is unsuccessfully resolved. - */ -function throwResolutionError( - message: string, - moduleSpecifier: ModuleSpecifier, - containingFile: ContainingFile -): never { - throw new Error( - // tslint:disable-next-line:max-line-length - `Cannot resolve module "${moduleSpecifier}" from "${containingFile}".\n ${message}` - ); -} - /** * A singleton class that combines the TypeScript Language Service host API * with Deno specific APIs to provide an interface for compiling and running @@ -514,9 +500,9 @@ export class DenoCompiler if (fileName && this._moduleMetaDataMap.has(fileName)) { return this._moduleMetaDataMap.get(fileName)!; } - let moduleId: ModuleId = ""; - let sourceCode: SourceCode | undefined; - let outputCode: OutputCode | undefined; + let moduleId: ModuleId; + let sourceCode: SourceCode; + let outputCode: OutputCode | null; if ( moduleSpecifier.startsWith(ASSETS) || containingFile.startsWith(ASSETS) @@ -524,38 +510,24 @@ export class DenoCompiler // Assets are compiled into the runtime javascript bundle. // we _know_ `.pop()` will return a string, but TypeScript doesn't so // not null assertion - const moduleId = moduleSpecifier.split("/").pop()!; + moduleId = moduleSpecifier.split("/").pop()!; const assetName = moduleId.includes(".") ? moduleId : `${moduleId}.d.ts`; assert(assetName in assetSourceCode, `No such asset "${assetName}"`); sourceCode = assetSourceCode[assetName]; fileName = `${ASSETS}/${assetName}`; + outputCode = ""; } else { // We query Rust with a CodeFetch message. It will load the sourceCode, // and if there is any outputCode cached, will return that as well. - let fetchResponse; - try { - fetchResponse = this._os.codeFetch(moduleSpecifier, containingFile); - } catch (e) { - return throwResolutionError( - `os.codeFetch message: ${e.message}`, - moduleSpecifier, - containingFile - ); - } - moduleId = fetchResponse.moduleName || ""; - fileName = fetchResponse.filename || undefined; - sourceCode = fetchResponse.sourceCode || undefined; - outputCode = fetchResponse.outputCode || undefined; - } - if (!sourceCode || sourceCode.length === 0 || !fileName) { - return throwResolutionError( - "Invalid source code or file name.", - moduleSpecifier, - containingFile - ); + const fetchResponse = this._os.codeFetch(moduleSpecifier, containingFile); + moduleId = fetchResponse.moduleName!; + fileName = fetchResponse.filename!; + sourceCode = fetchResponse.sourceCode!; + outputCode = fetchResponse.outputCode!; } + assert(sourceCode!.length > 0); this._log("resolveModule sourceCode length:", sourceCode.length); - this._log("resolveModule has outputCode:", !!outputCode); + this._log("resolveModule has outputCode:", outputCode! != null); this._setFileName(moduleSpecifier, containingFile, fileName); if (fileName && this._moduleMetaDataMap.has(fileName)) { return this._moduleMetaDataMap.get(fileName)!; diff --git a/src/deno_dir.rs b/src/deno_dir.rs index bc6a1f306b..a56062a34c 100644 --- a/src/deno_dir.rs +++ b/src/deno_dir.rs @@ -1,6 +1,7 @@ // Copyright 2018 the Deno authors. All rights reserved. MIT license. use errors::DenoError; use errors::DenoResult; +use errors::ErrorKind; use fs as deno_fs; use net; use ring; @@ -159,7 +160,7 @@ impl DenoDir { let (module_name, filename) = self.resolve_module(module_specifier, containing_file)?; - let out = self + let result = self .get_source_code(module_name.as_str(), filename.as_str()) .and_then(|source_code| { Ok(CodeFetchOutput { @@ -168,7 +169,24 @@ impl DenoDir { source_code, maybe_output_code: None, }) - })?; + }); + let out = match result { + Err(err) => { + if err.kind() == ErrorKind::NotFound { + // For NotFound, change the message to something better. + return Err(DenoError::from(std::io::Error::new( + std::io::ErrorKind::NotFound, + format!( + "Cannot resolve module \"{}\" from \"{}\"", + module_specifier, containing_file + ), + ))); + } else { + return Err(err); + } + } + Ok(out) => out, + }; let result = self.load_cache(out.filename.as_str(), out.source_code.as_str()); diff --git a/tests/error_004_missing_module.ts.out b/tests/error_004_missing_module.ts.out index a64360f898..afb65a46c6 100644 --- a/tests/error_004_missing_module.ts.out +++ b/tests/error_004_missing_module.ts.out @@ -1,12 +1,11 @@ -Error: Cannot resolve module "bad-module.ts" from "[WILDCARD]error_004_missing_module.ts". - os.codeFetch message: [WILDCARD] (os error 2) - at throwResolutionError (deno/js/compiler.ts:[WILDCARD]) +NotFound: Cannot resolve module "bad-module.ts" from "[WILDCARD]/tests/error_004_missing_module.ts" + at maybeError (deno/js/errors.ts:[WILDCARD]) + at maybeThrowError (deno/js/errors.ts:[WILDCARD]) + at send (deno/js/fbs_util.ts:[WILDCARD]) + at Object.codeFetch (deno/js/os.ts:[WILDCARD]) at DenoCompiler.resolveModule (deno/js/compiler.ts:[WILDCARD]) at DenoCompiler._resolveModuleName (deno/js/compiler.ts:[WILDCARD]) at moduleNames.map.name (deno/js/compiler.ts:[WILDCARD]) at Array.map () at DenoCompiler.resolveModuleNames (deno/js/compiler.ts:[WILDCARD]) at Object.compilerHost.resolveModuleNames (deno/third_party/node_modules/typescript/lib/typescript.js:[WILDCARD]) - at resolveModuleNamesWorker (deno/third_party/node_modules/typescript/lib/typescript.js:[WILDCARD]) - at resolveModuleNamesReusingOldState (deno/third_party/node_modules/typescript/lib/typescript.js:[WILDCARD]) - at processImportedModules (deno/third_party/node_modules/typescript/lib/typescript.js:[WILDCARD]) diff --git a/tests/error_005_missing_dynamic_import.ts.out b/tests/error_005_missing_dynamic_import.ts.out index dbcaa0e9e2..0871eb780a 100644 --- a/tests/error_005_missing_dynamic_import.ts.out +++ b/tests/error_005_missing_dynamic_import.ts.out @@ -1,12 +1,11 @@ -Error: Cannot resolve module "bad-module.ts" from "[WILDCARD]deno/tests/error_005_missing_dynamic_import.ts". - os.codeFetch message: [WILDCARD] (os error 2) - at throwResolutionError (deno/js/compiler.ts:[WILDCARD]) +NotFound: Cannot resolve module "bad-module.ts" from "[WILDCARD]deno/tests/error_005_missing_dynamic_import.ts" + at maybeError (deno/js/errors.ts:[WILDCARD]) + at maybeThrowError (deno/js/errors.ts:[WILDCARD]) + at send (deno/js/fbs_util.ts:[WILDCARD]) + at Object.codeFetch (deno/js/os.ts:[WILDCARD]) at DenoCompiler.resolveModule (deno/js/compiler.ts:[WILDCARD]) at DenoCompiler._resolveModuleName (deno/js/compiler.ts:[WILDCARD]) at moduleNames.map.name (deno/js/compiler.ts:[WILDCARD]) at Array.map () at DenoCompiler.resolveModuleNames (deno/js/compiler.ts:[WILDCARD]) at Object.compilerHost.resolveModuleNames (deno/third_party/node_modules/typescript/lib/typescript.js:[WILDCARD]) - at resolveModuleNamesWorker (deno/third_party/node_modules/typescript/lib/typescript.js:[WILDCARD]) - at resolveModuleNamesReusingOldState (deno/third_party/node_modules/typescript/lib/typescript.js:[WILDCARD]) - at processImportedModules (deno/third_party/node_modules/typescript/lib/typescript.js:[WILDCARD])