Fix null reference in pg-cursor #handleError

[juneidy]

No description provided.

[charmander]

Can you explain the events that result in this.connection being null at that time? (They’ll probably need to end up in a test, too.)

[juneidy]

The simplest case I can reproduce the issue:

const QueryStream = require('pg-query-stream');
const { Client } = require('pg');

const client = new Client({
	host: 'localhost',
	port: '5432',
	user: 'juneidy',
	database: 'juneidy',
	password: 'secret'
});

const stmt = 'SELECT * FROM goose;';

(async function(){
	await client.connect();

	client.query(stmt);

	client.query(new QueryStream(stmt));

	await client.end();
})();

The console output:

root@83fec2234d8d:/data/pg# node index
/data/node_modules/pg-cursor/index.js:141
  this.connection.removeListener('noData', this._ifNoData)
                  ^

TypeError: Cannot read property 'removeListener' of null
    at Cursor.handleError (/data/node_modules/pg-cursor/index.js:141:19)
    at /data/node_modules/pg/lib/client.js:66:13
    at processTicksAndRejections (internal/process/task_queues.js:79:11)
root@83fec2234d8d:/data/pg#

[juneidy]

To expand on this, the usecase would be autocomplete scenario.

The autocomplete (throttled) input makes request to server to get potential autocomplete. But as user type more characters, you'd want to terminate previous query immediately to save on resources.

[juneidy]

Hi @charmander , just wondering if there's anything wrong with my pull request?

[charmander]

@juneidysoo I haven’t had the chance to investigate yet, but whatever the bug is, it’s unlikely the PR as written now (changing the code to ignore a state it didn’t previously expect) is the correct fix for it.

[brianc]

Yeah if you can include a test case which reproduces the issue then we can look at the code, suggest any changes, etc...but almost without exception I don't merge code without tests. Having to maintain contributed code for years after its landed and the contributor has moved on is incredibly difficult without test coverage.