Errored QueryStream causes Client to throw on message receive #1674
Comments
|
I'm able to reproduce this and I think it's related to the internal client query queue. I haven't totally figured out the cause but I suspect it's from the FEBE messages for the // npm install pg pg-query-stream async-main
const pg = require('pg');
const { Client } = pg;
const QueryStream = require('pg-query-stream');
const main = require('async-main').default;
function log(message, ...args) {
console.log(`%s ${message}`, new Date().toISOString(), ...args);
}
function streamToPromise(stream) {
return new Promise((resolve, reject) => {
stream.on('end', () => resolve());
stream.on('error', reject);
});
}
function streamClose(stream) {
return new Promise((resolve) => {
stream.close(resolve);
});
}
main(async () => {
log('Started');
const client = new Client(process.env.DATABASE_URL);
try {
log('Connecting');
await client.connect();
log('Connected');
let stream;
try {
const query = new QueryStream('SELECT $1::uuid AS x', ['bad data']);
stream = client.query(query);
stream.on('data', (data) => {
log('Received data: %j', data);
});
await streamToPromise(stream);
} catch (e) {
log('Error streaming query: %s', e);
} finally {
if (stream) {
log('Closing stream');
// Replace with `stream.close()` to get the error
await streamClose(stream);
log('After close stream');
}
}
log('Before execute new query');
const result = await client.query('SELECT 1 AS x');
log('Result: %j', result.rows);
} finally {
await client.end();
}
});Output when waiting for Replacing the There's probably a fix that could be added to the driver to better handle this situation but knex should also be waiting for the |
|
Do we have a progress on this issue? I think I just hit this error and it seems like this issue is abandoned. |
|
Here's a repro for const pg = require('pg');
const QueryStream = require('pg-query-stream');
const pool = new pg.Pool({
poolSize: 1,
connectionTimeoutMillis: 400,
statement_timeout: 400,
});
(async () => {
const query = new QueryStream('SELECT TRUE');
const client = await pool.connect();
const stream = await client.query(query);
try {
const res = await pool.query('SELECT TRUE');
throw new Error(`Unexpected res (should have failed): ${JSON.stringify(res, null, 2)}`);
} catch(err) {
if(err.message !== 'timeout exceeded when trying to connect') throw new Error(`Unexpected failure: ${err}`);
}
await stream.destroy(); // without this, next attempt at pool.query() will hang
await client.release();
const res = await pool.query('SELECT TRUE AS val');
if(res.rows.length !== 1 || res.rows[0].val !== true) {
throw new Error(`Unexpected res: ${JSON.stringify(res.rows)}`);
}
})();Output (node
|
|
Tested with:
|
|
I believe I am hitting this issue as well. In my case, I am trying to cancel a QueryStream query with Here's a reproduction (pg: 8.11.0, pg-query-stream: 4.5.0, node 18.9.0, postgres: 13.6): const pg = require('pg');
const QueryStream = require('pg-query-stream');
(async () => {
const pool = new pg.Pool();
pool.on('error', (err) => {
console.error('Pool error: ', err);
});
const conn = await pool.connect();
conn.on('error', (err) => {
console.error('Connection error: ', err);
});
const result = await conn.query('SELECT pg_backend_pid() AS pid;');
const { pid } = result.rows[0];
const stream = conn.query(new QueryStream('SELECT pg_sleep(10);'));
stream.on('data', (chunk) => {
console.log('stream data: ', chunk);
});
stream.on('error', (err) => {
console.error('stream error: ', err);
});
// Create a promise that is resolved when the stream is closed
const closed = new Promise((res) => {
stream.on('close', res);
});
// Wait 1 second before cancelling the query
await new Promise((res) => {
setTimeout(() => res(), 1000);
});
await pool.query('SELECT pg_cancel_backend($1);', [pid]);
stream.destroy();
await closed;
// Triggers uncaught exception
await conn.query('SELECT pg_sleep(1);');
conn.release()
await pool.end()
})();This sometimes results in the following uncaught exception (it appears to be a bit of a race condition): Waiting for the stream to be closed should theoretically be a reliable signal that the connection is ready to be used again, since the stream isn't closed until a I think the multiple Given that, I believe #2836 (which looks like it would prevent the second EDIT: Just tested #2836, and it does indeed fix the issue 🎉. |
|
Consider closing now that #2836 is merged. At least two consumers have confirmed that the issue is fixed. |
|
Good point, thanks! |
Hi,
I have encountered a really bizarre issue when using
knexwithpgand query streams. A parameterized QueryStream which fails in the parsing stage (e.g. because of a column type / value type mismatch) breaks subsequent queries on the same connection - either by:result.rows(this is much harder to reproduce and will not be covered here, but it occurs in one of our apps usingknex)Below are two ways to reproduce this issue, one using
knexand another simplified to barepg(which simulates howknexinteracts withpg).Test conditions:
CREATE TABLE events (id UUID);pg@7.4.3pg-query-stream@1.1.1knex@0.14.6(optional, for the knex-enabled test)PostgreSQL 9.6.7(this probably does not matter)What should happen:
What actually happens
TypeError: Cannot read property 'handleRowDescription' of null, as described at the beginning of this bug reportIt does not crash if:
idparam is a valid UUID string like8b0fb8cb-b145-4e41-a847-dfe2f1a47aa1(the second query then also runs OK)rawStream.close();from our stream error handler (but knex.js does it automatically)Overall, I think some state machine inside
pg.Clientbreaks, andactiveQueryis set to null because of it, but I have not been able to pin-point exactly where or why it happens. It seems to be connected to closing the ReadableStream returned fromclient.query().This is especially hard to debug when using
pgwith a pool (such as knex.js's), because the error may go unnoticed until the same client object is re-acquired from the pool and used; this is why I opted for single-connection knex / pg examples above.The text was updated successfully, but these errors were encountered: