nestjsx / nestjs-amqp

Amqp connections for nestjs :love_letter:
MIT License
190 stars 24 forks source link

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

Open probil opened 5 years ago

probil commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

Have a nice holiday! 🎉

bashleigh commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

Sorry it's been ages... Been mega stressed :joy: 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 commented 5 years ago

@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 commented 5 years ago

@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.