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

Calling kue.Job.get() with undefined or null causes uncaught exception #989

Open
niklassletteland opened this issue Nov 21, 2016 · 1 comment

Comments

@niklassletteland
Copy link
Contributor

Calling kue.Job.get() and passing an undefined or null id causes an uncaught exception:

FATAL: UncaughtException: Cannot read property 'toString' of undefined TypeError: Cannot read property 'toString' of undefined
   at Redis.client.createFIFO (/app/node_modules/kue/lib/redis.js:82:26)
   at Function.exports.get (/app/node_modules/kue/lib/queue/job.js:173:20)

from this entry point: https://github.com/Automattic/kue/blob/master/lib/queue/job.js#L161

the get() function does job.zid = client.createFIFO(id);.

The uncaught exception happens on this line in createFIFO(): (https://github.com/Automattic/kue/blame/master/lib/redis.js#L82)

var idLen = '' + id.toString().length;

Granted, it shouldn't be passing in undefined as id to Job.get() initially, and I can add a check to make sure that doesn't happen in my app code.

That being said, it might be better not to allow this use case to generate an uncaught exception in the Kue code, and instead call the return fn with a validation error

niklassletteland added a commit to niklassletteland/kue that referenced this issue Nov 21, 2016
issue Automattic#989 - calling `Job.get()` with an `id` param that is null or undefined will result in an uncaught exception.
this adds a validation check to prevent this, and returns an application level error instead
@niklassletteland
Copy link
Contributor Author

submitted a proposed fix: #990

adds:

  if (id === null || id === undefined) {
    return fn(new Error('invalid "id" param in Job.get()'));
  }

check at the top of the Job.get() function to return an error instead of causing exception

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant