nest icon indicating copy to clipboard operation
nest copied to clipboard

RabbitMQ Unconfirmed Server Message on test Close

Open jmcdo29 opened this issue 3 years ago • 1 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Current behavior

When running an e2e test and using a ClientRMQ isntance, generated with ClientsModule.register, occasionally a message will be left unconfirmed in the rabbit mq server instance, causing the following error:

~/node_modules/.pnpm/[email protected][email protected]/node_modules/amqp-connection-manager/dist/cjs/ChannelWrapper.js:365
                    message.reject(new Error('Channel closed'));
                                   ^

Error: Channel closed
    at ~/node_modules/.pnpm/[email protected][email protected]/node_modules/amqp-connection-manager/dist/cjs/ChannelWrapper.js:365:36
    at Array.forEach (<anonymous>)
    at ~/node_modules/.pnpm/[email protected][email protected]/node_modules/amqp-connection-manager/dist/cjs/ChannelWrapper.js:361:43
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

Note: this does not happen every time, sometimes I can run the test 10 times and see no error. Other times its immediate.

Minimum reproduction code

https://github.com/jmcdo29/rabbitmq-test-server-close-error

Steps to reproduce

  1. clone this repo
  2. install dependencies
  3. run the test:e2e:jest:fail and test:e2e:uvu:fail npm scripts and see that eventually there is an error a) These scripts are written specifically as while loops to wait for the failure so you don't have to keep running them

Expected behavior

Upon closing the server, there should be no unconfirmed messages so that there's no error.

I think this is happening in a test scenario because things are closing too quickly. I've tried this with both jest and uvu and have confirmed it does reproduce in both situations.

Package

  • [ ] I don't know. Or some 3rd-party package
  • [x] @nestjs/common
  • [x] @nestjs/core
  • [X] @nestjs/microservices
  • [ ] @nestjs/platform-express
  • [ ] @nestjs/platform-fastify
  • [ ] @nestjs/platform-socket.io
  • [ ] @nestjs/platform-ws
  • [x] @nestjs/testing
  • [ ] @nestjs/websockets
  • [ ] Other (see below)

Other package

amqp-connection-manager : 4.1.7 uvu : 0.5.6 jest : 28.1.2

NestJS version

9.1.2

Packages versions

platform-express version : 9.1.2
microservices version    : 9.1.2
schematics version       : 9.0.3
testing version          : 9.1.2
common version           : 9.1.2
core version             : 9.1.2
cli version              : 9.1.4
amqp-connection-manager  : 4.1.7 

Node.js version

16.17.0, 18.10.0

In which operating systems have you tested?

  • [ ] macOS
  • [ ] Windows
  • [X] Linux

Other

This is related to #10306 I've spent some time looking into it and pared it down as much as I can. I can't seem to find the reason that there's a message left in the unconfirmed queue on close. This may have to do with using a client and server in the same Nest application, and if that's the case we should just document that that's not an expected scenario.

The main use case for this scenario is e2e testing the server without having to create a separate client application. I'm totally fine if that's the solution too, just wanted to make sure if it is or not.

jmcdo29 avatar Oct 02 '22 18:10 jmcdo29

Not sure if this is something we could fix on the framework level though 🤔 I'll think & see if I can come up with any quick fix. If you have any ideas, go ahead and create a PR 🙌

kamilmysliwiec avatar Oct 03 '22 07:10 kamilmysliwiec

Hi, @jmcdo29 there is missing await, afterAll should look like

  afterAll(async () => {
    await client.close();
    await app.close();
  });

also to avoid other issues, use --runInBand for jest testing

JakubMrzyglod avatar Nov 27 '22 12:11 JakubMrzyglod

Hi @JakubMrzyglod client.close() either return any or void adding await is redundant.

tushar1998 avatar Dec 07 '22 16:12 tushar1998