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

fix(microservices): Ensure noAck when consume reply queue channel #11862

Closed
wants to merge 1 commit into from
Closed

fix(microservices): Ensure noAck when consume reply queue channel #11862

wants to merge 1 commit into from

Conversation

VeryCrazyDog
Copy link

To fix RabbitMQ server returning error "PRECONDITION_FAILED - reply consumer cannot acknowledge"

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?

In NestJS v10, when we pass noAck: false to RabbitMQ transporter, upon connection to RabbitMQ server, it will fail with the following error message:

Error: Operation failed: BasicConsume; 406 (PRECONDITION-FAILED) with message "PRECONDITION_FAILED - reply consumer cannot acknowledge"

This bug does not show up in NestJS v9 because in v9 noAck in the following line is always true.

const noAck = this.getOptionsProp(this.options, 'noAck', RQM_DEFAULT_NOACK);

This is due to a bug in ClientProxy.getOptionsProp() in v9 show in the following, which always returns default true when false is passed in the options object.

return (obj && obj[prop]) || defaultValue;

Issue Number: N/A

What is the new behavior?

It is able to connect to RabbitMQ server successfully.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Since I am running Windows, I am not able to run the formatter, linter and test. Hopefully my change didn't break anything.

To fix RabbitMQ server returning error "PRECONDITION_FAILED - reply consumer cannot acknowledge"
@coveralls
Copy link

Pull Request Test Coverage Report for Build 2e7b935a-a802-4703-915c-433a77f4177e

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.001%) to 92.843%

Totals Coverage Status
Change from base Build 60a45f7f-f5f0-474c-9154-bafe02cb2141: -0.001%
Covered Lines: 6343
Relevant Lines: 6832

💛 - Coveralls

@kamilmysliwiec
Copy link
Member

This bug does not show up in NestJS v9 because in v9 noAck in the following line is always true.

Which was unexpected.

If you set noAck: false, why would you want the framework to ignore your choice and pass true anyway?

@VeryCrazyDog
Copy link
Author

VeryCrazyDog commented Jun 22, 2023

My bad, looks like it is my problem of reusing the same options for both ServerRMQ and ClientRMQ. In ServerRMQ I need noAck to be false to use the dead letter exchange.

Since the error message stated that acknowledgement is impossible on reply consumer, my initial thought is that the framework should always enable noAck on reply consumer and ignoring the noAck options.

Anyway given that it is possible to avoid the precondition fail by revising the options, I think keeping the existing implementation is more future proof. I am closing this PR. Thanks a lot spending time on this PR.

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.

3 participants