contributte / rabbitmq

🐰 RabbitMQ (AMQP, STOMP, MQTT) using BunnyPHP for Nette Framework (@nette).
https://contributte.org/packages/contributte/rabbitmq.html
MIT License
24 stars 25 forks source link

BulkConsumer doesn't produce errors #63

Open jeancz opened 2 years ago

jeancz commented 2 years ago

I have suspicious about BulkConsumer which silently skips throwable errors. I quickly looked into the code and there is the try-catch block in the BulkConsumer.php which probably cause this.

I would prefer to remove this block and let control returning values by man.

https://github.com/contributte/rabbitmq/blob/96a55a92dd805087e8e9f68af58fc5f46c708899/src/Consumer/BulkConsumer.php#L104

bckp commented 1 year ago

Well, it will definitivly swallow the exception, but how you imagine this will work? If exception is thrown, and 2 messages did pass and third have exception and $ waiting, should we send 2 acks and then nacks with exceptions?

TakeruDavis commented 3 weeks ago

I reached similar problem, although first I tried to ask nicely and sent out IConsumer::MESSAGE_REJECT_AND_TERMINATE in the array or responses and when that didn't work, I tried resorting to throwing an exception.

In my case, database was temporarily too busy and EntityManager closed. I wanted to reopen EntityManager, but it doesn't have a "reopen" method, seems it needs to be created again, but Nette DI would just give me the same instance of it and using the static create method wouldn't update DI

Considering messages that already passed, the code should never resort to throwing an exception, it should handle them and pass one of the status codes. On the other hand, BulkConsumer should respect the _AND_TERMINATE part of the response, it should always terminate if at least one of the returned status codes requests it.

bckp commented 1 week ago

Well, problem with _AND_TERMINATE is, it is handled after all messages are completed... Right now,i belive the proper behavior is, you throw exceptions when BulkConsumer try to process data, on every message that have error, bulkConsumer should send NACK, and if one of messages is _AND_TERMINATE, it should kill itself after sending response.

EntityManager can be reopen, and you can even tell Container to flush instance and create new one, you can do so via removeService() method of container.

Right now, I do not contribute to this package anymore, made myself a fork (bckp/rabbitmq) and implemented some ideas I have in mind, now im in process of complete rewriting code, to bring plugins into play, so you can create BulkConsumer, DlxConsumer etc by yourself :) but there is NO ETA, simply when it will be ready, it will be ready :D

bckp commented 1 week ago

But quick look into code, you are indeed right, BulkConsumer does ignore AND_TERMINATE completly... I will think about that