knex: Knex is not clearing timeouts after they have become unnecessary

Environment

Knex version: 0.20.10 Database + version: postgres 12.1 OS: Linux Node: 12.15.0 Jest: 25.1.0

Bug

Jest prints a warning about open handles when a test creates and rollbacks a transaction. When I comment out transacting code, the warning disappears. The connection to postgres database is closed in afterAll call.

Jest did not exit one second after the test run has completed.

This usually means that there are asynchronous operations that weren't stopped in your
 tests. Consider running Jest with `--detectOpenHandles` to troubleshoot this issue.

--detectOpenHandles flag does not detect anything.

Here is the reproduction code:

const Knex = require('knex');

const db = Knex({
  client: process.env.DB_TYPE || 'pg',
  connection: {
    host: process.env.DB_HOST || 'localhost',
    port: Number(process.env.DB_PORT) || 5432,
    user: process.env.DB_USER || 'postgres',
    password: process.env.DB_USER || 'postgres',
    database: process.env.DB_DATABASE || 'postgres',
  },
});

describe('test', () => {
  afterAll(async () => {
    await db.destroy();
  });

  it('transacts', async () => {
    const t = await db.transaction();

    await t.rollback();
  });
});

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Comments: 15 (4 by maintainers)

Most upvoted comments

@briandamaged It makes sense now!

let timeoutId;

const timeoutPromise = new Promise((res, rej) => {
  timeoutId = setTimeout(() => rej(new KnexTimeoutException), 5000);
});

const queryPromise = this.query(conn, 'ROLLBACK', 2, error).then(result => {
  clearTimeout(timeoutId);
  return result;
});

return Promise.race([timeoutPromise, queryPromise]).catch(err => { 
  if (!(err instanceof KnexTimeoutError)) { 
    return Promise.reject(err); 
  } 
  this._rejecter(error); 
});

This modification would solve my problem. But what if timeout rejects first? In my code the query handler would be cleaned up by db.destory(), but in general case?

@elhigu : Makes sense. In that case, we might just need to cancel the setTimeout(..) call like @prk3 suggested.