neumino / rethinkdbdash

An advanced Node.js driver for RethinkDB with a connection pool, support for streams etc.
MIT License
848 stars 108 forks source link

First log event will not be fired on the newly created instance of r #334

Closed riteshsangwan closed 7 years ago

riteshsangwan commented 7 years ago

I have this code

const R = require('rethinkdbdash');
const driverOptions = _.extend(options.db, { silent: true });
const r = new R(driverOptions);
r.getPoolMaster().on('log', (message) => {
  console.log('driver message');
  console.log(message);
});

The issue is

Whenever a new instance of R is created there is this log from pool.js file.

When this log is invoked there are no listeners yet attached to newly created instance and hence the log event will not be received by the application for this message.

this._log('Creating a pool connected to '+this.getAddress());
neumino commented 7 years ago

Hum yea we probably need to pass the listener as part of the options themselves.

marshall007 commented 7 years ago

@neumino in addition to a simple function(message) callback, I think it would make sense to allow the user to pass a logger instance that conforms to the standard log levels defined by libraries like bunyan and winston. That is, an object with the methods { error, warn, info, verbose, debug }.

  1. when a user passes a logger instance, we use that object directly.
  2. when a user passes a single function callback as their logger instance, we create a mock that defines each of the log methods as the user's callback.
  3. when no logger is specified (the default), we create a mock that defines each of the log methods using the console API (error => error, warn => warn, info|verbose|debug => log).

This would address the concerns brought up in https://github.com/neumino/rethinkdbdash/issues/325 whilst still leaving any additional logging dependencies up to the user.

riteshsangwan commented 7 years ago

@marshall007 +1

neumino commented 7 years ago

Fixed in 2.3.29 - You can pass a function to the log option when initializing the driver.

I looked into delaying the initialization of events, but that seems to be a non trivial change.