Skip to content

Commit 3e070cb

Browse files
committed
fixed some major issues with how errors are handled
The old code here ignored initialisation errors. The new code fixes this with some slightly convoluted code. The reason it's a bit complicated is the nature of some errors being immediate and some being sent on a channel. Which must then be wrapped into a second layer of concurrency and multiplexed back into the central for-select loop.
1 parent 46b2a52 commit 3e070cb

File tree

2 files changed

+32
-10
lines changed

2 files changed

+32
-10
lines changed

service/reconfigure.go

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package service
22

33
import (
4+
"context"
45
"reflect"
56

67
"github.com/Southclaws/gitwatch"
@@ -76,22 +77,32 @@ func (app *App) watchConfig() (err error) {
7677
if err != nil {
7778
return errors.Wrap(err, "failed to watch config target")
7879
}
79-
go app.configWatcher.Run() //nolint:errcheck - no worthwhile errors returned
80+
81+
go func() {
82+
e := app.configWatcher.Run()
83+
if e != nil && !errors.Is(e, context.Canceled) {
84+
app.errors <- e
85+
}
86+
}()
8087
zap.L().Debug("created new config watcher, awaiting setup")
8188

82-
<-app.configWatcher.InitialDone
83-
zap.L().Debug("config initial setup done")
89+
select {
90+
case <-app.configWatcher.InitialDone:
91+
zap.L().Debug("config initial setup done")
92+
93+
case err = <-app.errors:
94+
}
8495

8596
return
8697
}
8798

8899
// watchTargets creates or restarts the watcher that reacts to changes to target
89100
// repositories that contain actual apps and services
90101
func (app *App) watchTargets() (err error) {
91-
var targetURLs []string
92-
for _, t := range app.targets {
102+
targetURLs := make([]string, len(app.targets))
103+
for i, t := range app.targets {
93104
zap.L().Debug("assigned target", zap.String("url", t.RepoURL))
94-
targetURLs = append(targetURLs, t.RepoURL)
105+
targetURLs[i] = t.RepoURL
95106
}
96107

97108
if app.targetsWatcher != nil {
@@ -107,11 +118,21 @@ func (app *App) watchTargets() (err error) {
107118
if err != nil {
108119
return errors.Wrap(err, "failed to watch targets")
109120
}
110-
go app.targetsWatcher.Run() //nolint:errcheck - no worthwhile errors returned
121+
122+
go func() {
123+
e := app.targetsWatcher.Run()
124+
if e != nil && !errors.Is(e, context.Canceled) {
125+
app.errors <- e
126+
}
127+
}()
111128
zap.L().Debug("created targets watcher, awaiting setup")
112129

113-
<-app.targetsWatcher.InitialDone
114-
zap.L().Debug("targets initial setup done")
130+
select {
131+
case <-app.targetsWatcher.InitialDone:
132+
zap.L().Debug("targets initial setup done")
133+
134+
case err = <-app.errors:
135+
}
115136

116137
return
117138
}

service/service.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ type App struct {
2929
state config.State
3030
ctx context.Context
3131
cancel context.CancelFunc
32+
errors chan error
3233
}
3334

3435
type Config struct {
@@ -100,7 +101,7 @@ func (app *App) Start() (final error) {
100101
zap.Error(e))
101102
}
102103

103-
case e := <-errorMultiplex(app.configWatcher.Errors, app.targetsWatcher.Errors):
104+
case e := <-errorMultiplex(app.configWatcher.Errors, app.targetsWatcher.Errors, app.errors):
104105
zap.L().Error("git error",
105106
zap.Error(e))
106107

0 commit comments

Comments
 (0)