timgit / pg-boss

Queueing jobs in Postgres from Node.js like a boss
MIT License
2.13k stars 158 forks source link

Clearer assertion message #44

Closed haoliangyu closed 6 years ago

haoliangyu commented 6 years ago

Right now the assert() function is widely used in the code to validate execution results. This is very nice but sometimes the assertion message is not very clear because the assertion is written in a compact form.

Example

In the manager.setStateForJob(), an assertion is used to determine whether the update is successful:

assert(result.rowCount === 1, `${actionName}(): Job ${id} could not be updated.`);

If it fails, it will print something likes actual: true, actual: false. This is not very clear since the result.rowCount could be zero or larger than one in this case.

If such assertion could be rewritten using assert.equal() to provide more precise error message, it would be very helpful for debugging.

timgit commented 6 years ago

Which issue are you debugging? This assertion is merely to throw an error for downstream tests. It's not what I use for testing.

haoliangyu commented 6 years ago

I am debugging a job status update issue. The error message in the application log is like

{ AssertionError [ERR_ASSERTION]: complete(): Job d3bb5eb2-e1e2-11e7-aebc-b73a7a1bc7d6 could not be updated.
    at /opt/hub-indexer/node_modules/pg-boss/lib/manager.js:318:9
    at tryCatcher (/opt/hub-indexer/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/opt/hub-indexer/node_modules/bluebird/js/release/promise.js:512:31)
    at Promise._settlePromise (/opt/hub-indexer/node_modules/bluebird/js/release/promise.js:569:18)
    at Promise._settlePromise0 (/opt/hub-indexer/node_modules/bluebird/js/release/promise.js:614:10)
    at Promise._settlePromises (/opt/hub-indexer/node_modules/bluebird/js/release/promise.js:693:18)
    at Async._drainQueue (/opt/hub-indexer/node_modules/bluebird/js/release/async.js:133:16)
    at Async._drainQueues (/opt/hub-indexer/node_modules/bluebird/js/release/async.js:143:10)
    at Immediate.Async.drainQueues (/opt/hub-indexer/node_modules/bluebird/js/release/async.js:17:14)
    at runCallback (timers.js:800:20)
    at tryOnImmediate (timers.js:762:5)
    at processImmediate [as _immediateCallback] (timers.js:733:5)
  generatedMessage: false,
  name: 'AssertionError [ERR_ASSERTION]',
  code: 'ERR_ASSERTION',
  actual: false,
  expected: true,
  operator: '==' }

This error is thrown by the line I quoted at my previous post. I am not sure about the cause of this error yet.

haoliangyu commented 6 years ago

Though it is unlikely the problem of pg-boss, I would be happier to see something like expected: 1, actual: 0 or expect: 1, actual: 2 :-)

timgit commented 6 years ago

In this case, I think complete(): Job d3bb5eb2-e1e2-11e7-aebc-b73a7a1bc7d6 could not be updated. is sufficient to explain the cause. Sorry that this is confusing. I'm really only using assert from node core instead of new Error() out of convenience. It's not meant to convey assertion metadata.

haoliangyu commented 6 years ago

No problem. I have resolved my problem. It is not caused by pg-boss 😃