`pool.connect` raises `Client has an active query.` and the connection never releases

[mekto]

As in the example below even if we catch an error before returning from pool.connect it throws Client has an active query. and the connection never releases (pendingReleaseConnections increases every time such error occours).

await pool.connect(async (connection) => {
  try {
    const [timestamp, random] = await Promise.all([
      connection.oneFirst(sql.unsafe`SELECT CURRENT_TIMESTAMP`),

      (async function() {
        throw new Error('random error');
        return Math.random();
      })(),
    ]);

    console.log({ timestamp, random });
  } catch (err) {
    console.log('caugth an error');
  }
});

Stack trace:

108 |                     }
109 |                     if (currentState !== 'ACQUIRED') {
110 |                         throw new Error('Client is not acquired.');
111 |                     }
112 |                     if (activeQueryPromise) {
113 |                         throw new Error('Client has an active query.');
                                    ^
error: Client has an active query.
      at .../node_modules/@slonik/driver/dist/factories/createDriverFactory.js:113:31
      at internalRelease (.../node_modules/@slonik/driver/dist/factories/createDriverFactory.js:101:53)
      at release .../node_modules/@slonik/driver/dist/factories/createDriverFactory.js:140:38)
      at .../node_modules/slonik/dist/factories/createConnection.js:108:26

If we replace pool.connect with pool.transaction then it throws Expected `activeQueryPromise` to be set, which could be related to #648.

In this case the stack trace is:

183 |                         }
184 |                         try {
185 |                             activeQueryPromise = query(sql, values);
186 |                             const result = await activeQueryPromise;
187 |                             if (!activeQueryPromise) {
188 |                                 throw new Error('Expected `activeQueryPromise` to be set.');
                                                ^
error: Expected `activeQueryPromise` to be set.
      at .../node_modules/@slonik/driver/dist/factories/createDriverFactory.js:188:43
      at processTicksAndRejections (native:7:39)

Real life example

To provide more context this is real life example from an app build using Sveltekit. This is an server hook (aka middleware) and runs before each request. This is a good place to provide database connection to the app endpoints.

// hooks.server.js

/** @type {import('@sveltejs/kit').Handle} */
export async function handle({ event, resolve }) {
  const pool = await reusePoolInstance();

  return pool.connect(async (db) => {
    event.locals.db = db;
    event.locals.user = await getSessionUser(event);

    return resolve(event);  // <-- returns Response object without exception
  });
}

The resolve function returns Promise<Response> and no exception is raised. But what the resolve function does is similar to Promise.all([...]) part from my example at the top. E.g. it tries to load parallel data from a page and its layout and both can load some data from database using provided connection in the hook. If any of them raises en exception (which are caught be the framework before returning from pool.connect) the pool.connect raises Client has an active query. and full page request resolves to 500.

What is alse common is to call throw redirect(302, '/login') in the endpoints. The framework handels it and returns proper Response object, but it causes the 'Client has an active query.' error.

We could wrap pool.connect in the try ... catch block like this:

/** @type {import('@sveltejs/kit').Handle} */
export async function handle({ event, resolve }) {
  const pool = await reusePoolInstance();

  let response;
  
  try {
    return await pool.connect(async (db) => {
      event.locals.db = db;
      event.locals.user = await getSessionUser(event);
  
      response = await resolve(event);  // <-- returns Response object without exception
      return response;
    });
  } catch (err) {
    if (err === 'Client has an active query.') {
      return response;
    }
    throw err;
  }
}

and the app handles errors and redirects correctly without the 500 error. But, when pool.connect raises an error, pendingReleaseConnections increases every time and after 10 such cases it is unable to aquire new connection and app hangs and needs to be restarted.

So I consider it as a bug in slonik. Possible workaround could be to have any way to manually realese such failed connection.