Not to close pipe even when Scan() fails

[honteng]

• Have you test the code?
• Have you check that the existing tests are passing?
• The destination branch is develop?

Let's say when you run the following child program:

func main() {
	i := 0
	for {
		fmt.Println(i)
		i++
		time.Sleep(time.Second)
		if i%3 == 0 {
			buf := make([]byte, 1000*1000)
			fmt.Println("%v", buf)
		}
	}
}

Every three seconds, it dumps big []byte array. This is not well written program though, but it may happen if you dump the whole struct.

When immortal captures the whole text coming from the child process, Scan() fails and Err() truns bufio.ErrTooLong. And the current code doesn't check the error and it close the pipe.

The closing pipe triggers SIGPIPE signal to the child and the child process is killed. In this case, the process is killed always after three seconds since it starts.

This patch mitigates not to trigger SIGPIPE signal even if the child process dumps long text log.

[nbari]

hi @honteng many thanks, could you please add a test case to cover this probably in logger_test.go

Thanks in advance.

[nbari]

@honteng I created this small test, what do you think? https://github.com/immortal/immortal/blob/develop/logger_test.go#L113-L183

[coveralls]

Coverage decreased (-0.03%) to 85.513% when pulling 1b43564 on honteng:fix_sigpipe into a81b5c4 on immortal:develop.

[codecov]

Codecov Report

Merging #46 into develop will decrease coverage by 0.03%.
The diff coverage is 85.71%.

@@             Coverage Diff             @@
##           develop      #46      +/-   ##
===========================================
- Coverage    84.16%   84.12%   -0.04%     
===========================================
  Files           16       16              
  Lines          941      945       +4     
===========================================
+ Hits           792      795       +3     
- Misses         111      112       +1     
  Partials        38       38

Impacted Files	Coverage Δ
logger.go	95.52% <85.71%> (-1.31%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a81b5c4...1b43564. Read the comment docs.

[honteng]

Thank you for your test. Yes, it looks good. Only one thing I could feed back is that you wouldn't need to wait 3 times to trigger the long log. I guess dumping the long log right away after the child process launch should be ok.