microsoft / tsyringe

Lightweight dependency injection container for JavaScript/TypeScript
MIT License
5.04k stars 167 forks source link

improve exceptions #70

Open xenoterracide opened 4 years ago

xenoterracide commented 4 years ago

hmmm... getting this in a jest test...

import { ApolloServerTestClient } from 'apollo-server-testing';
import 'reflect-metadata';
import testContext from '../test-fixtures/testContext';
import { Beans } from '../types/Beans';

test('version query', async() => {
  const { query } = await testContext()
    .resolve<Promise<ApolloServerTestClient>>(Beans.APOLLO_TEST_CLIENT);
  const res = await query({ query: '{ version }' });
  expect(res.data).toEqual({ version: 'dev' });
});

it's complaining at test

Error: Failed: "TypeInfo not known for function Object() { [native code] }"
Error: 
    at Env.it (/Users/calebcushing/IdeaProjects/service-graph/node_modules/jest-jasmine2/build/jasmineAsyncInstall.js:91:24)
    at it (/Users/calebcushing/IdeaProjects/service-graph/node_modules/jest-jasmine2/build/jasmine/jasmineLight.js:93:21)
    at Object.<anonymous> (/Users/calebcushing/IdeaProjects/service-graph/src/__tests__/VersionTest.ts:6:1)
    at Runtime._execModule (/Users/calebcushing/IdeaProjects/service-graph/node_modules/jest-runtime/build/index.js:867:68)
    at Runtime._loadModule (/Users/calebcushing/IdeaProjects/service-graph/node_modules/jest-runtime/build/index.js:577:12)
    at Runtime.requireModule (/Users/calebcushing/IdeaProjects/service-graph/node_modules/jest-runtime/build/index.js:433:10)
    at /Users/calebcushing/IdeaProjects/service-graph/node_modules/jest-jasmine2/build/index.js:202:13
    at Generator.next (<anonymous>)
    at asyncGeneratorStep (/Users/calebcushing/IdeaProjects/service-graph/node_modules/jest-jasmine2/build/index.js:27:24)
    at _next (/Users/calebcushing/IdeaProjects/service-graph/node_modules/jest-jasmine2/build/index.js:47:9)

the problem was I didn't include @inject("tag") in my dependencies.

For this ticket I'd like to see an improved exception. At the very least I need to know which class it was trying to inject when it failed, ideally with the type or parameter position trying to be injected, as I had about a dozen. At best I'd like to see the actual source of the resolve in the stacktrace.

xenoterracide commented 4 years ago

here's another

(node:58963) UnhandledPromiseRejectionWarning: Attempted to resolve unregistered dependency token: Symbol(corsOrigin)
(node:58963) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:58963) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

https://github.com/microsoft/tsyringe/blob/master/src/dependency-container.ts#L172

I think that prefixing with tsyringe: might help, though not sure why this is a promise

chrislambe commented 4 years ago

I actually just ran into this issue as well, providing null as a managed value.

Minimal reproduction:

import 'reflect-metadata';
import { container } from 'tsyringe';

container.register('foo', { useValue: null });

container.resolve('foo');

Resulting error:

/Volumes/Storage/Documents/Projects/tsyringe/node_modules/tsyringe/dist/cjs/dependency-container.js:179
            throw new Error(`TypeInfo not known for "${ctor.name}"`);
                  ^
Error: TypeInfo not known for "undefined"
    at InternalDependencyContainer.construct (/Volumes/Storage/Documents/Projects/tsyringe/node_modules/tsyringe/dist/cjs/dependency-container.js:179:19)
    at InternalDependencyContainer.resolveRegistration (/Volumes/Storage/Documents/Projects/tsyringe/node_modules/tsyringe/dist/cjs/dependency-container.js:103:24)
    at InternalDependencyContainer.resolve (/Volumes/Storage/Documents/Projects/tsyringe/node_modules/tsyringe/dist/cjs/dependency-container.js:77:25)
    at Object.<anonymous> (/Volumes/Storage/Documents/Projects/tsyringe/main.ts:6:11)
    at Module._compile (internal/modules/cjs/loader.js:776:30)
    at Module.m._compile (/Volumes/Storage/Documents/Projects/tsyringe/node_modules/ts-node/src/index.ts:814:23)
    at Module._extensions..js (internal/modules/cjs/loader.js:787:10)
    at Object.require.extensions.(anonymous function) [as .ts] (/Volumes/Storage/Documents/Projects/tsyringe/node_modules/ts-node/src/index.ts:817:12)
    at Module.load (internal/modules/cjs/loader.js:653:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:593:12)

This was difficult to debug, not knowing that null was an invalid managed value.

avin-kavish commented 4 years ago

Me too, this is bit of a problem. How do we handle optional dependencies?

@chrislambe did you find a solution for it?

MeltingMosaic commented 4 years ago

@avin-kavish , what is the scenario you have that needs the Optional decorator? I haven't seen the need for it yet, but I do see that Angular has it, and we could follow suit if there were compelling use-cases.

avin-kavish commented 4 years ago

Optional dependencies, I tried to use it to inject the current Principal acting on the resource. And sometimes the request can be from an unauthenticated user. I was planning to use the null value to indicate that.

I did,

container.register(Principal, { useValue: req.user }) // req.user may be null

But I can model the problem differently. I could change it to a wrapper class, similar to ASP.Net Core identity,

class Principal {

  ctor(public user: User)

  isLoggedIn() {
    return !!this.user
  }
}

container.register(Principal, { useValue: new Principal(req.user) })

So yeah, I can't exactly say optional dependencies are needed, but I think having them wouldn't be bad.

chrislambe commented 4 years ago

@avin-kavish I ended up just doing this:

container.register('foo', { useFactory: instanceCachingFactory(() => possiblyNullishValue) });

And it seems to work fine, though I suspect there's probably a reason not to do this.

avin-kavish commented 4 years ago

If that works and the other doesn't, it's a bug.

Harshita-mindfire commented 2 years ago

This issue still persists as of today. For optional dependencies, tsyringe throws TypeInfo not known for "undefined" error.