porsager / postgres

Postgres.js - The Fastest full featured PostgreSQL client for Node.js, Deno, Bun and CloudFlare
The Unlicense
7.39k stars 267 forks source link

CONNECTION_CLOSED on subscribe() #757

Open dvv opened 9 months ago

dvv commented 9 months ago

I use standard

const { unsubscribe: _unsubscribe } = await sql.subscribe(
  "*",
  (row, info) => {
    console.log("SUB", row, info)
  },
  () => {
    console.log("SUB!")
  },
)

and it works well.

When postgres goes down I immediately get

Unexpected error during logical streaming - reconnecting Error: write CONNECTION_CLOSED ...
    at closed (https://deno.land/x/postgresjs@v3.4.2/src/connection.js:443:57)
    at https://deno.land/x/postgresjs@v3.4.2/polyfills.js:138:30
    at Array.forEach (<anonymous>)
    at call (https://deno.land/x/postgresjs@v3.4.2/polyfills.js:138:16)
    at closed (https://deno.land/x/postgresjs@v3.4.2/polyfills.js:127:5)
    at success (https://deno.land/x/postgresjs@v3.4.2/polyfills.js:109:7)
    at eventLoopTick (ext:core/01_core.js:189:11) {
  code: "CONNECTION_CLOSED",
  errno: "CONNECTION_CLOSED",

When postgres goes up then I get everything working except for the above subscription -- it never reconnects.

Louis-Tian commented 9 months ago

Just got caught with the same error. And it's causing my database to bloat with the WAL piling up...

Louis-Tian commented 8 months ago

I believe the key problem here is the "subscribe" creates a temporary replication slot on the database. As soon as the database shuts down, that original replication slot is gone forever. Postgres.js at the moment is not smart enough to recreating a new replication slot when the database is back online.

@porsager What is really problematic at the moment is that there is no way for us to handle it ourselves in this situation. As the error is being completely swallowed by the stream here. https://github.com/porsager/postgres/blob/6f20a4820c683b33e7670b606d8daf5670f4b973/src/subscribe.js#L99-L107

I think even just rethrow the error would be better. Because then I can catch that error using the global "UncaughtException" event. And even if I don't, the program would completely crash and restarted by the container runtime or process manager alike.

dvv commented 8 months ago

@Louis-Tian right. I also drafted https://github.com/porsager/postgres/compare/master...dvv:postgres:master to move "close" logic uplevel.