Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve exceptions #70

Open
xenoterracide opened this issue Dec 27, 2019 · 8 comments
Open

improve exceptions #70

xenoterracide opened this issue Dec 27, 2019 · 8 comments

Comments

@xenoterracide
Copy link
Contributor

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
Copy link
Contributor Author

xenoterracide commented Dec 30, 2019

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

emilioastarita added a commit to emilioastarita/tsyringe that referenced this issue Jan 29, 2020
emilioastarita added a commit to emilioastarita/tsyringe that referenced this issue Jan 29, 2020
emilioastarita added a commit to emilioastarita/tsyringe that referenced this issue Jan 29, 2020
Xapphire13 pushed a commit that referenced this issue Jan 31, 2020
* Improve error messages information (#70)

* Improve multiline regexp readability

* Fix params info type definition
@chrislambe
Copy link

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
Copy link

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

@chrislambe did you find a solution for it?

@MeltingMosaic
Copy link
Collaborator

@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
Copy link

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
Copy link

chrislambe commented Mar 17, 2020

@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
Copy link

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

@Harshita-mindfire
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants