search

rob3000 / nestjs-kafka

NestJS integration with KafkaJS
The Unlicense
132 stars 44 forks source link

Add option to not automatically connect producer and admin #16

Closed andres-halls closed 3 years ago

andres-halls commented 3 years ago

Hi, currently producer and admin are automatically connected in onModuleInit. https://github.com/rob3000/nestjs-kafka/blob/ee85744b3a5ed890a1a3b1d94b160011525efe8b/src/kafka.service.ts#L78

This causes a problem however when Kafka is unreachable. After the configured number of retries, KafkaJS will throw KafkaJSNumberOfRetriesExceeded which is not caught and Nest will not start. Increasing retries does not help because Nest is waiting for the module to be initialized which will never happen until the producer connects successfully.

From the KafkaJS documentation:

If the max number of retries is exceeded the retrier will throw KafkaJSNumberOfRetriesExceeded and interrupt. Producers will bubble up the error to the user code; Consumers will wait the retry time attached to the exception (it will be based on the number of attempts) and perform a full restart.

I propose adding an option to not connect the producer and admin in onModuleInit, instead make it possible for users to connect manually.

rob3000 commented 3 years ago

Hi @andres-liiver ,

We could look to add an option. What would be the benefit vs the current implementation now? In most cases you would want Kafka to close and the service to stop if Kafka is unreachable?

andres-halls commented 3 years ago

In my case, I need the Nest application to start no matter what. Kafka being down cannot prevent it from starting.

It seems the real issue is the promise rejection not being caught in https://github.com/rob3000/nestjs-kafka/blob/ee85744b3a5ed890a1a3b1d94b160011525efe8b/src/kafka.service.ts#L77

Wrapping everything in a try/catch in the connect function fixes the issue of Nest not starting. However, after some time, another UnhandledPromiseRejectionWarning: KafkaJSNonRetriableError is still thrown, haven't found the source of that yet.

I think adding an option to not automatically connect producer and admin still makes sense though.

rob3000 commented 3 years ago

Hi @andres-liiver apologies for the delay. I've just added a flag that you can can set in your config to disable the consumer and admin from auto connecting: https://github.com/rob3000/nestjs-kafka/blob/master/src/kafka.service.ts#L80-L82

rob3000 commented 3 years ago

Deployed in 1.4.0