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

Error: Nest can't resolve dependencies of the AlertProvider (?). #33

Open
probil opened this issue May 10, 2019 · 10 comments
Open

Error: Nest can't resolve dependencies of the AlertProvider (?). #33

probil opened this issue May 10, 2019 · 10 comments
Assignees
Labels
bug Something isn't working

Comments

@probil
Copy link

probil commented May 10, 2019

I'm new to nest.js. I don't know why Nest cant resolve dependencies. And there is nothing in README. I had found something similar in closed issues but it didn't help. Here is an error

Error: Nest can't resolve dependencies of the AlertProvider (?). Please make sure that the argument at index [0] is available in the AlertModule context.

Here is my files (cleaned up a little bit):

// app/app.module.ts
import { Module } from '@nestjs/common';

@Module({
  imports: [
    ConfigModule.load(
      path.resolve(process.cwd(), 'config', '**/!(*.d).{ts,js}'),
    ),
    AmqpModule.forRootAsync({
      inject: [ConfigService],
      useFactory: (config: ConfigService) => config.get('amqp'),
    }),
    //...
})
export class AppModule implements NestModule
// config/amqp.ts
export default {
  name: 'default',
  hostname: process.env.AMQP_HOST || 'mq-service',
  port: process.env.AMQP_PORT || 5672,
  username: process.env.USERNAME || 'guest',
  password: process.env.PASSWORD || 'guest',
};
// alert/alert.module.ts
import { Module } from '@nestjs/common';
import { AmqpModule } from 'nestjs-amqp';
import { AlertService } from './alert.service';
import { AlertProvider } from './alert.provider';

@Module({
  imports: [
    AmqpModule.forFeature(),  // The error also occurs without this line
  ],
  providers: [
    AlertService,
    AlertProvider,
  ],
})
export class AlertModule {}
// alert/alert.provider.ts
import { InjectAmqpConnection } from 'nestjs-amqp';

const QUEUE = 'alerts';

export class AlertProvider {
  constructor(
    @InjectAmqpConnection('default') private readonly amqp,
  ) {
    this.setupListeners();
  }

  async setupListeners() {
    const channel = await this.amqp.createChannel();
    await channel.assertQueue(QUEUE);
    channel.consume(QUEUE, (msg) => {
      if (msg === null) {
        return;
      }
      console.log(msg.content.toString());
    });
  }
}

Maybe you can suggest me something.
I use nest@v6 and latest version of nestjs-amqp

@bashleigh bashleigh self-assigned this May 11, 2019
@bashleigh bashleigh added the bug Something isn't working label May 11, 2019
@bashleigh
Copy link
Collaborator

bashleigh commented May 11, 2019

This issue is because the connection provider is created 'dynamically' when the module's forRootAsync is called. So the provider is created by awaiting an amqp connection and then the provider exists. That's the problem with this module. Having a provider depend on that connection provider sometimes cannot be instanced because the provider it requires cannot be passed in at the time provider being instanced.

So to fix this I should probably refactor this package to handle this problem better but I've been struggling for time to dedicate to it plus you can use Transport.RMQ so I think this package is pretty much redundant.

In the short term you can fix this by making AlertProvider an AsyncProvider like so

@Module({
  imports: [
    AmqpModule.forFeature(),
  ],
  providers: [
    AlertService,
    {
        provide: AlertProvider,
        useFactory: (connection) => new AlertProvider(connection),
        inject: ['AMQP_CONNECTION_PROVIDER_default'],
    },
  ],
})
export class AlertModule {}

It's a bit of a shit fix but should work. In the future I think I should make a connection's manager and make that the injectable so providers aren't reliant on the connection being created to inject.

Plus I should probably make this test better https://github.com/nestjs-community/nestjs-amqp/blob/master/src/__tests__/amqp.module.spec.ts#L103

Perhaps after I finish off the config package's V2 I should do a V2 of this one!

@probil
Copy link
Author

probil commented May 13, 2019

The fix doesn't work. I still have the same error.

I didn't get the point about Transport.RMQ - Nest.js docs are not straightforward. I've found a useful link, so I will try to do so.

In any case, thanks for your answer @bashleigh

@probil
Copy link
Author

probil commented May 15, 2019

I can't use Transport.RMQ - it forces me to use predefined structure.
But I have no control over data structure in RabbitMQ
Also I need to ack only successfully processed messages

@bashleigh
Copy link
Collaborator

Sorry I did start writing a reply but been mega busy! I'll try and make you an example repo and play around with it. I'm going on holiday tomorrow night so I won't be able to reply after then and not before the 21st? I think I come back then? Sorry! I'll do my best!

@probil
Copy link
Author

probil commented May 15, 2019

Ok, thank you @bashleigh
I'm looking forward to check your example

Have a nice holiday! 🎉

@bashleigh
Copy link
Collaborator

Sorry I didn't forget just been mega busy! So I tried replicating your code into a test, I updated my nest modules to the latest of latest and I get this same result for all tests now so looks like the package needs an update! I'll have a think as to why the provider tokens are now invalid!

@probil
Copy link
Author

probil commented May 23, 2019

@bashleigh Nice to hear that you can reproduce an issue!
Hope you will find some time to find out what's wrong. Good luck ✋

@bashleigh
Copy link
Collaborator

Sorry it's been ages... Been mega stressed 😂 isn't life great! So I've taken a quick look and there was only one issues causing the v6 to not pass tests which is odd.

If I'm really honest this package could do with a rebuild forbetter implementation, performance and a lot of other things. It's not the best thing in the world. I'll add a test to replicate your issue 👍

@jmcdo29
Copy link

jmcdo29 commented Jul 9, 2019

@probil I just helped someone with a very similar problem, in your test are your providing the following code to your Test.createTestingModule()?

beforeEach(async() => {
  const module = await Test.createTestingModule({
    providers: [
      {
        provide: 'AMQP_CONNECTION_PROVIDER_default',
        useValue: yourMockObject
      }
    ]
  });
});

This is how the package is creating the DI injection token, after looking through the decorator and the token creator method.

@bashleigh Do you think it would be possible to expose a method similar to the TypeORM or Mongo methods to get the DI token? It could make for easier testing. Maybe in the rebuild of the package?

@bashleigh
Copy link
Collaborator

bashleigh commented Jul 10, 2019

@probil I've just had a thought. You are running await app.init(); right? Because the amqp providers are created in onModuleInit().

@jmcdo29 yea sure I've thought about doing the same thing with the nestjs-config module. I've exported everything in v2 expect that and I needed to use it yesterday. You're welcome to make a PR if you want :)
Really not sure what to do for a rebuild. Next time I have the oppitunity to work with amqp and nest I'll have a look at replacing this package with something better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants