fastify-postgres icon indicating copy to clipboard operation
fastify-postgres copied to clipboard

pg-pool force closing fastify instance

Open yukha-dw opened this issue 2 years ago • 5 comments

Prerequisites

  • [X] I have written a descriptive issue title
  • [X] I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.x.x

Plugin version

5.2.2

Node.js version

18.x

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

22.04

Description

Hi, I just found out this weird behavior about pg-pool and it turned out a known issue on its repo. When we shutdown/restart PostgreSQL instance, this module will force close our Fastify app! As my understanding, this happens due to lack of error handler in pg-pool, resulting unexpected termination event when we wrap the query logic using try catch. Due to my poor explanation, please read this issue: https://github.com/brianc/node-postgres/issues/2439.

Steps to Reproduce

  1. Deploy PostgreSQL
  2. Deploy Fastify Instance with PostgreSQL support
  3. Shutdown PostgreSQL

Expected Behavior

No response

yukha-dw avatar Dec 26 '23 11:12 yukha-dw

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

mcollina avatar Dec 26 '23 12:12 mcollina

I would like to, but I am not sure how to test it reliably. Perhaps someone else has better idea to do it. The bug itself can be done by adding the EventHandler after creating the pool. This way, @fastify/postgres will only return error log instead of terminating the app.

https://github.com/fastify/fastify-postgres/blob/ce90bc6b72bbf01b24984ec3a6ac40d407daeffb/index.js#L90

const pool = new pg.Pool(options)

pool.on('connect', (client) => {
  client.on('error', (err) => {
    console.error(err)
  })
})
pool.on('error', (err) => {
  console.error(err)
})

yukha-dw avatar Dec 27 '23 02:12 yukha-dw

Adding a few log lines would be perfect.

mcollina avatar Dec 27 '23 08:12 mcollina

I would like to, but I am not sure how to test it reliably. Perhaps someone else has better idea to do it.

In this case I would try testcontainers: https://node.testcontainers.org/quickstart/ It is very handy because you can start and stop a container programmatically.

Eomm avatar Dec 28 '23 09:12 Eomm

How about using a simpler solution?

'use strict'

const net = require('node:net');

const addrRegex = /^(([a-zA-Z\-\.0-9]+):)?([0-9]{1,5})$/;

class SocketProxy {
  #connection = null;
  #from = null;
  #to = null;

  constructor(from, to) {
    this.#from = addrRegex.exec(from);
    this.#to = addrRegex.exec(to);
  }

  start() {
    if (this.#connection) return Promise.resolve();
    return new Promise((resolve, reject) => {
      try {
        this.#connection = net.createServer(from => {
          const to = net.createConnection({
            host: this.#to[2],
            port: this.#to[3]
          });
          from.pipe(to);
          to.pipe(from);
        }).listen(this.#from[3], this.#from[2], () => {
          resolve();
        });
      } catch (err) {
        this.#connection = null;
        reject(err);
      }
    })
  }

  stop() {
    this.#connection.close();
    this.#connection = null;
  }
}

// Example usage to forward requests from port 8080 to 3000
// node ./test/socket-proxy.js 8080 3000
const proxy = new SocketProxy(process.argv[2], process.argv[3]);
proxy.start().then(() => {
  console.log('Proxy started')

  console.log('Shutting down in 5 seconds')
  setTimeout(() => {
    proxy.stop();
  }, 5000)
})

So a test would consist of following steps:

  1. use npm package get-port to determine an available free port
  2. start a socketproxy like above and forward requests from the free port to the postgres port 5432
  3. modify postgresql connection string to use the free port
  4. set up your pool
  5. stop socket proxy and provoke the issue.

Uzlopak avatar Dec 28 '23 11:12 Uzlopak