Bull: Create queue with separate port and host throws error

[liquorvicar]

Since the release of 1.1.0, creating a queue with a separate port and hostname like this:

Queue('QueueName', 6379, 'redishost.example.com');

fails with an error: Error: Expected an object as redis option

Is this an intended change? In which case, the docs needs to be updated. Or is this a bug?

[bradvogel]

The docs need to be updated, and the release should probably be bumped to v2:

    Queue(name, {
      redis: {
        port: redisParams.port,
        host: redisParams.hostname,
        opts: {
          auth_pass: redisParams.password,
          keyPrefix: 'mykey'
        }
      }
    });

[xycloud]

just read the source code

[manast]

I have missed this breaking change. I think we should keep the old way to instantiate the queue.

[liquorvicar]

@manast I may have time to submit a PR over the next few days if that would help...

@xycloud That is really not a helpful comment.

[manast]

hmm, I checked the code and tested it and your example above works as with older versions. No exceptions thrown unless you passed the port argument as string.

[manast]

This is the code that throws that exception:
https://github.com/OptimalBits/bull/blob/master/lib/queue.js#L76-L77

[shane-walker]

We had an issue with this ourselves when it was installed automatically because of our semver rules and we had to spend some time narrowing it down to the new version of bull.

It seems to me that the better way to test this would be to perform a URL test on the variable rather than checking if it is a string. Anyone that loads variables from the environment would run into this issue.

[liquorvicar]

@manast Yes, you're right, it only breaks if you pass the port in as a string. But it is still technically a breaking change as it worked fine before (as @shane-walker says anyone using an env var directly would hit this issue). Given JS's looseness with types, it probably should accept a string for the port?

[manast]

I can add a parseInt(port) == port, and use that to determine if it is a valid port or not. That should do it, what do you think?

[bradvogel]

Could you instead do if (arguments.length === 2)? That would be easier to read in code.

[liquorvicar]

@manast I created a PR that fixes this. Would welcome comments.