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

Optionally log errors #351

Open codermapuche opened 7 years ago

codermapuche commented 7 years ago

Not all log are errors and it is useful to differentiate when something goes wrong when everything is ok

neumino commented 7 years ago

Why isn't that useful? If you add a new server, you want to know whether a pool was created or not

aleclarson commented 7 years ago

@neumino I'm confused, are you saying console.error is necessary to know if the pool was created? Why can't console.log work?

codermapuche commented 7 years ago

@neumino when use rethinkdbdash in a child proccess, a console.error redirect messages to stderr, not to stdout, this is a ploblem, because the stderr can be used to dispatch an alert, log activity, trace execution and eventually restart the child process, i agree that log new pool connections is useful, but this is not an error, is a desired task. For this reason not removes the line of the log, only change the target of this message from stderror to stdout.

neumino commented 7 years ago

Why can't stderr dispatch an alert? Isn't that an issue with your paging system? You can also just use the log event if you don't want everything to go to stderr.

If you want to change the core/default behavior, please submit a proposal to classify all messages in debug/info/warn/error. So far no one has proposed a clean API - feel free to reopen https://github.com/neumino/rethinkdbdash/issues/325 if you want

marshall007 commented 7 years ago

I had started to spec out an API for this in https://github.com/neumino/rethinkdbdash/issues/334#issuecomment-293423825. That's probably a good starting point if anyone wants to submit a formal proposal.

aleclarson commented 7 years ago

Check out #355 as the follow-up to this PR. 😄