Skip to content

Commit

Permalink
Improve error messages information (microsoft#70) (microsoft#88)
Browse files Browse the repository at this point in the history
* Improve error messages information (microsoft#70)

* Improve multiline regexp readability

* Fix params info type definition
  • Loading branch information
emilioastarita authored Jan 31, 2020
1 parent 4f63bb3 commit 75f769d
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 26 deletions.
3 changes: 1 addition & 2 deletions src/__tests__/auto-injectable.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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/
);
});

Expand Down
60 changes: 60 additions & 0 deletions src/__tests__/errors.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
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(
new RegExp(
[
/Cannot inject the dependency "b" at position #1 of "A" constructor\. Reason:/,
/Cannot inject the dependency "c" at position #0 of "B" constructor\. Reason:/,
/Cannot inject the dependency "s" at position #0 of "C" constructor\. Reason:/,
/TypeInfo not known for "Object"/
]
.map(x => x.source)
.join("\\s+")
)
);
});

test("Param position", () => {
@injectable()
class A {
constructor(@inject("missing") public j: any) {}
}

expect(() => {
globalContainer.resolve(A);
}).toThrow(
new RegExp(
[
/Cannot inject the dependency "j" at position #0 of "A" constructor\. Reason:/,
/Attempted to resolve unregistered dependency token: "missing"/
]
.map(x => x.source)
.join("\\s+")
)
);
});
12 changes: 2 additions & 10 deletions src/decorators/auto-injectable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -29,16 +30,7 @@ function autoInjectable(): (target: constructor<any>) => 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));
}
})
)
Expand Down
40 changes: 27 additions & 13 deletions src/dependency-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ import {
} from "./providers";
import Provider, {isProvider} from "./providers/provider";
import FactoryProvider from "./providers/factory-provider";
import InjectionToken, {isTokenDescriptor} from "./providers/injection-token";
import InjectionToken, {
isTokenDescriptor,
TokenDescriptor
} from "./providers/injection-token";
import TokenProvider from "./providers/token-provider";
import ValueProvider from "./providers/value-provider";
import ClassProvider from "./providers/class-provider";
Expand All @@ -17,14 +20,17 @@ 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<T = any> = {
provider: Provider<T>;
options: RegistrationOptions;
instance?: T;
};

export const typeInfo = new Map<constructor<any>, any[]>();
export type ParamInfo = TokenDescriptor | InjectionToken<any>;

export const typeInfo = new Map<constructor<any>, ParamInfo[]>();

/** Dependency Container */
class InternalDependencyContainer implements DependencyContainer {
Expand Down Expand Up @@ -174,7 +180,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()}"`
);
}

Expand Down Expand Up @@ -246,7 +252,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()}"`
);
}

Expand Down Expand Up @@ -338,20 +344,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<T>(context: ResolutionContext, ctor: constructor<T>) {
return (param: ParamInfo, 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();
Expand Down
27 changes: 27 additions & 0 deletions src/error-helpers.ts
Original file line number Diff line number Diff line change
@@ -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<any>,
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
);
}
3 changes: 2 additions & 1 deletion src/reflection-helpers.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import Dictionary from "./types/dictionary";
import constructor from "./types/constructor";
import InjectionToken from "./providers/injection-token";
import {ParamInfo} from "./dependency-container";

export const INJECTION_TOKEN_METADATA_KEY = "injectionTokens";

export function getParamInfo(target: constructor<any>): any[] {
export function getParamInfo(target: constructor<any>): ParamInfo[] {
const params: any[] = Reflect.getMetadata("design:paramtypes", target) || [];
const injectionTokens: Dictionary<InjectionToken<any>> =
Reflect.getOwnMetadata(INJECTION_TOKEN_METADATA_KEY, target) || {};
Expand Down

0 comments on commit 75f769d

Please sign in to comment.