chore: log failure to run CLI command#12863
Conversation
|
If I run a random CLI command and it fails, will I now see the error twice? Once as a log line, and once as a message? |
Depends on what the logging configuration is. For example, log could be going to a file. But certainly possible for one to see it twice. I am happy to acomodate an alternative way to do this, as long as: Logs capture the exit error of the application (in this case |
|
I'm not sure the best way to solve this. Right now, the user gets the error on the console twice where the error log is pretty verbose. E.g., running
Where it would otherwise output just:
However, I do get the desire to see failures in
I'd prefer the first and that should be as simple as calling (IMO, if you're using systemd, you should generally just dump everything to systemd and we should probably have better support for syslog log levels, but that's a different can of worms) |
Indeed. Fiddling around with |
Prior code only conditionally logged the output when `LOTUS_DEV` env var was present. This doesn't make sense for a number of reasons: * In case of an error starting up `lotus daemon` via systemctl for example the logs would miss the error. One then has to search through system journal to see what happened. * There are no other places that I can find where `LOTUS_DEV` env var is used. The commit that introduced this condition is very old with no clear commit message to shed light into the rationale. For the first reason alone, the changes here remove that condition, and log the message when the logging output is not standard err or out. This way we at least cover the case where lotus is run in a production environment and would avoid partial logging of errors.
masih
force-pushed
the
masih/log-failure-to-start
branch
from
fb6a8e8 to
8242aa6
Compare
|
@Stebalien This is ready for another look whenever you get a chance. Thanks |
Log failure to run CLI command Prior code only conditionally logged the output when `LOTUS_DEV` env var was present. This doesn't make sense for a number of reasons: * In case of an error starting up `lotus daemon` via systemctl for example the logs would miss the error. One then has to search through system journal to see what happened. * There are no other places that I can find where `LOTUS_DEV` env var is used. The commit that introduced this condition is very old with no clear commit message to shed light into the rationale. For the first reason alone, the changes here remove that condition, and log the message when the logging output is not standard err or out. This way we at least cover the case where lotus is run in a production environment and would avoid partial logging of errors. (cherry picked from commit bb70977)
Log failure to run CLI command Prior code only conditionally logged the output when `LOTUS_DEV` env var was present. This doesn't make sense for a number of reasons: * In case of an error starting up `lotus daemon` via systemctl for example the logs would miss the error. One then has to search through system journal to see what happened. * There are no other places that I can find where `LOTUS_DEV` env var is used. The commit that introduced this condition is very old with no clear commit message to shed light into the rationale. For the first reason alone, the changes here remove that condition, and log the message when the logging output is not standard err or out. This way we at least cover the case where lotus is run in a production environment and would avoid partial logging of errors.
2 participants
Related Issues
Proposed Changes
Prior code only conditionally logged the output when
LOTUS_DEVenv var was present. This doesn't make sense for a number of reasons:In case of an error starting up
lotus daemonvia systemctl for example the logs would miss the error. One then has to search through system journal to see what happened.There are no other places that I can find where
LOTUS_DEVenv var is used. The commit that introduced this condition is very old with no clear commit message to shed light into the rationale.For the first reason alone, the changes here remove that condition, and log the message when the logging output is not standard err or out. This way we at least cover the case where lotus is run in a production environment and would avoid partial logging of errors.
Additional Info
Checklist
Before you mark the PR ready for review, please make sure that: