health: reduce severity of some warnings, improve update messages (#12689)

Updates tailscale/tailscale#4136

High severity health warning = a system notification will appear, which can be quite disruptive to the user and cause unnecessary concern in the event of a temporary network issue.

Per design decision (@sonovawolf), the severity of all warnings but "network is down" should be tuned down to medium/low. ImpactsConnectivity should be set, to change the icon to an exclamation mark in some cases, but without a notification bubble.

I also tweaked the messaging for update-available, to reflect how each platform gets updates in different ways.

Signed-off-by: Andrea Gottardo <andrea@gottardo.me>

health/health.go

@@ -213,8 +213,10 @@ type Warnable struct {
 	// Deprecated: this is only used in one case, and will be removed in a future PR
 	MapDebugFlag string
 
-	// If true, this warnable is related to configuration of networking stack
+	// ImpactsConnectivity is whether this Warnable in an unhealthy state will impact the user's
-	// on the machine that impacts connectivity.
+	// ability to connect to the Internet or other nodes on the tailnet. On platforms where
+	// the client GUI supports a tray icon, the client will display an exclamation mark
+	// on the tray icon when ImpactsConnectivity is set to true and the Warnable is unhealthy.
 	ImpactsConnectivity bool
 }
 
@@ -252,9 +254,16 @@ func (t *Tracker) nil() bool {
 type Severity string
 
 const (
-	SeverityHigh   Severity = "high"
+	// SeverityHigh is the highest severity level, used for critical errors that need immediate attention.
+	// On platforms where the client GUI can deliver notifications, a SeverityHigh Warnable will trigger
+	// a modal notification.
+	SeverityHigh Severity = "high"
+	// SeverityMedium is used for errors that are important but not critical. This won't trigger a modal
+	// notification, however it will be displayed in a more visible way than a SeverityLow Warnable.
 	SeverityMedium Severity = "medium"
-	SeverityLow    Severity = "low"
+	// SeverityLow is used for less important notices that don't need immediate attention. The user will
+	// have to go to a Settings window, or another "hidden" GUI location to see these messages.
+	SeverityLow Severity = "low"
 )
 
 // Args is a map of Args to string values that can be used to provide parameters regarding

health/warnings.go

@@ -5,7 +5,10 @@ package health
 
 import (
 	"fmt"
+	"runtime"
 	"time"
+
+	"tailscale.com/version"
 )
 
 /**
@@ -18,7 +21,11 @@ var updateAvailableWarnable = Register(&Warnable{
 	Title:    "Update available",
 	Severity: SeverityLow,
 	Text: func(args Args) string {
-		return fmt.Sprintf("An update from version %s to %s is available. Run `tailscale update` or `tailscale set --auto-update` to update.", args[ArgCurrentVersion], args[ArgAvailableVersion])
+		if version.IsMacAppStore() || version.IsAppleTV() || version.IsMacSys() || version.IsWindowsGUI() || runtime.GOOS == "android" {
+			return fmt.Sprintf("An update from version %s to %s is available.", args[ArgCurrentVersion], args[ArgAvailableVersion])
+		} else {
+			return fmt.Sprintf("An update from version %s to %s is available. Run `tailscale update` or `tailscale set --auto-update` to update now.", args[ArgCurrentVersion], args[ArgAvailableVersion])
+		}
 	},
 })
 
@@ -26,9 +33,13 @@ var updateAvailableWarnable = Register(&Warnable{
 var securityUpdateAvailableWarnable = Register(&Warnable{
 	Code:     "security-update-available",
 	Title:    "Security update available",
-	Severity: SeverityHigh,
+	Severity: SeverityMedium,
 	Text: func(args Args) string {
-		return fmt.Sprintf("An urgent security update from version %s to %s is available. Run `tailscale update` or `tailscale set --auto-update` to update now.", args[ArgCurrentVersion], args[ArgAvailableVersion])
+		if version.IsMacAppStore() || version.IsAppleTV() || version.IsMacSys() || version.IsWindowsGUI() || runtime.GOOS == "android" {
+			return fmt.Sprintf("A security update from version %s to %s is available.", args[ArgCurrentVersion], args[ArgAvailableVersion])
+		} else {
+			return fmt.Sprintf("A security update from version %s to %s is available. Run `tailscale update` or `tailscale set --auto-update` to update now.", args[ArgCurrentVersion], args[ArgAvailableVersion])
+		}
 	},
 })
 
@@ -38,15 +49,15 @@ var unstableWarnable = Register(&Warnable{
 	Code:     "is-using-unstable-version",
 	Title:    "Using an unstable version",
 	Severity: SeverityLow,
-	Text:     StaticMessage("This is an unstable version of Tailscale meant for testing and development purposes: please report any bugs to Tailscale."),
+	Text:     StaticMessage("This is an unstable version of Tailscale meant for testing and development purposes. Please report any issues to Tailscale."),
 })
 
 // NetworkStatusWarnable is a Warnable that warns the user that the network is down.
 var NetworkStatusWarnable = Register(&Warnable{
 	Code:                "network-status",
 	Title:               "Network down",
-	Severity:            SeverityHigh,
+	Severity:            SeverityMedium,
-	Text:                StaticMessage("Tailscale cannot connect because the network is down. (No network interface is up.)"),
+	Text:                StaticMessage("Tailscale cannot connect because the network is down. Check your Internet connection."),
 	ImpactsConnectivity: true,
 })
 
@@ -94,18 +105,19 @@ var notInMapPollWarnable = Register(&Warnable{
 
 // noDERPHomeWarnable is a Warnable that warns the user that Tailscale doesn't have a home DERP.
 var noDERPHomeWarnable = Register(&Warnable{
-	Code:      "no-derp-home",
+	Code:                "no-derp-home",
-	Title:     "No home relay server",
+	Title:               "No home relay server",
-	Severity:  SeverityHigh,
+	Severity:            SeverityMedium,
-	DependsOn: []*Warnable{NetworkStatusWarnable},
+	DependsOn:           []*Warnable{NetworkStatusWarnable},
-	Text:      StaticMessage("Tailscale could not connect to any relay server. Check your Internet connection."),
+	Text:                StaticMessage("Tailscale could not connect to any relay server. Check your Internet connection."),
+	ImpactsConnectivity: true,
 })
 
 // noDERPConnectionWarnable is a Warnable that warns the user that Tailscale couldn't connect to a specific DERP server.
 var noDERPConnectionWarnable = Register(&Warnable{
 	Code:      "no-derp-connection",
 	Title:     "Relay server unavailable",
-	Severity:  SeverityHigh,
+	Severity:  SeverityMedium,
 	DependsOn: []*Warnable{NetworkStatusWarnable},
 	Text: func(args Args) string {
 		if n := args[ArgDERPRegionName]; n != "" {
@@ -114,6 +126,7 @@ var noDERPConnectionWarnable = Register(&Warnable{
 			return fmt.Sprintf("Tailscale could not connect to the relay server with ID '%s'. Your Internet connection might be down, or the server might be temporarily unavailable.", args[ArgDERPRegionID])
 		}
 	},
+	ImpactsConnectivity: true,
 })
 
 // derpTimeoutWarnable is a Warnable that warns the user that Tailscale hasn't heard from the home DERP region for a while.
@@ -135,7 +148,7 @@ var derpTimeoutWarnable = Register(&Warnable{
 var derpRegionErrorWarnable = Register(&Warnable{
 	Code:      "derp-region-error",
 	Title:     "Relay server error",
-	Severity:  SeverityMedium,
+	Severity:  SeverityLow,
 	DependsOn: []*Warnable{NetworkStatusWarnable},
 	Text: func(args Args) string {
 		return fmt.Sprintf("The relay server #%v is reporting an issue: %v", args[ArgDERPRegionID], args[ArgError])
@@ -146,7 +159,7 @@ var derpRegionErrorWarnable = Register(&Warnable{
 var noUDP4BindWarnable = Register(&Warnable{
 	Code:                "no-udp4-bind",
 	Title:               "Incoming connections may fail",
-	Severity:            SeverityHigh,
+	Severity:            SeverityMedium,
 	DependsOn:           []*Warnable{NetworkStatusWarnable},
 	Text:                StaticMessage("Tailscale couldn't listen for incoming UDP connections."),
 	ImpactsConnectivity: true,