Subscribe handlers should only return void logged if using RpcException or RpcExceptionFilter

[Nathan-Nesbitt]

I've created the following using @golevelup/nestjs-rabbitmq as a demo of the issue:

@RabbitSubscribe({
    exchange: "EXCHANGE",
    routingKey: "EXCHANGE",
    queue: "EXCHANGE",
  })
  public async handler() {
   // For the sake of ensuring the queued message doesn't infinitely retry
   new Nack(false);
   throw new RpcException("Exception that will be caught");
  }

I get the following every time I handle an exception using the proposed RpcException handler by NestJS:

WARN [AmqpConnection] Received response: [{}] from subscribe handler [handler]. Subscribe handlers should only return void

On the other hand if you throw a standard error it doesn't appear to show up

@RabbitSubscribe({
    exchange: "EXCHANGE",
    routingKey: "EXCHANGE",
    queue: "EXCHANGE",
  })
  public async handler() {
   throw new Error("Exception that will be caught");
  }

Not sure how to narrow this down as I can only reproduce this when specific errors are thrown in the body of the subscribe handler. No matter what is done, this gets thrown if you use either @Catch() with and RpcExceptionFilter or by throwing an RpcException in the subscriber.

[underfisk]

@ttshivers Are you able to give us some hand on this one?

@Nathan-Nesbitt What's the expected behavior? Would you be able to create a PR with the actual fix?

[Nathan-Nesbitt]

I can tell you what the issue/fix might be.

I tried setting up a global exception handler with NestJS. Each time I would trigger and catch any exception with it, a second log was printed from your library. (See the log above)

For NestJS, seems next() within the handler is returning {} instead of void which is causing issues, as you've got a line of code that logs a warning each time the return type is anything but void.

The easiest fix is probably to remove the logging for that if you support NestJS exception handling. The better fix probably had to do with why it's returning {} and if that's intended or not

[Nathan-Nesbitt]

https://github.com/golevelup/nestjs/blob/master/packages%2Frabbitmq%2Fsrc%2Famqp%2Fconnection.ts#L460

This is the location, and it's repro-able using the basic implementation of the RPC exception handler from nestJS and your subscription setup.

[underfisk]

@Nathan-Nesbitt Thanks for all the information.
What you're saying makes sense but this could have been an intentional API design decision as a subscription in fact should never return. Your use case of providing a nack should never happen manually therefore, you will receive a weird behaviour when throwing an error and a nack one after the other.
Unfortunately, i don't have the time to tweak this and make it so the end user can control this behaviour but if you could help us improving the existing API extending it so that the consumer can override the default behaviour i would be happy to review 😄

[underfisk]

@Nathan-Nesbitt This behavior seems to be intentional, looking at the code if a subscriber provides a response we acknowledge it.
I would need a reproduction but if you'd like to contribute with a fix I would welcome it!