Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
riteshsangwan opened this issue Apr 10, 2017 · 4 comments
Closed

Comments

@riteshsangwan
Copy link

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());
@riteshsangwan riteshsangwan changed the title First log event will not be fired on the newly created instance of r First log event will not be fired on the newly created instance of r Apr 10, 2017
@neumino
Copy link
Owner

neumino commented Apr 11, 2017

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

@marshall007
Copy link
Contributor

@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 #325 whilst still leaving any additional logging dependencies up to the user.

@riteshsangwan
Copy link
Author

@marshall007 +1

@neumino
Copy link
Owner

neumino commented May 28, 2017

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.

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 a pull request may close this issue.

3 participants