Skip to content

Optionally log errors#351

Open
codermapuche wants to merge 2 commits into
neumino:masterfrom
codermapuche:master
Open

Optionally log errors#351
codermapuche wants to merge 2 commits into
neumino:masterfrom
codermapuche:master

Conversation

@codermapuche
Copy link
Copy Markdown

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

@neumino
Copy link
Copy Markdown
Owner

neumino commented Sep 3, 2017

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

@aleclarson
Copy link
Copy Markdown
Contributor

@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
Copy link
Copy Markdown
Author

@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
Copy link
Copy Markdown
Owner

neumino commented Sep 9, 2017

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 #325 if you want

@marshall007
Copy link
Copy Markdown
Contributor

I had started to spec out an API for this in #334 (comment). That's probably a good starting point if anyone wants to submit a formal proposal.

@aleclarson aleclarson mentioned this pull request Sep 13, 2017
@aleclarson
Copy link
Copy Markdown
Contributor

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

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.

4 participants