From 6fbd5e6217fc42aeb668c68c4eb8201440f85dc4 Mon Sep 17 00:00:00 2001 From: Emilio Astarita Date: Wed, 29 Jan 2020 09:20:55 -0300 Subject: [PATCH] Improve error messages information (#70) --- src/__tests__/auto-injectable.test.ts | 3 +- src/__tests__/errors.test.ts | 42 +++++++++++++++++++++++++++ src/decorators/auto-injectable.ts | 12 ++------ src/dependency-container.ts | 31 +++++++++++++------- src/error-helpers.ts | 27 +++++++++++++++++ 5 files changed, 92 insertions(+), 23 deletions(-) create mode 100644 src/__tests__/errors.test.ts create mode 100644 src/error-helpers.ts diff --git a/src/__tests__/auto-injectable.test.ts b/src/__tests__/auto-injectable.test.ts index bdb3648..448af3e 100644 --- a/src/__tests__/auto-injectable.test.ts +++ b/src/__tests__/auto-injectable.test.ts @@ -121,9 +121,8 @@ test("@autoInjectable throws a clear error if a dependency can't be resolved.", class Foo { constructor(public myBar?: Bar) {} } - expect(() => new Foo()).toThrow( - /Cannot inject the dependency myBar of Foo constructor. Error: TypeInfo/ + /Cannot inject the dependency "myBar" at position #0 of "Foo" constructor\. Reason:\s+TypeInfo/ ); }); diff --git a/src/__tests__/errors.test.ts b/src/__tests__/errors.test.ts new file mode 100644 index 0000000..36226f1 --- /dev/null +++ b/src/__tests__/errors.test.ts @@ -0,0 +1,42 @@ +import {instance as globalContainer} from "../dependency-container"; +import {inject, injectable} from "../decorators"; + +afterEach(() => { + globalContainer.reset(); +}); + +test("Error message composition", () => { + class Ok {} + + @injectable() + class C { + constructor(public s: any) {} + } + + @injectable() + class B { + constructor(public c: C) {} + } + + @injectable() + class A { + constructor(public d: Ok, public b: B) {} + } + expect(() => { + globalContainer.resolve(A); + }).toThrow( + /Cannot inject the dependency "b" at position #1 of "A" constructor. Reason:\s+Cannot inject the dependency "c" at position #0 of "B" constructor. Reason:\s+Cannot inject the dependency "s" at position #0 of "C" constructor. Reason:\s+TypeInfo not known for Object/ + ); +}); + +test("Param position", () => { + @injectable() + class A { + constructor(@inject("missing") public j: any) {} + } + expect(() => { + globalContainer.resolve(A); + }).toThrow( + /Cannot inject the dependency "j" at position #0 of "A" constructor. Reason:\s+Attempted to resolve unregistered dependency token: "missing"/ + ); +}); diff --git a/src/decorators/auto-injectable.ts b/src/decorators/auto-injectable.ts index 54a2981..c924073 100644 --- a/src/decorators/auto-injectable.ts +++ b/src/decorators/auto-injectable.ts @@ -2,6 +2,7 @@ import constructor from "../types/constructor"; import {getParamInfo} from "../reflection-helpers"; import {instance as globalContainer} from "../dependency-container"; import {isTokenDescriptor} from "../providers/injection-token"; +import {formatErrorCtor} from "../error-helpers"; /** * Class decorator factory that replaces the decorated class' constructor with @@ -29,16 +30,7 @@ function autoInjectable(): (target: constructor) => any { return globalContainer.resolve(type); } catch (e) { const argIndex = index + args.length; - - const [, params = null] = - target.toString().match(/constructor\(([\w, ]+)\)/) || []; - const argName = params - ? params.split(",")[argIndex] - : `#${argIndex}`; - - throw new Error( - `Cannot inject the dependency ${argName} of ${target.name} constructor. ${e}` - ); + throw new Error(formatErrorCtor(target, argIndex, e)); } }) ) diff --git a/src/dependency-container.ts b/src/dependency-container.ts index cb93625..4f57cc2 100644 --- a/src/dependency-container.ts +++ b/src/dependency-container.ts @@ -17,6 +17,7 @@ import constructor from "./types/constructor"; import Registry from "./registry"; import Lifecycle from "./types/lifecycle"; import ResolutionContext from "./resolution-context"; +import {formatErrorCtor} from "./error-helpers"; export type Registration = { provider: Provider; @@ -174,7 +175,7 @@ class InternalDependencyContainer implements DependencyContainer { if (!registration && isNormalToken(token)) { throw new Error( - `Attempted to resolve unregistered dependency token: ${token.toString()}` + `Attempted to resolve unregistered dependency token: "${token.toString()}"` ); } @@ -246,7 +247,7 @@ class InternalDependencyContainer implements DependencyContainer { if (!registrations && isNormalToken(token)) { throw new Error( - `Attempted to resolve unregistered dependency token: ${token.toString()}` + `Attempted to resolve unregistered dependency token: "${token.toString()}"` ); } @@ -338,20 +339,28 @@ class InternalDependencyContainer implements DependencyContainer { const paramInfo = typeInfo.get(ctor); if (!paramInfo || paramInfo.length === 0) { - throw new Error(`TypeInfo not known for ${ctor}`); + throw new Error(`TypeInfo not known for ${ctor.name}`); } - const params = paramInfo.map(param => { - if (isTokenDescriptor(param)) { - return param.multiple - ? this.resolveAll(param.token) - : this.resolve(param.token, context); - } - return this.resolve(param, context); - }); + const params = paramInfo.map(this.resolveParams(context, ctor)); return new ctor(...params); } + + private resolveParams(context: ResolutionContext, ctor: constructor) { + return (param: any, idx: number) => { + try { + if (isTokenDescriptor(param)) { + return param.multiple + ? this.resolveAll(param.token) + : this.resolve(param.token, context); + } + return this.resolve(param, context); + } catch (e) { + throw new Error(formatErrorCtor(ctor, idx, e)); + } + }; + } } export const instance: DependencyContainer = new InternalDependencyContainer(); diff --git a/src/error-helpers.ts b/src/error-helpers.ts new file mode 100644 index 0000000..fdd6635 --- /dev/null +++ b/src/error-helpers.ts @@ -0,0 +1,27 @@ +import constructor from "./types/constructor"; + +function formatDependency(params: string | null, idx: number): string { + if (params === null) { + return `at position #${idx}`; + } + const argName = params.split(",")[idx].trim(); + return `"${argName}" at position #${idx}`; +} + +function composeErrorMessage(msg: string, e: Error, indent = " "): string { + return [msg, ...e.message.split("\n").map(l => indent + l)].join("\n"); +} + +export function formatErrorCtor( + ctor: constructor, + paramIdx: number, + error: Error +): string { + const [, params = null] = + ctor.toString().match(/constructor\(([\w, ]+)\)/) || []; + const dep = formatDependency(params, paramIdx); + return composeErrorMessage( + `Cannot inject the dependency ${dep} of "${ctor.name}" constructor. Reason:`, + error + ); +}