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

feat(microservices): handle kafka consumer crashes #11910

Closed

Conversation

esahin90
Copy link
Contributor

@esahin90 esahin90 commented Jun 27, 2023

To be able to react to consumer crashes within kafkajs, which are not retryable, the ClientKafka and ServerKafka have to listen to CRASH events and act accordingly. Listen to CRASH events and throw an exception, if the crash is not retried.

Documentation: https://kafka.js.org/docs/instrumentation-events#a-name-consumer-a-consumer

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior?

Whenever the kafka consumer crashes, the application will not cleanly shutdown. The rest of the application will still work as expected, but no messages from kafka will be consumed

Issue Number: N/A

Related Issue, but not solving it: #11616

What is the new behavior?

Whenever a crash of the kafka consumer is not retriable, the handler will throw an exception to shutdown the nest application

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

To be able to react to consumer crashes within kafkajs, which are not
retryable, the ClientKafka and ServerKafka have to listen to CRASH
events and act accordingly. Listen to CRASH events and throw an
exception, if the crash is not retried.

Documentation: https://kafka.js.org/docs/instrumentation-events#a-name-consumer-a-consumer
@coveralls
Copy link

Pull Request Test Coverage Report for Build 3cfd9218-59df-4aa0-9b37-47f96a0aeb4f

  • 2 of 8 (25.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.08%) to 92.784%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/microservices/client/client-kafka.ts 1 4 25.0%
packages/microservices/server/server-kafka.ts 1 4 25.0%
Totals Coverage Status
Change from base Build 84712c3b-1ec5-44cb-bc70-b040faf818e5: -0.08%
Covered Lines: 6365
Relevant Lines: 6860

💛 - Coveralls

@esahin90
Copy link
Contributor Author

looks like the integration tests failed on the websocket adapter, which was not touched by this PR

@esahin90
Copy link
Contributor Author

will close the PR for now, since there are some other discussion in related PR in #11616

@esahin90 esahin90 closed this Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants