timgit / pg-boss

Queueing jobs in Node.js using PostgreSQL like a boss
MIT License
1.93k stars 153 forks source link

Error: Jest has detected the following 1 open handle potentially keeping Jest from exiting #456

Closed elyran closed 1 week ago

elyran commented 1 month ago

Hi, test is fails because PgBoss (appearantly) doesn't stop correctly.

Reproduction

yarn add @testcontainers/postgresql pg-boss

PgBoss.test.ts:

import {
  PostgreSqlContainer,
  StartedPostgreSqlContainer,
} from '@testcontainers/postgresql';
import PgBoss from 'pg-boss';

const pause = (seconds: number = 99999) =>
  new Promise((r) => {
    setTimeout(r, 1000 * seconds);
  });

describe(__filename, () => {
  let dbContainer: StartedPostgreSqlContainer = null;
  let pgBoss: PgBoss = null;

  beforeAll(async () => {
    dbContainer = await new PostgreSqlContainer('postgres').start();
    await dbContainer.exec([
      'psql',
      '-U',
      dbContainer.getUsername(),
      '-d',
      dbContainer.getDatabase(),
      '-c',
      'CREATE EXTENSION pgcrypto;',
    ]);
    console.log('connectionString:', dbContainer.getConnectionUri());
    pgBoss = new PgBoss(dbContainer.getConnectionUri());
    await pgBoss.start();
  });

  afterAll(async () => {
    await pgBoss.stop({
      // Disable the following lines for a scraier error: "terminating connection due to administrator command"
      destroy: true,
      graceful: false,
    });
    await dbContainer.stop();
    // await pause(4);
  });

  it(`performs operations on a queue`, async () => {
    const NUMBER_OF_JOBS = 5;
    const jobHandler = jest.fn();

    const promises = [];
    for (let i = 0; i < NUMBER_OF_JOBS; i++) {
      promises.push(
        new Promise((resolve) => {
          jobHandler.mockImplementationOnce(() => {
            resolve(null);
          });
        }),
      );
    }

    const queueName = 'queue1';
    await pgBoss.work(queueName, (payload) => {
      jobHandler(payload.data);
      return null;
    });

    for (let i = 0; i < NUMBER_OF_JOBS; i++) pgBoss.send(queueName, { n: i }); // INFO: No need to wait for it, because next we're waiting for `promises` which depends on these `addJob()` calls.

    await Promise.all(promises);

    expect(jobHandler).toHaveBeenCalledTimes(NUMBER_OF_JOBS);

    for (let i = 0; i < NUMBER_OF_JOBS; i++)
      expect(jobHandler).toHaveBeenCalledWith(
        // INFO: Not using `toHaveBeenNthCalledWith` because it doesn't guarantee order of execution.
        expect.objectContaining({ n: i }),
      );
  });
});

Now run it: node --inspect ./node_modules/jest/bin/jest.js --runInBand --detectOpenHandles PgBoss.test.ts

Getting this output:

$ node --inspect ./node_modules/jest/bin/jest.js --runInBand --detectOpenHandles src/queue/PgBoss.test.ts 
Debugger listening on ws://127.0.0.1:9229/8eb4e14c-e7de-4b97-85ea-bad76d3b2bc5
For help, see: https://nodejs.org/en/docs/inspector
  console.log
    connectionString: postgres://test:test@localhost:32993/test

      at Object.<anonymous> (src/queue/PgBoss.test.ts:27:13)

 PASS  src/queue/PgBoss.test.ts (18.075 s)
  /home/user1/tmp/project1/src/queue/PgBoss.test.ts
    ✓ performs operations on a queue (10012 ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        18.138 s
Ran all test suites matching /src\/queue\/PgBoss.test.ts/i.

Jest has detected the following 1 open handle potentially keeping Jest from exiting:

  ●  Timeout

      31 |
      32 |   afterAll(async () => {
    > 33 |     await pgBoss.stop({
         |                  ^
      34 |       destroy: true,
      35 |       graceful: false,
      36 |     });

      at Manager.offWork (node_modules/pg-boss/src/manager.js:289:5)
      at Manager.stop (node_modules/pg-boss/src/manager.js:110:20)
      at PgBoss.stop (node_modules/pg-boss/src/index.js:134:24)
      at Object.<anonymous> (src/queue/PgBoss.test.ts:33:18)

What am I missing?

P.S: Regarding testcontainers, it stops fine in another TypeORM test, and others, so it's unlikely an issue.

BTW, thank you for the package! :)

elyran commented 1 month ago

I do notice this:

user1@user1-pc:~$ docker ps
CONTAINER ID   IMAGE                       COMMAND       CREATED         STATUS         PORTS                                         NAMES
01b0bae5029e   testcontainers/ryuk:0.5.1   "/bin/ryuk"   8 minutes ago   Up 8 minutes   0.0.0.0:32995->8080/tcp, :::32995->8080/tcp   testcontainers-ryuk-fd45566e0de9

It belongs to testcontainers and should not block pg-boss. Let me know if you believe this is a cause.

elyran commented 1 month ago

Did remove testcontainers, for better isolation. Same error.

Spin up a PostgreSQL instance: docker run --rm -e POSTGRES_USER=admin -e POSTGRES_PASSWORD=admin -p 30001:5432 postgres

PgBoss.test.ts

import PgBoss from 'pg-boss';

const pause = (seconds: number = 99999) =>
  new Promise((r) => {
    setTimeout(r, 1000 * seconds);
  });

describe(__filename, () => {
  let pgBoss: PgBoss = null;

  beforeAll(async () => {
    const connectionString = 'postgres://admin:admin@localhost:30001/';
    console.log('connectionString:', connectionString);
    pgBoss = new PgBoss(connectionString);
    await pgBoss.start();
  });

  afterAll(async () => {
    await pgBoss.stop({
      // Disable the following lines for a scraier error: "terminating connection due to administrator command"
      destroy: true,
      graceful: false,
    });
    // await pause(4);
  });

  it(`performs operations on a queue`, async () => {
    const NUMBER_OF_JOBS = 5;
    const jobHandler = jest.fn();

    const promises = [];
    for (let i = 0; i < NUMBER_OF_JOBS; i++) {
      promises.push(
        new Promise((resolve) => {
          jobHandler.mockImplementationOnce(() => {
            resolve(null);
          });
        }),
      );
    }

    const queueName = 'queue1';
    await pgBoss.work(queueName, (payload) => {
      jobHandler(payload.data);
      return null;
    });

    for (let i = 0; i < NUMBER_OF_JOBS; i++) pgBoss.send(queueName, { n: i }); // INFO: No need to wait for it, because next we're waiting for `promises` which depends on these `addJob()` calls.

    await Promise.all(promises);

    expect(jobHandler).toHaveBeenCalledTimes(NUMBER_OF_JOBS);

    for (let i = 0; i < NUMBER_OF_JOBS; i++)
      expect(jobHandler).toHaveBeenCalledWith(
        // INFO: Not using `toHaveBeenNthCalledWith` because it doesn't guarantee order of execution.
        expect.objectContaining({ n: i }),
      );
  });
});

Run the test: node --inspect ./node_modules/jest/bin/jest.js --runInBand --detectOpenHandles PgBoss.test.ts

predragnikolic commented 1 month ago

Does this comment help https://github.com/timgit/pg-boss/issues/233#issuecomment-824627353 ?

I used the default Node test runner. When I added pg-boss, the test never exited. so I added the following:

export const jobs = new PgBoss({
  connectionString: process.env.DATABASE_URL,
  deleteAfterDays: 7,
  // properties to make testing work
  schema: process.env.NODE_ENV === 'test' ? 'public' : 'pgboss',
  // properties to make PGBoss exit
  ...process.env.NODE_ENV === 'test' ? {
+    noSupervisor: true,
+    noScheduling: true
  } : {}
})
export const app = App({
  async setup() {
    await jobs.start()

    return async function cleanup() {
      await jobs.stop({
+        graceful: process.env.NODE_ENV === 'test' ? false : true,
+        destroy: true
      })
    }
  }
  //....
})

After that the test runner successfully exited once all test finished.

timgit commented 1 week ago

This should be resolved in v10 that was recently merged. Please reopen if not.

predragnikolic commented 1 week ago

I'm now migrating to v10.0.1, I'm getting the following error:

  Error [CannotConnectAlreadyConnectedError]: Cannot create a "default" connection because connection to the database already established.

I have describe what I did to make pg-boss work with v9 in this comment

I have followed the migration guide, I updated my code accordingly:


export const jobs = new PgBoss({
  connectionString: process.env.DATABASE_URL,
  deleteAfterDays: 7,
  // properties to make testing work
  schema: process.env.NODE_ENV === 'test' ? 'public' : 'pgboss',
  // properties to make PGBoss exit
  ...process.env.NODE_ENV === 'test' ? {
-    noSupervisor: true,
-    noScheduling: true
+   supervise: false,
+   schedule: false,
  } : {}
})

...

      await jobs.stop({
        graceful: process.env.NODE_ENV === "test" ? false : true,
-        destroy: true
+        close: true, // this is not necessary as it is the default value
+        wait: process.env.NODE_ENV === "test" ? false : true,
      })

When I run tests I now get this error:

  Error [CannotConnectAlreadyConnectedError]: Cannot create a "default" connection because connection to the database already established.

I do not expect the error to happen, because I specified all the properties that I know to configure pg-boss to not wait for the connection to close immediately.

Now I am looking on how to achieve the same behavior as in v9.

Does anyone know maybe know what i am doing wrong?

UPDATE: The bug lied in my code. pg-boss works as expected. My application has one database development, and uses a separate database to run tests. The issues was the following, I saw a 'Migrations are not supported to v10' error, so I just deleted the "pgboss" schema in the test database, and run the tests again.