Skip to content

fix(graphql-server): attach the notify client's error handler before the LISTEN - #1710

Merged
pyramation merged 1 commit into
mainfrom
fix/notify-listener-error-handler
Aug 11, 2026
Merged

fix(graphql-server): attach the notify client's error handler before the LISTEN#1710
pyramation merged 1 commit into
mainfrom
fix/notify-listener-error-handler

Conversation

@pyramation

Copy link
Copy Markdown
Contributor

Summary

Fixes constructive-io/constructive-planning#1605: graphql-public exits on Error: Connection terminated unexpectedly emitted on a Client with no error listener, taking every GraphQL host down until Kubernetes restarts the pod.

listenForChanges issued LISTEN "schema:update" before attaching the error handler, so between pool checkout and that client.on('error', …) the client was unguarded — and Node rethrows an unlistened error event. The window is re-entered on every reconnect (the error handler itself calls addEventListener()), so a flapping Postgres gets repeated chances to kill the process.

 this.listenClient = client;
 this.listenRelease = release;
+client.on('error', (e) => this.dropListener(client, 'Error with database notify listener', e));
 client.on('notification', …);
-client.query('LISTEN "schema:update"');
-client.on('error', …);
+client.query('LISTEN "schema:update"').catch((e) => this.dropListener(client, 'Failed to LISTEN for schema:update', e));

Teardown moved into dropListener(client, message, err), which no-ops unless this.listenClient === client — so a second failure (or one racing shutdown/removeEventListener) cannot double-release or spawn a second reconnect chain. Reconnect/log/release behavior is otherwise unchanged.

Also addresses the second half of the issue: the LISTEN was fire-and-forget, so a failed LISTEN left the server believing it was watching schema:update while schema changes silently stopped propagating. It now reconnects through the same path.

Test plan

New graphql/server-test/__tests__/notify-listener.test.ts (no database needed — fake PoolClient) asserts: an error listener is present at the moment query() is called, a dropped connection releases + reconnects exactly once, repeated failures tear down once, and a rejected LISTEN reconnects.

Link to Devin session: https://app.devin.ai/sessions/acb0555695da4f628f32371a9a93dc02
Requested by: @pyramation

…the LISTEN

A dropped connection between checkout and line 279 emitted 'error' on a Client with no listener, so Node rethrew and the whole GraphQL server exited. Also observes the LISTEN itself so a failed LISTEN reconnects instead of leaving the server silently deaf to schema:update.
@pyramation pyramation self-assigned this Aug 11, 2026
@devin-ai-integration

Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@pyramation
pyramation merged commit 3491796 into main Aug 11, 2026
20 checks passed
@pyramation
pyramation deleted the fix/notify-listener-error-handler branch August 11, 2026 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant