panates / postgrejs

Professional PostgreSQL client for NodeJS
https://postgrejs.panates.com
MIT License
50 stars 12 forks source link

Runtime logging? #22

Closed unilynx closed 1 year ago

unilynx commented 1 year ago

Is your feature request related to a problem? Please describe. I want lowlevel debug information from the driver (both queries sent by my code and implicit commands such as the SAVEPOINT management) and be able to control when I see them and eg prefix log entries with additional information

Describe the solution you'd like There were already a few debug hooks, eg:

const debug = (() => void 0) as any;// _debug("pgc:connection");

but even if reenabled they would go to the debug package. I'd like to inject this handler myself, and would also like some more information where the debug command comes from so I can selectively enable/disable Connection's and IntlConnection's statements

There are many ways to do that and I have no problems coding it, but it would be nice if I can find an approach that upstream would like to support/keep too.

One approach could be

export const onPgcDebug = null; //there should be an API/hook to explicitly set this

pgcDebug(source: "IntlConnection" | "Connection" | "Pool", format: string, ...args: unknown[]): void {
  if(onPgcDebug) 
    onPgcDebug(source, format, ...args);
}

and to update the various ts files to do:

- debug("[%s] ref %d", this.processID, this._refCount);
+ pgcDebug("IntlConnection", "[%s] ref %d", this.processID, this._refCount);

or

- const debug = (() => void 0) as any;// _debug("pgc:intlcon");
+ const debug = (format, ...args) => pgcDebug("intlConnection", format, ...args)

It's still not completely what I need as ideally this would be bound to an AsyncContext and/or the debug API would receive the associated connection so I can specifically debug the actions associated with a specific incoming request. We haven't verified yet how asynccontext-safe postgresql-client is so we might still run into issues there in the end.

Describe alternatives you've considered Not really, unless we can get TC39 to consider adding #define to JavaScript.

We depend on fine grained debugging of the SQL actions related to specific requests so any debugging support gets quite intrusive fast. If needed we could fork and maintain the debugging solution ourselves.

erayhanoglu commented 1 year ago

Hi, i ve disabled "debug" library because it vulnerable. Here is the report

Dependency npm:debug:4.3.4 is vulnerable 
Cx8bc4df28-fcf5 7.5  Inefficient Regular Expression Complexity vulnerability pending CVSS allocation  
Results powered by Checkmarx(c)

The "debug" package has not been updated for a year.

About custom debug hooks solution you propose, it can be made.

erayhanoglu commented 1 year ago

I ve just released 2.7.0. which includes "debug" events in Pool and Connection. do get debug events just use

pool.on('debug', info => console.log(info.message))
connection.on('debug', info => console.log(info.message))
unilynx commented 1 year ago

@erayhanoglu thanks, this is a much nicer interface

It would be nice to expose the types of the debugmessages, eg for Connection

interface ConnectionDebugEvent {
  location: string;
  connection: Connection;
  message: string;
  sql?: string;
}

(I played a bit with getting emit("debug") to actually typescript-validate the debug message, but gave up after about 10 minutes because it's a bit tricky with SafeEventEmitter defining a generic emit which caused any debug message to validate anyway)

One drawback is that you also removed the debug statements from intl-connection.ts so you can't see the savepoint etc commands happening behind the scenes (which helped me when debugging #20). Even if the endpoints are still there and require a TS override or patching it's nice to have them during a complex debugging session.

erayhanoglu commented 1 year ago

Hi. I am sorry but i couldn't get what you mean with "typescript-validate", and did not understand with the problem with SafeEventEmitter. It is extended from EventEmitter and only difference is it doesnt throw Error on ("error") event.

Yes i ve removed "debug" statements. Because "debug" package is vulnerable. But emit('debug') can be used. You may make a patch.

unilynx commented 1 year ago

Check, here's the patch: https://github.com/panates/postgresql-client/pull/28

It doesn't define a public API for it, but it's cleaner than patching console.logs straight into pgsocket