Top level error handling (#56)

* add a goroutine stack trace when exiting in debug mode

* added error logging to the reconfigure daemon

* added a couple of additional debug log entries to significant events

* improved the top-level error handling so any point of failure ends the application

Author: Southclaws

main.go

@@ -2,8 +2,10 @@ package main
 
 import (
 	"context"
+	"log"
 	"os"
 	"os/signal"
+	"runtime"
 	"time"
 
 	_ "github.com/joho/godotenv/autoload"
@@ -114,6 +116,19 @@ this repository has new commits, Pico will automatically reconfigure.`,
 		},
 	}
 
+	if os.Getenv("DEBUG") != "" {
+		go func() {
+			sigs := make(chan os.Signal, 1)
+			signal.Notify(sigs, os.Interrupt)
+			buf := make([]byte, 1<<20)
+			for {
+				<-sigs
+				stacklen := runtime.Stack(buf, true)
+				log.Printf("\nPrinting goroutine stack trace because `DEBUG` was set.\n%s\n", buf[:stacklen])
+			}
+		}()
+	}
+
 	err := app.Run(os.Args)
 	if err != nil {
 		zap.L().Fatal("exit", zap.Error(err))

reconfigurer/git.go

@@ -88,6 +88,9 @@ func (p *GitProvider) reconfigure(w watcher.Watcher) (err error) {
 		state.Env["HOSTNAME"] = p.hostname
 	}
 
+	zap.L().Debug("setting state for watcher",
+		zap.Any("new_state", state))
+
 	return w.SetState(state)
 }
 

service/service.go

@@ -10,7 +10,6 @@ import (
 	"github.com/eapache/go-resiliency/retrier"
 	"github.com/pkg/errors"
 	"go.uber.org/zap"
-	"golang.org/x/sync/errgroup"
 	"gopkg.in/src-d/go-git.v4/plumbing/transport"
 	"gopkg.in/src-d/go-git.v4/plumbing/transport/http"
 	"gopkg.in/src-d/go-git.v4/plumbing/transport/ssh"
@@ -110,39 +109,43 @@ func Initialise(c Config) (app *App, err error) {
 
 // Start launches the app and blocks until fatal error
 func (app *App) Start(ctx context.Context) error {
-	g, ctx := errgroup.WithContext(ctx)
-
-	zap.L().Debug("starting service daemon")
-
-	// TODO: Replace this errgroup with a more resilient solution.
-	// Not all of these tasks fail in the same way. Some don't fail at all.
-	// This needs to be rewritten to be more considerate of different failure
-	// states and potentially retry in some circumstances. Pico should be the
-	// kind of service that barely goes down, only when absolutely necessary.
+	errs := make(chan error)
 
 	ce := executor.NewCommandExecutor(app.secrets, app.config.PassEnvironment, app.config.VaultConfig, "GLOBAL_")
-	g.Go(func() error {
+	go func() {
 		ce.Subscribe(app.bus)
-		return nil
-	})
+	}()
 
-	// TODO: gw can fail when setting up the gitwatch instance, it should retry.
 	gw := app.watcher.(*watcher.GitWatcher)
-	g.Go(gw.Start)
+	go func() {
+		errs <- errors.Wrap(gw.Start(), "git watcher terminated fatally")
+	}()
 
-	// TODO: reconfigurer can also fail when setting up gitwatch.
-	g.Go(func() error {
-		return app.reconfigurer.Configure(app.watcher)
-	})
+	go func() {
+		errs <- errors.Wrap(app.reconfigurer.Configure(app.watcher), "git watcher terminated fatally")
+	}()
 
 	if s, ok := app.secrets.(*vault.VaultSecrets); ok {
-		g.Go(func() error {
-			return retrier.New(retrier.ConstantBackoff(3, 100*time.Millisecond), nil).
-				RunCtx(ctx, s.Renew)
-		})
+		go func() {
+			errs <- errors.Wrap(retrier.New(retrier.ConstantBackoff(3, 100*time.Millisecond), nil).RunCtx(ctx, s.Renew), "git watcher terminated fatally")
+		}()
 	}
 
-	return g.Wait()
+	handle := func() error {
+		select {
+		case err := <-errs:
+			return err
+		case <-ctx.Done():
+			return context.Canceled
+		}
+	}
+
+	zap.L().Debug("starting service main loop")
+	for {
+		if err := handle(); err != nil {
+			return err
+		}
+	}
 }
 
 func getAuthMethod(c Config, secretConfig map[string]string) (transport.AuthMethod, error) {

watcher/git.go

@@ -62,6 +62,8 @@ func NewGitWatcher(
 
 // Start runs the watcher loop and blocks until a fatal error occurs
 func (w *GitWatcher) Start() error {
+	zap.L().Debug("git watcher initialising, waiting for first state to be set")
+
 	// wait for the first config event to set the initial state
 	<-w.initialise