weyoss / redis-smq

A simple high-performance Redis message queue for Node.js.
MIT License
588 stars 64 forks source link

Inadequate process.exit(1) #33

Closed danielmelogpi closed 4 years ago

danielmelogpi commented 5 years ago

When a connection error occurs (Connection timetout), one of the events will end the main application and there is no way to prevent that. There should be some handler that allowed whoever used the application to handle the failure and decide by himself if the application should be terminated.

File:

weyoss commented 4 years ago

I would not call it inadequate as it was intentionally meant to be. A consumer should not continue to be alive in case of any unexpected error. Think of it like a consumer being alive without sending heartbeats for example.

Fortunately we have shutdown methods that should be used instead of process.exit().

A new RedisSMQ version is out. Please upgrade. If you encounter any problem please let me know.

danielmelogpi commented 4 years ago

Allow me to disagree: the process.exit call is highly invasive to the main logic of the application. A very simple example is that the redis server goes down and all the applications that depend on it shut down because this dependency decided to. This should be a decision of the main consumer code developer. There will be scenarios where other actions can be taken instead of a full shutdown.

As a lib, redis-smq should launch some error, provide ways to handle them elegantly and give this decision to the developer. If it makes sense to the application/flow where it's been used, a process.exit will be called. There will be times this is not the case, like when the flow using redis is non-crucial, for example.

weyoss commented 4 years ago

It depends on the way you are using RedisSMQ consumers.

I agree. If a consumer is running in your app's main process, then sure, the library should NOT shutdown the whole app in case of a Redis error or whatever.

BUT RedisSMQ consumers are considered as workers, stand-alone processes, running in parallel. They can go down at anytime and not only because of process.exit.

Regarding errors handling, the API uses callbacks. You can handle user errors (errors when consuming or producing a message). System fatal errors are handled by the library itself and should not be exposed to the outside world.

As said before, latest RedisSMQ release does not shutdown the whole process upon a library fatal error, but just shutdown the consumer/producer instance. In other words, if you are running a consumer in your app's main process, then upon a library fatal error your app should not go down.