RedisQueueDriver mutates EventEmitter prototype indirectly

[vladi-dev]

Describe the bug
Configuration of redisClient is using the package util-promisifyall to promisify all redisClient methods (on redisClient.prototype) but it also promisifies redisClient protototype parent one level up the chain, which is EventEmitter's prototype since redisClient inherits EventEmitter class.

Calling promisifyAll:
https://github.com/statsbotco/cube.js/blob/50f1bbb7aac34ae7c4fd82a86ecee8506b17f4ea/packages/cubejs-query-orchestrator/orchestrator/RedisQueueDriver.js#L2-L7

redisClient inherits EventEmitter:
https://github.com/NodeRedis/node_redis/blob/12265a5079a133d2003bef9cdb0f2deee0251518/index.js#L189

Implementation of utils-promisifyAll takes methods of the input class and creates copies with Async suffix that return a promisified version of the original method.

For me the problem arises when we are trying to use other libraries that use the concept of promisifying all.

For example node-soap:

https://github.com/vpulim/node-soap/blob/0efafa219be15b0efac84c9228c161b8662f720f/lib/client.js#L22-L35

Here you can see promisification with Bluebird.promisifyAll of the Client class which inherits EventEmitter but since EventEmitter has been mutated it adds xxxAsync methods to Client's prototype and then Bluebird.promisifyAll errors out with message: Cannot promisify an API that has normal methods - http://bluebirdjs.com/docs/error-explanations.html#error-cannot-promisify-an-api-that-has-normal-methods

Here is the code that throws the error:
https://github.com/petkaantonov/bluebird/blob/17f69f3b0fa89a0b7a5cfb9a6546d1180a3610e0/src/promisify.js#L49-L65

So to solve that issue for me and possibly for other people that use Bluebird.promisifyAll I will open a PR to use Bluebird instead of util-promisifyall if that is okay with you.
I hope it is not an issue since Bluebird is already in some of sub-dependencies of this package.

To Reproduce
Sandbox: https://codesandbox.io/embed/rmjmmnn73p

Or code:

'use strict';

const util = require('util');
const { EventEmitter } = require('events');

const promisifyAll = require('util-promisifyall');

function RedisClient() {}

util.inherits(RedisClient, EventEmitter);

require('bluebird').promisifyAll(RedisClient.prototype);

console.log(Object.keys(EventEmitter.prototype));

promisifyAll(RedisClient.prototype);

console.log(Object.keys(EventEmitter.prototype));

Expected behavior
Expect that using the library does not modify prototype of the global class EventEmitter.

Version:
0.7.0

[paveltiunov]

Hey @vladi-dev ! Nice catch!
Yeah. It's a bit hacky solution so we need to get rid of it for sure. As we extracted all Redis code to separate classes it shouldn't be a problem just to call promisify on specific instance methods or something like it.

[vladi-dev]

Hey @vladi-dev ! Nice catch!
Yeah. It's a bit hacky solution so we need to get rid of it for sure. As we extracted all Redis code to separate classes it shouldn't be a problem just to call promisify on specific instance methods or something like it.

Not exactly understand what is the solution you are proposing

Meanwhile I opened a PR#85 to fix that, maybe could be a temporary fix

Let me know if I can help with a better solution

[vladi-dev]

@paveltiunov please take a look at #89