Added default value to prevent error queryCallback is not a function

[drzippie]

fix issue #1860

Set default value to prevent Fatal exception with timeouts in a queryStream

[drzippie]

the fix is a "fast" fix, queryStream uses Cursor and query.callback always is undefined

 FATAL: uncaughtException
    error: {
      "type": "TypeError",
      "message": "queryCallback is not a function",
      "stack":
          TypeError: queryCallback is not a function     

I changed the "fix" to something simple:

queryCallback = typeof query.callback === 'function' ? query.callback : () => {}

This PR only prevents that query.callback (can be not a function) don't be called "as is" if is not a function throwing a fatal exception.

The issue reported is about this "condition" because callback is undefined

Apologies for the first commit, I've been working with a modified version in node_modules to test how to solve this (last solution) and when I went to commit I did it on the fly.

[drzippie]

Added a test:

without the change the test fails:

2560-tests.js
  empty query.callback do not throw fatal exception ✔
Message: queryCallback is not a function
TypeError: queryCallback is not a function
    at Query.query.callback (/workspace/packages/pg/lib/client.js:549:9)
    at Query.handleError (/workspace/packages/pg/lib/query.js:128:19)
    at /workspace/packages/pg/lib/client.js:75:15
    at processTicksAndRejections (internal/process/task_queues.js:79:11)

The test only makes a query (with timeout) setting callback as "undefined"

[drzippie]

Removed new test from PR: Fail other test in lts/carbon (https://travis-ci.com/github/brianc/node-postgres/builds/228104653)

without this new test the build do not fail ¿?

--- test file: packages/pg/test/integration/gh-issues/2560-tests.js (removed from PR) ---

'use strict'

const pg = require('../../../lib')
const helper = require('../test-helper')
if (helper.args.native) return // don't run in native

var Query = helper.pg.Query

const suite = new helper.Suite()

suite.test('empty query.callback do not throw fatal exception', async (done) => {
  const config = {
    query_timeout: 100,
  }
  const notExpectedErr = new Error('queryCallback is not a function')
  var errorReceived = undefined

  try {
    var client = new Client(config)
    await client.connect()
    var sleepQuery = 'select pg_sleep($1)'
    var queryConfig = {
      name: 'sleep query',
      text: sleepQuery,
      values: [2],
      callback: undefined,
    }
    await client.query(new Query(queryConfig))
    await client.end()
  } catch (error) {
    if (error === notExpectedErr) {
      done(new Error('Received error queryCallback is not a function'))
    }
  }
  done()
})

[idandagan1]

Hi, When are you guys planning to merge this?

[charmander]

It needs working tests. If you can fix https://app.travis-ci.com/github/brianc/node-postgres/jobs/511426643, that’ll help.

[brianc]

Hi, When are you guys planning to merge this?

There's some code in the original issue. I can use that to write a test & fix. I'll add this to my TODO list & hopefully get to it this week/weekend. Then a quick lil' release & we should be good.

[avivshafir]

+1 for merging this ASAP, thanks!

[mmtdm]

+1 for merging this ASAP, thanks!

[yaverin]

@brianc, Please provide ETA for this fix - to prevent Fatal exception using "query_timeout" with queryStream.

[karunmotorq]

@brianc Will this be merged? We're encountering an error using pg-promise which this would fix

[brianc]

Will this be merged? We're encountering an error using pg-promise which this would fix

need tests to merge this. Generally I don't merge PRs w/o tests unless they're trivial changes or documentation changes. If you wanna write a test & it passes I'll happily merge it!