break up health watchers

Some watchers need to know about every warnable change (E.g. to report to
control), some send the UI the full list of warnable changes on any change.
Let's make a separate type for the latter so we have a handy way to send all
UnhealthyStates to the UI when ControlHealth messages change.

control/controlclient/direct.go

@@ -1614,10 +1614,6 @@ func postPingResult(start time.Time, logf logger.Logf, c *http.Client, pr *tailc
 // ReportHealthChange reports to the control plane a change to this node's
 // health. w must be non-nil. us can be nil to indicate a healthy state for w.
 func (c *Direct) ReportHealthChange(w *health.Warnable, us *health.UnhealthyState) {
-	// TODO: Don't tell control about health messages that came from control?
-	if w == nil {
-		return
-	}
 	if w == health.NetworkStatusWarnable || w == health.IPNStateWarnable || w == health.LoginStateWarnable {
 		// We don't report these. These include things like the network is down
 		// (in which case we can't report anyway) or the user wanted things

health/health.go

@@ -87,9 +87,11 @@ type Tracker struct {
 
 	// sysErr maps subsystems to their current error (or nil if the subsystem is healthy)
 	// Deprecated: using Warnables should be preferred
-	sysErr   map[Subsystem]error
+	sysErr      map[Subsystem]error
-	watchers set.HandleSet[func(*Warnable, *UnhealthyState)] // opt func to run if error state changes
+	watchers    set.HandleSet[func(*Warnable, *UnhealthyState)] // opt func to run if error state changes
-	timer    tstime.TimerController
+	anyWatchers set.HandleSet[func()]                           // opt func to run if any error state changes
+
+	timer tstime.TimerController
 
 	latestVersion   *tailcfg.ClientVersion // or nil
 	checkForUpdates bool
@@ -423,6 +425,25 @@ func (t *Tracker) setUnhealthyLocked(w *Warnable, args Args) {
 			})
 			mak.Set(&t.pendingVisibleTimers, w, tc)
 		}
+
+		for _, cb := range t.anyWatchers {
+			if w.IsVisible(ws, t.now) {
+				go cb()
+				continue
+			}
+			visibleIn := w.TimeToVisible - t.now().Sub(brokenSince)
+			var tc tstime.TimerController = t.clock().AfterFunc(visibleIn, func() {
+				t.mu.Lock()
+				defer t.mu.Unlock()
+				// Check if the Warnable is still unhealthy, as it could have become healthy between the time
+				// the timer was set for and the time it was executed.
+				if t.warnableVal[w] != nil {
+					go cb()
+					delete(t.pendingVisibleTimers, w)
+				}
+			})
+			mak.Set(&t.pendingVisibleTimers, w, tc)
+		}
 	}
 }
 
@@ -453,6 +474,9 @@ func (t *Tracker) setHealthyLocked(w *Warnable) {
 	for _, cb := range t.watchers {
 		go cb(w, nil)
 	}
+	for _, cb := range t.anyWatchers {
+		go cb()
+	}
 }
 
 // AppendWarnableDebugFlags appends to base any health items that are currently in failed
@@ -509,6 +533,28 @@ func (t *Tracker) RegisterWatcher(cb func(w *Warnable, r *UnhealthyState)) (unre
 	}
 }
 
+// this function will register an observer for notifications when the set of
+// current UnhealthyStates (Warnables + control Health messages) changes. Unlike
+// RegisterWatcher it will not fire for each Warnable but only once any time any
+// amount of Warnables (or control health messages) change.
+func (t *Tracker) RegisterAnyWatcher(cb func()) (unregister func()) {
+	if t.nil() {
+		return func() {}
+	}
+	t.initOnce.Do(t.doOnceInit)
+	t.mu.Lock()
+	defer t.mu.Unlock()
+	if t.anyWatchers == nil {
+		t.anyWatchers = set.HandleSet[func()]{}
+	}
+	handle := t.anyWatchers.Add(cb)
+	return func() {
+		t.mu.Lock()
+		defer t.mu.Unlock()
+		delete(t.anyWatchers, handle)
+	}
+}
+
 // SetRouterHealth sets the state of the wgengine/router.Router.
 //
 // Deprecated: Warnables should be preferred over Subsystem errors.
@@ -1152,13 +1198,8 @@ func (t *Tracker) updateBuiltinWarnablesLocked() {
 	}
 
 	if t.controlHealth != nil {
-		// for _, cb := range t.watchers {
+		for _, cb := range t.anyWatchers {
-		// 	go cb(nil, nil)
+			go cb()
-		// }
-		if len(t.controlHealth) > 0 {
-			t.setUnhealthyLocked(controlHealthWarnable, nil)
-		} else {
-			t.setHealthyLocked(controlHealthWarnable)
 		}
 	}
 

ipn/ipnlocal/local.go

@@ -240,6 +240,7 @@ type LocalBackend struct {
 	backendLogID             logid.PublicID
 	unregisterNetMon         func()
 	unregisterHealthWatch    func()
+	unregisterAnyHealthWatch func()
 	unregisterSysPolicyWatch func()
 	portpoll                 *portlist.Poller // may be nil
 	portpollOnce             sync.Once        // guards starting readPoller
@@ -609,6 +610,7 @@ func NewLocalBackend(logf logger.Logf, logID logid.PublicID, sys *tsd.System, lo
 	b.unregisterNetMon = netMon.RegisterChangeCallback(b.linkChange)
 
 	b.unregisterHealthWatch = b.health.RegisterWatcher(b.onHealthChange)
+	b.unregisterAnyHealthWatch = b.health.RegisterAnyWatcher(b.onAnyHealthChange)
 
 	if tunWrap, ok := b.sys.Tun.GetOK(); ok {
 		tunWrap.PeerAPIPort = b.GetPeerAPIPort
@@ -968,15 +970,16 @@ func (b *LocalBackend) linkChange(delta *netmon.ChangeDelta) {
 }
 
 func (b *LocalBackend) onHealthChange(w *health.Warnable, us *health.UnhealthyState) {
-	if w == nil {
-		return
-	}
 	if us == nil {
 		b.logf("health(warnable=%s): ok", w.Code)
 	} else {
 		b.logf("health(warnable=%s): error: %s", w.Code, us.Text)
 	}
 
+	b.onAnyHealthChange()
+}
+
+func (b *LocalBackend) onAnyHealthChange() {
 	// Whenever health changes, send the current health state to the frontend.
 	state := b.health.CurrentState()
 	b.send(ipn.Notify{
@@ -1127,6 +1130,7 @@ func (b *LocalBackend) Shutdown() {
 
 	b.unregisterNetMon()
 	b.unregisterHealthWatch()
+	b.unregisterAnyHealthWatch()
 	b.unregisterSysPolicyWatch()
 	if cc != nil {
 		cc.Shutdown()