Stop startup when a setting does not parse (closes #219)

config.New read the number and true/false settings with viper's
GetInt, GetFloat64 and GetBool. These return zero or false for a value
that does not parse, so a typo silently changed how the server ran.

config.New now parses these settings with strconv. It also checks that
MESSAGE_MAX_AGE, QUEUE_MAX_AGE and SESSION_IDLE_TIMEOUT are valid Go
durations. A value that does not parse makes config.New return an
error naming the setting and its value, and startup stops. Unset
settings keep their defaults.

Model: opus-5-5

internal/cli/app.go

@@ -627,10 +627,6 @@ func (a *App) cmdWindow(args string) {
 				nick, buf.Name, "connected",
 			)
 		} else {
-			a.mu.Lock()
-			a.target = ""
-			a.mu.Unlock()
-
 			a.ui.SetStatus(nick, "", "connected")
 		}
 	}

internal/cli/app_internal_test.go

@@ -219,63 +219,6 @@ func TestJoinResponse(t *testing.T) {
 	}
 }
 
-// TestAltDigit checks that Alt and a digit shows that window and
-// makes its channel or user the target, as /window does. Window
-// 0, the status window, leaves no target.
-func TestAltDigit(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name  string
-		digit rune
-		// The buffer shown and the target after the key.
-		wantBuffer string
-		wantTarget string
-	}{
-		{
-			name:       "channel window",
-			digit:      '1',
-			wantBuffer: testChannel,
-			wantTarget: testChannel,
-		},
-		{
-			name:       "status window",
-			digit:      '0',
-			wantBuffer: "(status)",
-			wantTarget: "",
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			t.Parallel()
-
-			app := &App{ui: NewUI(), nick: testNick, target: testOtherNick}
-			app.ui.OnInput(app.handleInput)
-			runUI(t, app.ui)
-
-			// Window 1 is the channel. Window 2, the one shown, is
-			// the other user's.
-			app.ui.SwitchToBuffer(testChannel)
-			app.ui.SwitchToBuffer(testOtherNick)
-
-			key := tcell.NewEventKey(tcell.KeyRune, test.digit, tcell.ModAlt)
-			app.ui.app.GetInputCapture()(key)
-
-			var shown string
-
-			app.ui.app.QueueUpdate(func() {
-				shown = app.ui.buffers[app.ui.currentBuffer].Name
-			})
-
-			if shown != test.wantBuffer || app.target != test.wantTarget {
-				t.Fatalf("got buffer %s and target %q, want %s and %q",
-					shown, app.target, test.wantBuffer, test.wantTarget)
-			}
-		})
-	}
-}
-
 // TestOwnEventFromOtherClient checks that a message, notice,
 // join or part that the user's session makes from another
 // client shows as one line in the buffer it belongs to.

internal/cli/ui.go

@@ -240,13 +240,10 @@ func (ui *UI) initKeyCapture() {
 				return event
 			}
 
-			// Alt and a digit runs /window with that digit.
-			// SwitchBuffer alone would leave the target as it was.
 			r := event.Rune()
 			if r >= '0' && r <= '9' {
-				if ui.onInput != nil {
-					ui.onInput("/window " + string(r))
-				}
+				idx := int(r - '0')
+				ui.SwitchBuffer(idx)
 
 				return nil
 			}

internal/config/config.go

@@ -5,7 +5,6 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
-	"math"
 	"os"
 	"strconv"
 	"time"
@@ -32,8 +31,6 @@ Type /help for available commands.`
 const DefaultDBURL = "file:///var/lib/neoirc/state.db" +
 	"?_pragma=journal_mode(WAL)&_pragma=busy_timeout(5000)"
 
-var errOutOfRange = errors.New("setting out of range")
-
 // Params defines the dependencies for creating a Config.
 type Params struct {
 	fx.In
@@ -70,7 +67,7 @@ type Config struct {
 
 // New creates a new Config by reading from files and environment variables.
 // It returns an error when a number, true/false or duration setting does
-// not parse or is out of range.
+// not parse.
 func New(
 	_ fx.Lifecycle, params Params,
 ) (*Config, error) {
@@ -152,16 +149,14 @@ func New(
 }
 
 // parseSettings reads the number and true/false settings into cfg and
-// checks the duration settings. Each number and duration must be above
-// zero. PORT and NEOIRC_HASHCASH_BITS may also be zero: port 0 lets the
-// system pick a port, and 0 bits turns proof of work off. The error
-// names each setting that does not parse or is out of range.
+// checks that each duration setting is a valid Go duration. Its error
+// names each setting whose value does not parse.
 func parseSettings(cfg *Config) error {
 	return errors.Join(
-		parseInt("PORT", 0, &cfg.Port),
-		parseInt("MAX_MESSAGE_SIZE", 1, &cfg.MaxMessageSize),
-		parseInt("NEOIRC_HASHCASH_BITS", 0, &cfg.HashcashBits),
-		parseInt("LOGIN_RATE_BURST", 1, &cfg.LoginRateBurst),
+		parseInt("PORT", &cfg.Port),
+		parseInt("MAX_MESSAGE_SIZE", &cfg.MaxMessageSize),
+		parseInt("NEOIRC_HASHCASH_BITS", &cfg.HashcashBits),
+		parseInt("LOGIN_RATE_BURST", &cfg.LoginRateBurst),
 		parseFloat("LOGIN_RATE_LIMIT", &cfg.LoginRateLimit),
 		parseBool("DEBUG", &cfg.Debug),
 		parseBool("MAINTENANCE_MODE", &cfg.MaintenanceMode),
@@ -171,43 +166,25 @@ func parseSettings(cfg *Config) error {
 	)
 }
 
-// parseInt sets *value to the setting key read as a whole number. It
-// returns an error when the value does not parse or is below minimum.
-func parseInt(key string, minimum int, value *int) error {
-	raw := viper.GetString(key)
-
-	parsed, err := strconv.Atoi(raw)
+// parseInt sets *value to the setting key read as a whole number.
+func parseInt(key string, value *int) error {
+	parsed, err := strconv.Atoi(viper.GetString(key))
 	if err != nil {
 		return fmt.Errorf("cannot parse %s: %w", key, err)
 	}
 
-	if parsed < minimum {
-		return fmt.Errorf("%w: %s is %s, but must be at least %d",
-			errOutOfRange, key, raw, minimum)
-	}
-
 	*value = parsed
 
 	return nil
 }
 
-// parseFloat sets *value to the setting key read as a floating-point
-// number. It returns an error when the value does not parse or is not a
-// finite number above zero.
+// parseFloat sets *value to the setting key read as a decimal number.
 func parseFloat(key string, value *float64) error {
-	raw := viper.GetString(key)
-
-	parsed, err := strconv.ParseFloat(raw, 64)
+	parsed, err := strconv.ParseFloat(viper.GetString(key), 64)
 	if err != nil {
 		return fmt.Errorf("cannot parse %s: %w", key, err)
 	}
 
-	if math.IsNaN(parsed) || math.IsInf(parsed, 0) || parsed <= 0 {
-		return fmt.Errorf(
-			"%w: %s is %s, but must be a finite number above zero",
-			errOutOfRange, key, raw)
-	}
-
 	*value = parsed
 
 	return nil
@@ -226,17 +203,12 @@ func parseBool(key string, value *bool) error {
 }
 
 // checkDuration returns an error naming the setting key when value is
-// not a Go duration above zero.
+// not a valid Go duration.
 func checkDuration(key, value string) error {
-	duration, err := time.ParseDuration(value)
+	_, err := time.ParseDuration(value)
 	if err != nil {
 		return fmt.Errorf("cannot parse %s: %w", key, err)
 	}
 
-	if duration <= 0 {
-		return fmt.Errorf("%w: %s is %s, but must be above zero",
-			errOutOfRange, key, value)
-	}
-
 	return nil
 }

internal/config/config_test.go

@@ -78,30 +78,28 @@ func TestUnsetSettingsKeepDefaults(t *testing.T) {
 }
 
 func TestValidSettingsAreUsed(t *testing.T) {
-	// Each whole number is the lowest value its setting allows, and 1ns
-	// is the shortest duration allowed.
-	t.Setenv("PORT", "0")
-	t.Setenv("MAX_MESSAGE_SIZE", "1")
+	t.Setenv("PORT", "9000")
+	t.Setenv("MAX_MESSAGE_SIZE", "1024")
 	t.Setenv("NEOIRC_HASHCASH_BITS", "0")
-	t.Setenv("LOGIN_RATE_BURST", "1")
+	t.Setenv("LOGIN_RATE_BURST", "10")
 	t.Setenv("LOGIN_RATE_LIMIT", "0.5")
 	t.Setenv("DEBUG", "true")
 	t.Setenv("MAINTENANCE_MODE", "true")
-	t.Setenv("MESSAGE_MAX_AGE", "1ns")
+	t.Setenv("MESSAGE_MAX_AGE", "24h")
 	t.Setenv("QUEUE_MAX_AGE", "1h30m")
-	t.Setenv("SESSION_IDLE_TIMEOUT", "45m")
+	t.Setenv("SESSION_IDLE_TIMEOUT", "0")
 
 	want := settings{
-		Port:               0,
-		MaxMessageSize:     1,
+		Port:               9000,
+		MaxMessageSize:     1024,
 		HashcashBits:       0,
-		LoginRateBurst:     1,
+		LoginRateBurst:     10,
 		LoginRateLimit:     0.5,
 		Debug:              true,
 		MaintenanceMode:    true,
-		MessageMaxAge:      "1ns",
+		MessageMaxAge:      "24h",
 		QueueMaxAge:        "1h30m",
-		SessionIdleTimeout: "45m",
+		SessionIdleTimeout: "0",
 	}
 
 	got := settingsOf(newConfig(t))
@@ -139,45 +137,39 @@ func settingsOf(cfg *config.Config) settings {
 	}
 }
 
-func TestBadSettingStopsStartup(t *testing.T) {
-	// For each setting, the first value does not parse and the others
-	// parse but are out of range.
+func TestUnparseableSettingStopsStartup(t *testing.T) {
 	tests := []struct {
-		key    string
-		values []string
+		key   string
+		value string
 	}{
-		{key: "PORT", values: []string{"80a", "-1"}},
-		{key: "MAX_MESSAGE_SIZE", values: []string{"4k", "0", "-4096"}},
-		{key: "NEOIRC_HASHCASH_BITS", values: []string{"twenty", "-1"}},
-		{key: "LOGIN_RATE_BURST", values: []string{"five", "0", "-5"}},
-		{key: "LOGIN_RATE_LIMIT", values: []string{
-			"fast", "0", "-1", "NaN", "Inf", "-Inf",
-		}},
-		{key: "DEBUG", values: []string{"yes"}},
-		{key: "MAINTENANCE_MODE", values: []string{"on"}},
-		{key: "MESSAGE_MAX_AGE", values: []string{"30d", "0", "-1h"}},
-		{key: "QUEUE_MAX_AGE", values: []string{"720", "0s", "-24h"}},
-		{key: "SESSION_IDLE_TIMEOUT", values: []string{"a month", "0", "-1m"}},
+		{key: "PORT", value: "80a"},
+		{key: "MAX_MESSAGE_SIZE", value: "4k"},
+		{key: "NEOIRC_HASHCASH_BITS", value: "twenty"},
+		{key: "LOGIN_RATE_BURST", value: "five"},
+		{key: "LOGIN_RATE_LIMIT", value: "fast"},
+		{key: "DEBUG", value: "yes"},
+		{key: "MAINTENANCE_MODE", value: "on"},
+		{key: "MESSAGE_MAX_AGE", value: "30d"},
+		{key: "QUEUE_MAX_AGE", value: "720"},
+		{key: "SESSION_IDLE_TIMEOUT", value: "a month"},
 	}
 
 	for _, test := range tests {
-		for _, value := range test.values {
-			t.Run(test.key+"="+value, func(t *testing.T) {
-				t.Setenv(test.key, value)
+		t.Run(test.key, func(t *testing.T) {
+			t.Setenv(test.key, test.value)
 
-				_, err := loadConfig(t)
-				if err == nil {
-					t.Fatalf("%s=%q: got no error", test.key, value)
-				}
+			_, err := loadConfig(t)
+			if err == nil {
+				t.Fatalf("%s=%q: got no error", test.key, test.value)
+			}
 
-				msg := err.Error()
-				if !strings.Contains(msg, test.key) ||
-					!strings.Contains(msg, value) {
-					t.Errorf("error %q does not name %s and %q",
-						msg, test.key, value)
-				}
-			})
-		}
+			msg := err.Error()
+			if !strings.Contains(msg, test.key) ||
+				!strings.Contains(msg, test.value) {
+				t.Errorf("error %q does not name %s and %q",
+					msg, test.key, test.value)
+			}
+		})
 	}
 }