Skip to content

Conversation

@awarus
Copy link

@awarus awarus commented Oct 21, 2025

Add a new getConnectionParameters() method to NeonClient that exposes PostgreSQL connection parameters sent by the server during the connection lifecycle.

Added test coverage in tests/cli/ws.test.ts

@awarus awarus requested review from jawj and pffigueiredo October 21, 2025 14:49
Add a new getConnectionParameters() method to NeonClient
that exposes PostgreSQL connection parameters sent
by the server during the connection lifecycle.

Added test coverage in tests/cli/ws.test.ts
@awarus awarus force-pushed the add-get-parameters branch from 1f20c9a to 09b61ee Compare October 21, 2025 15:07
@jawj
Copy link
Collaborator

jawj commented Oct 22, 2025

On the implementation side, I wonder if this should be a getConnectionParameters() method, or simply a connectionParameters property? (And if additional work were ever needed at call time, which currently it is not, that property could always be changed to a getter).

But to step back a minute, I'm curious about the specific reason for adding this functionality to the driver itself. It's more or less a one-liner for users to add their own parameterStatus listener, as this implementation shows. So is this not just one more thing to maintain (and a divergence from the underlying node-postgres driver)?

@awarus
Copy link
Author

awarus commented Oct 22, 2025

I am fine with any option that lets us parsing custom ParameterStatus. If it's better to have as field, I am fine with that - I have zero experience in ts :)

My main motivation behind that is that we use ParameterStatus messages for debugging purposes of the serverless driver connections. It helps us measuring latencies and provide additional debug data.
I think if this change become a burden with high divergence from node-postgres we can always decide to drop it. Reason to have it for now - easy to use vanilla serverless driver without any additional tweaks and because it's harmless

@jawj
Copy link
Collaborator

jawj commented Oct 23, 2025

I feel like I'm being difficult here — sorry! But I think the change is only mostly harmless. It means every Client created by everyone everywhere will suck up a few hundred extra bytes to store the connection parameters, even though 99%+ of the time these will never be used.

Can we not just drop in a single-file subclass anywhere we need this? Namely:

import { Client, Pool, Connection } from '@neondatabase/serverless';

// reassure TypeScript that `connection` exists on Client instances
export interface ConnParamClient {
  connection: Connection;
}

// a Client subclass that saves parameterStatus key/values
export class ConnParamClient extends Client {
  connectionParameters: Map<string, string> = new Map();
  constructor(...args: any[]) {
    super(...args);
    this.connection.on('parameterStatus', (msg: any) =>
      this.connectionParameters.set(msg.parameterName, msg.parameterValue));
  }
}

// a Pool subclass for ConnParamClients
export class ConnParamPool extends Pool {
  Client = ConnParamClient;
}

@awarus
Copy link
Author

awarus commented Oct 23, 2025

Sorry, didin't get it with the first comment, now I see the issue. Yes, I agree, that would be much better! Will fix this now

@awarus
Copy link
Author

awarus commented Oct 23, 2025

Handle this in a proper way in another PR. Thanks again :)

@awarus awarus closed this Oct 23, 2025
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.

3 participants