feat: log stderr from daemon #270
Conversation
achingbrain
commented
Jun 19, 2018
achingbrain
commented
- Renames handlers to stderr and stdout to better reflect what they do.
- Hooks up stderr handler to data event of daemon's stderr to log data emitted.
- Uses the callback to handle process errors instead of an extra error handler.
- Specify stderr and stdout handlers in the method options instead of in a separate argument.
- No longer buffers all command output automatically as it can lead to out of memory errors on long running processes, instead the calling code can choose to receive stdout/stderr or not.
- Does not treat any data emitted on stderr as a reason to judge a process as failed when no stderr handler is passed
ghost
assigned 
Renames process handlers to stderr and stdout to better capture what they do. Hooks up stderr handler to data event of daemon's stderr to log data emitted. Uses the callback to handle errors instead of an extra error handler. Specify stderr and stdout handlers in the method options instead of in a separate argument. No longer buffers all command output automatically as it can lead to out of memory errors on long running processes.
achingbrain
force-pushed
the
log-stderr-from-daemon
branch
from
4405330
to
5de469f
Compare
| @@ -81,8 +113,7 @@ describe('exec', () => { | |||
| const args = hang.concat(tok) | |||
|
|
|||
| const p = exec(args[0], args.slice(1), {}, (err) => { | |||
| // `tail -f /dev/null somerandom` errors out | |||
There was a problem hiding this comment.
I'm not sure this is correct - on Mac OS at least it complains that somerandom doesn't exist but then continues running tailing /dev/null and doesn't error out.
There was a problem hiding this comment.
@achingbrain I didn't understand this part.
As far as I understood, on my Mac OS it seems ok, as well as in the CI for macos.
There was a problem hiding this comment.
The comment says the command 'errors out', which I took to mean it exits with a non-zero error code. When I run it, it prints out a message that somerandom doesn't exist but continues running.
There was a problem hiding this comment.
So, tail -f outputs appended data as a file grows. In this case, the file does not exist, but the process still hanging.
I tried to add some data to the file, which I tried to tail and nothing happened as the file does not exist when the command was executed.
This way, I think we can remove the comment as you did!
|
This also fixes #234 |
Sets up Hapi's debug logging when DEBUG env var is present. This will enable developers to see what is happening inside the daemon when ipfs/js-ipfsd-ctl#270 is merged instead of just getting error messages like 'Please retrace your steps'. It lets you do this sort of thing: ``` $ DEBUG=ipfsd-ctl:daemon:* npm test ``` ...and see Hapi's request/server logs in the output.
There was a problem hiding this comment.
Great job @achingbrain !
I really wanted this change to be made here as well.
|
Is this ok to merge? There's a slight drop in coverage but the tests I've added are skipped during the coverage run so I'm not really sure what I can do about that. |
Sets up Hapi's debug logging when DEBUG env var is present. This will enable developers to see what is happening inside the daemon when ipfs/js-ipfsd-ctl#270 is merged instead of just getting error messages like 'Please retrace your steps'. It lets you do this sort of thing: ``` $ DEBUG=ipfsd-ctl:daemon:* npm test ``` ...and see Hapi's request/server logs in the output.
