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

Redis connection error event handler is set ONLY after connection is established #666

Closed
YimingIsCOLD opened this issue Jul 28, 2021 · 7 comments
Labels
bug Something isn't working

Comments

@YimingIsCOLD
Copy link

Bull Version: 1.40.1

const queue = new Queue('test', {
    connection: {
        host: 'localhost',
        port: 6379,
    },
});

queue.on('error', () => console.log('Something went wrong!'));

An unhandled error event is logged by IORedis when creating a new queue with no redis server available.

[ioredis] Unhandled error event: Error: connect ECONNREFUSED 127.0.0.1:6379
    at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1142:16)

If there is a redis server available during the queue creation and somehow lost connection to the redis server, the queue will emit an error with Something went wrong!.

After some investigation, I found out that the error event hook is set after a initializing has been resolved. However, in the case where there is no redis server available during queue creation, it will be stuck in waitUntilReady promise and thus not setting the error event hook.

I will suggest to place the hook at here instead of after the initializing has been resolved. What you do think?

@manast
Copy link
Contributor

manast commented Jul 29, 2021

It sounds like a reasonable change.

@manast manast added the bug Something isn't working label Jul 29, 2021
@YimingIsCOLD
Copy link
Author

Alright! I will create a PR to address this.

@YimingIsCOLD
Copy link
Author

@manast How do I run the test locally? I got a fresh copy of the repo, did yarn and spin up a redis server with docker. When I tried to run the test with yarn test, it always failed with the following logs. I tried digging around but can't seems to find the root cause.

  connection
Error: ECONNRESET
    at Context.<anonymous> (/Users/yiming/workspace/bullmq/src/test/test_connection.ts:56:32)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    ✓ should recover from a connection loss (39ms)
Error: ECONNRESET
    at Worker.<anonymous> (/Users/yiming/workspace/bullmq/src/test/test_connection.ts:105:36)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    ✓ should handle jobs added before and after a redis disconnect
[ioredis] Unhandled error event: Error: connect ECONNREFUSED 127.0.0.1:1234
    at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1142:16)
    ✓ should fail if redis connection fails
    1) should fail if redis connection fails


  42 passing (4s)
  1 failing

  1) connection
       should fail if redis connection fails:
     Uncaught Error: Connection is closed.
      at Redis.endHandler (src/classes/redis-connection.ts:74:16)
      at Object.onceWrapper (node:events:514:26)
      at Redis.emit (node:events:394:28)
      at processTicksAndRejections (node:internal/process/task_queues:78:11)

@manast
Copy link
Contributor

manast commented Jul 29, 2021

That is quite strange considering there are 231 tests in total...

@roggervalf
Copy link
Collaborator

@YimingIsCOLD could you please try the last version, a fix was merged

@lukepolo
Copy link
Contributor

lukepolo commented Aug 9, 2021

i just tired this , and it works kinda.....

 const connection = this.redisManager.getConnection(this.config.redisConnection);
    const bullQueueOptions: BullQueueOptions = {
      prefix: this.config.prefix,
      connection
    };

    this.queue = new BullMQ(this.queueName, bullQueueOptions || {}).on('error', (error) => {
      logger.error("queue error", error.message)
    })
 BullMQQueue <error> queue error Connection is closed. +0ms
(node:68495) UnhandledPromiseRejectionWarning: Error: Connection is closed.
    at Redis.errorHandler (/Users/luke/Code/work/qx-microservice-framework/qx-microservice-template/node_modules/bullmq/dist/classes/redis-connection.js:60:24)
    at Object.onceWrapper (events.js:421:26)
    at Redis.emit (events.js:326:22)
    at Redis.silentEmit (/Users/luke/Code/work/qx-microservice-framework/qx-microservice-template/node_modules/ioredis/built/redis/index.js:544:26)
    at Socket.<anonymous> (/Users/luke/Code/work/qx-microservice-framework/qx-microservice-template/node_modules/ioredis/built/redis/event_handler.js:190:14)
    at Object.onceWrapper (events.js:421:26)
    at Socket.emit (events.js:326:22)
    at emitErrorNT (internal/streams/destroy.js:92:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
    at processTicksAndRejections (internal/process/task_queues.js:84:21)
(node:68495) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:68495) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
(node:68495) UnhandledPromiseRejectionWarning: Error: Connection is closed.
    at Redis.errorHandler (/Users/luke/Code/work/qx-microservice-framework/qx-microservice-template/node_modules/bullmq/dist/classes/redis-connection.js:60:24)
    at Object.onceWrapper (events.js:421:26)
    at Redis.emit (events.js:326:22)
    at Redis.silentEmit (/Users/luke/Code/work/qx-microservice-framework/qx-microservice-template/node_modules/ioredis/built/redis/index.js:544:26)
    at Socket.<anonymous> (/Users/luke/Code/work/qx-microservice-framework/qx-microservice-template/node_modules/ioredis/built/redis/event_handler.js:190:14)
    at Object.onceWrapper (events.js:421:26)
    at Socket.emit (events.js:326:22)
    at emitErrorNT (internal/streams/destroy.js:92:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
    at processTicksAndRejections (internal/process/task_queues.js:84:21)
(node:68495) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 2)

The Connection is closed is fine but still seems to still have a unhandled rejection.

not a huge deal~

@roggervalf
Copy link
Collaborator

@lukepolo, yes you are right, that unhandled promise rejection has some time in the base code, but the fix was preventing the unhandled error event as @YimingIsCOLD mentioned. I'll need to make more investigation about the unhandled rejection.

@manast manast closed this as completed Aug 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants