feat(daemon): add Err() to retrieve the tomb death reason

[thp-canonical]

Daemon has a .Dying() method that mirrors the Tomb.Dying() API, but not .Err() for the Tomb.Err() API. Add a .Err() pass-through function and print out the server exit reason for easier debugging should one of the daemon tomb goroutines die.

[thp-canonical]

Comment taken from d.tomb.Err()'s documentation.

[benhoyt]

Looks reasonable to me. However, I'm wondering how this can happen. Currently it looks like the daemon mostly calls d.tomb.Kill(nil) with a nil death reason. Do you have a repro?

[thp-canonical]

Looks reasonable to me. However, I'm wondering how this can happen. Currently it looks like the daemon mostly calls d.tomb.Kill(nil) with a nil death reason. Do you have a repro?

(Edit:) There's one kill occurrence here, but granted at that point we already decided to shut down:

pebble/internals/daemon/daemon.go

Line 567 in 02e5d2d

d.tomb.Kill(d.serve.Shutdown(ctx))

I think it also happens when one of the tomb goroutines exits with an error, right? From the Tomb.Go() docs:

If f returns a non-nil error, t.Kill is called with that error as the death reason parameter.

So any errors returned from here would (I think) also be reported, not just .Kill() calls:

pebble/internals/daemon/daemon.go

Lines 469 to 474 in 02e5d2d

	d.tomb.Go(func() error {
	if err := d.serve.Serve(d.generalListener); err != http.ErrServerClosed && d.tomb.Err() == tomb.ErrStillAlive {
	return err
	}
	return nil
	})

Same for this related place:

pebble/internals/daemon/daemon.go

Lines 479 to 485 in 02e5d2d

	d.tomb.Go(func() error {
	err := d.serve.Serve(d.httpListener)
	if err != http.ErrServerClosed && d.tomb.Err() == tomb.ErrStillAlive {
	return err
	}
	return nil
	})

With the documentation for Tomb.Kill() saying:

Althoguh (sic) Kill may be called multiple times, only the first non-nil error is recorded as the death reason.

So the && d.tomb.Err() == tomb.ErrStillAlive checks above are probably not needed (if their intention is only to avoid overwriting an existing error).

I haven't followed if there's a way to bubble up errors from some handlers all the way to the caller of the .Serve() function, but it seems like it at least any error return value from any of the .Serve() calls that's != http.ErrServerClosed would be good to have reported instead of silently ignored.