Skip to content

Postgres adapter: race condition in setupPgNotify, missing error handling, no accessors #1154

@hubyrod

Description

@hubyrod

Bug: Multiple issues in PostgresExternalService change detection setup

Version: @skip-adapter/postgres@0.0.19

1. Race condition in setupPgNotify

open_instances.add(instance) is called before the CREATE FUNCTION / LISTEN / CREATE TRIGGER queries execute:

// Current code (simplified from src/index.ts):
const setupPgNotify = async () => {
  if (!this.open_instances.has(instance)) {
    this.open_instances.add(instance);        // ← Marked as set up HERE
    await this.client.query(/* CREATE FUNCTION ... */);  // ← But these can fail
    await this.client.query(/* LISTEN ... */);
    await this.client.query(/* CREATE TRIGGER ... */);
  }
};

If any query fails (e.g., permission error, connection timeout), the instance is permanently marked as set up in open_instances, but the trigger/listener was never created. All subsequent calls to setupPgNotify for the same instance skip setup entirely (the has() check passes), leaving a dead channel — no change notifications are ever received for that table.

Fix: Move open_instances.add(instance) to after all three queries succeed, and wrap the block in try/catch.

2. No error handler on pg.Client

The constructor creates a pg.Client and calls .connect(), but never registers an error event handler:

constructor(db_config) {
  this.client = new pg.Client(db_config);
  // No this.client.on("error", ...) handler
  this.client.connect().then(() => { /* ... */ });
}

When the PostgreSQL connection drops (network blip, server restart, idle timeout), the error event fires. Without a handler:

  • Node.js may throw an unhandled error (crashes the process)
  • Stale entries remain in open_instances — the instance thinks channels are active, but the underlying connection is dead
  • On reconnect, setupPgNotify skips re-creation because open_instances still contains the old entries

Fix: Add this.client.on("error", ...) that clears open_instances, allowing channels to be re-created on the next call.

3. No accessors for client or open_instances

Both client and open_instances are private with no getters. This makes it impossible to:

  • Monitor connection health (client._connected, client.connection)
  • Check which instances are actively set up
  • Build diagnostics/health-check endpoints
  • Detect the stale-channel scenario described above

Fix: Expose read-only accessors:

getClient(): pg.Client {
  return this.client;
}

getOpenInstances(): Set<string> {
  return new Set(this.open_instances); // defensive copy
}

Impact

In production, a single PostgreSQL connection blip silently breaks all reactive change detection. The system appears to be running normally (no errors, no crashes), but no table changes propagate through the reactive graph. The only recovery is a full process restart. Without accessors, there is no way to detect this state programmatically.

Current workaround

We maintain a patch file for @skip-adapter/postgres@0.0.19 that:

  1. Wraps setupPgNotify queries in try/catch, moves open_instances.add() to after success
  2. Adds client.on("error") handler that clears open_instances
  3. Exposes getClient() and getOpenInstances() accessors

Expected behavior

  1. open_instances.add() should only be called after all setup queries succeed
  2. pg.Client should have an error event handler that invalidates stale channel state
  3. Read-only accessors should be available for monitoring and diagnostics

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions