refactor: code health audit using golangci-lint as a guide (#713)

Co-authored-by: Claude <[email protected]>

Author: corylanou

cmd/litestream/databases.go

@@ -12,7 +12,7 @@ import (
 type DatabasesCommand struct{}
 
 // Run executes the command.
-func (c *DatabasesCommand) Run(ctx context.Context, args []string) (err error) {
+func (c *DatabasesCommand) Run(_ context.Context, args []string) (err error) {
 	fs := flag.NewFlagSet("litestream-databases", flag.ContinueOnError)
 	configPath, noExpandEnv := registerConfigFlag(fs)
 	fs.Usage = c.Usage

cmd/litestream/main.go

@@ -68,7 +68,7 @@ func (e *ConfigValidationError) Unwrap() error {
 
 func main() {
 	m := NewMain()
-	if err := m.Run(context.Background(), os.Args[1:]); err == flag.ErrHelp || err == errStop {
+	if err := m.Run(context.Background(), os.Args[1:]); errors.Is(err, flag.ErrHelp) || errors.Is(err, errStop) {
 		os.Exit(1)
 	} else if err != nil {
 		slog.Error("failed to run", "error", err)
@@ -114,7 +114,7 @@ func (m *Main) Run(ctx context.Context, args []string) (err error) {
 		// Setup signal handler.
 		signalCh := signalChan()
 
-		if err := c.Run(); err != nil {
+		if err := c.Run(ctx); err != nil {
 			return err
 		}
 
@@ -139,7 +139,7 @@ func (m *Main) Run(ctx context.Context, args []string) (err error) {
 		}
 
 		// Gracefully close.
-		if e := c.Close(); e != nil && err == nil {
+		if e := c.Close(ctx); e != nil && err == nil {
 			err = e
 		}
 		slog.Info("litestream shut down")
@@ -330,9 +330,9 @@ func (c *Config) CompactionLevels() litestream.CompactionLevels {
 }
 
 // DBConfig returns database configuration by path.
-func (c *Config) DBConfig(path string) *DBConfig {
+func (c *Config) DBConfig(configPath string) *DBConfig {
 	for _, dbConfig := range c.DBs {
-		if dbConfig.Path == path {
+		if dbConfig.Path == configPath {
 			return dbConfig
 		}
 	}
@@ -477,13 +477,13 @@ type DBConfig struct {
 
 // NewDBFromConfig instantiates a DB based on a configuration.
 func NewDBFromConfig(dbc *DBConfig) (*litestream.DB, error) {
-	path, err := expand(dbc.Path)
+	configPath, err := expand(dbc.Path)
 	if err != nil {
 		return nil, err
 	}
 
 	// Initialize database with given path.
-	db := litestream.NewDB(path)
+	db := litestream.NewDB(configPath)
 
 	// Override default database settings if specified in configuration.
 	if dbc.MetaPath != nil {
@@ -508,11 +508,12 @@ func NewDBFromConfig(dbc *DBConfig) (*litestream.DB, error) {
 	// Instantiate and attach replica.
 	// v0.3.x and before supported multiple replicas but that was dropped to
 	// ensure there's a single remote data authority.
-	if dbc.Replica == nil && len(dbc.Replicas) == 0 {
+	switch {
+	case dbc.Replica == nil && len(dbc.Replicas) == 0:
 		return nil, fmt.Errorf("must specify replica for database")
-	} else if dbc.Replica != nil && len(dbc.Replicas) > 0 {
+	case dbc.Replica != nil && len(dbc.Replicas) > 0:
 		return nil, fmt.Errorf("cannot specify 'replica' and 'replicas' on a database")
-	} else if len(dbc.Replicas) > 1 {
+	case len(dbc.Replicas) > 1:
 		return nil, fmt.Errorf("multiple replicas on a single database are no longer supported")
 	}
 
@@ -649,26 +650,26 @@ func newFileReplicaClientFromConfig(c *ReplicaConfig, r *litestream.Replica) (_
 		return nil, fmt.Errorf("cannot specify url & path for file replica")
 	}
 
-	// Parse path from URL, if specified.
-	path := c.Path
+	// Parse configPath from URL, if specified.
+	configPath := c.Path
 	if c.URL != "" {
-		if _, _, path, err = ParseReplicaURL(c.URL); err != nil {
+		if _, _, configPath, err = ParseReplicaURL(c.URL); err != nil {
 			return nil, err
 		}
 	}
 
 	// Ensure path is set explicitly or derived from URL field.
-	if path == "" {
+	if configPath == "" {
 		return nil, fmt.Errorf("file replica path required")
 	}
 
 	// Expand home prefix and return absolute path.
-	if path, err = expand(path); err != nil {
+	if configPath, err = expand(configPath); err != nil {
 		return nil, err
 	}
 
 	// Instantiate replica and apply time fields, if set.
-	client := file.NewReplicaClient(path)
+	client := file.NewReplicaClient(configPath)
 	client.Replica = r
 	return client, nil
 }
@@ -682,7 +683,7 @@ func newS3ReplicaClientFromConfig(c *ReplicaConfig, _ *litestream.Replica) (_ *s
 		return nil, fmt.Errorf("cannot specify url & bucket for s3 replica")
 	}
 
-	bucket, path := c.Bucket, c.Path
+	bucket, configPath := c.Bucket, c.Path
 	region, endpoint, skipVerify := c.Region, c.Endpoint, c.SkipVerify
 
 	// Use path style if an endpoint is explicitly set. This works because the
@@ -701,8 +702,8 @@ func newS3ReplicaClientFromConfig(c *ReplicaConfig, _ *litestream.Replica) (_ *s
 		ubucket, uregion, uendpoint, uforcePathStyle := s3.ParseHost(host)
 
 		// Only apply URL parts to field that have not been overridden.
-		if path == "" {
-			path = upath
+		if configPath == "" {
+			configPath = upath
 		}
 		if bucket == "" {
 			bucket = ubucket
@@ -728,7 +729,7 @@ func newS3ReplicaClientFromConfig(c *ReplicaConfig, _ *litestream.Replica) (_ *s
 	client.AccessKeyID = c.AccessKeyID
 	client.SecretAccessKey = c.SecretAccessKey
 	client.Bucket = bucket
-	client.Path = path
+	client.Path = configPath
 	client.Region = region
 	client.Endpoint = endpoint
 	client.ForcePathStyle = forcePathStyle
@@ -745,7 +746,7 @@ func newGSReplicaClientFromConfig(c *ReplicaConfig, _ *litestream.Replica) (_ *g
 		return nil, fmt.Errorf("cannot specify url & bucket for gs replica")
 	}
 
-	bucket, path := c.Bucket, c.Path
+	bucket, configPath := c.Bucket, c.Path
 
 	// Apply settings from URL, if specified.
 	if c.URL != "" {
@@ -755,8 +756,8 @@ func newGSReplicaClientFromConfig(c *ReplicaConfig, _ *litestream.Replica) (_ *g
 		}
 
 		// Only apply URL parts to field that have not been overridden.
-		if path == "" {
-			path = upath
+		if configPath == "" {
+			configPath = upath
 		}
 		if bucket == "" {
 			bucket = uhost
@@ -771,7 +772,7 @@ func newGSReplicaClientFromConfig(c *ReplicaConfig, _ *litestream.Replica) (_ *g
 	// Build replica.
 	client := gs.NewReplicaClient()
 	client.Bucket = bucket
-	client.Path = path
+	client.Path = configPath
 	return client, nil
 }
 
@@ -873,7 +874,7 @@ func newNATSReplicaClientFromConfig(c *ReplicaConfig, _ *litestream.Replica) (_
 	// Parse URL if provided to extract bucket name and server URL
 	var url, bucket string
 	if c.URL != "" {
-		scheme, host, path, err := ParseReplicaURL(c.URL)
+		scheme, host, bucketPath, err := ParseReplicaURL(c.URL)
 		if err != nil {
 			return nil, fmt.Errorf("invalid NATS URL: %w", err)
 		}
@@ -887,8 +888,8 @@ func newNATSReplicaClientFromConfig(c *ReplicaConfig, _ *litestream.Replica) (_
 		}
 
 		// Extract bucket name from path
-		if path != "" {
-			bucket = strings.Trim(path, "/")
+		if bucketPath != "" {
+			bucket = strings.Trim(bucketPath, "/")
 		}
 	}
 

cmd/litestream/main_windows.go

@@ -68,7 +68,7 @@ func (s *windowsService) Execute(args []string, r <-chan svc.ChangeRequest, stat
 	}
 
 	// Execute replication command.
-	if err := c.Run(); err != nil {
+	if err := c.Run(s.ctx); err != nil {
 		slog.Error("cannot replicate", "error", err)
 		statusCh <- svc.Status{State: svc.StopPending}
 		return true, 2
@@ -82,7 +82,7 @@ func (s *windowsService) Execute(args []string, r <-chan svc.ChangeRequest, stat
 		case req := <-r:
 			switch req.Cmd {
 			case svc.Stop:
-				c.Close()
+				c.Close(s.ctx)
 				statusCh <- svc.Status{State: svc.StopPending}
 				return false, windows.NO_ERROR
 			case svc.Interrogate:

cmd/litestream/mcp.go

@@ -50,8 +50,9 @@ func NewMCP(ctx context.Context, configPath string) (*MCPServer, error) {
 
 func (s *MCPServer) Start(addr string) {
 	s.httpServer = &http.Server{
-		Addr:    addr,
-		Handler: s.mux,
+		Addr:              addr,
+		Handler:           s.mux,
+		ReadHeaderTimeout: 30 * time.Second,
 	}
 	go func() {
 		slog.Info("Starting MCP Streamable HTTP server", "addr", addr)
@@ -65,7 +66,7 @@ func (s *MCPServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	s.mux.ServeHTTP(w, r)
 }
 
-// Close attemps to gracefully shutdown the server.
+// Close attempts to gracefully shutdown the server.
 func (s *MCPServer) Close() error {
 	ctx, cancel := context.WithTimeout(s.ctx, 10*time.Second)
 	defer cancel()

cmd/litestream/replicate.go

@@ -44,7 +44,7 @@ func NewReplicateCommand() *ReplicateCommand {
 }
 
 // ParseFlags parses the CLI flags and loads the configuration file.
-func (c *ReplicateCommand) ParseFlags(ctx context.Context, args []string) (err error) {
+func (c *ReplicateCommand) ParseFlags(_ context.Context, args []string) (err error) {
 	fs := flag.NewFlagSet("litestream-replicate", flag.ContinueOnError)
 	execFlag := fs.String("exec", "", "execute subcommand")
 	configPath, noExpandEnv := registerConfigFlag(fs)
@@ -54,9 +54,22 @@ func (c *ReplicateCommand) ParseFlags(ctx context.Context, args []string) (err e
 	}
 
 	// Load configuration or use CLI args to build db/replica.
-	if fs.NArg() == 1 {
+	switch fs.NArg() {
+	case 0:
+		// No arguments provided, use config file
+		if *configPath == "" {
+			*configPath = DefaultConfigPath()
+		}
+		if c.Config, err = ReadConfigFile(*configPath, !*noExpandEnv); err != nil {
+			return err
+		}
+
+	case 1:
+		// Only database path provided, missing replica URL
 		return fmt.Errorf("must specify at least one replica URL for %s", fs.Arg(0))
-	} else if fs.NArg() > 1 {
+
+	default:
+		// Database path and replica URLs provided via CLI
 		if *configPath != "" {
 			return fmt.Errorf("cannot specify a replica URL and the -config flag")
 		}
@@ -73,14 +86,8 @@ func (c *ReplicateCommand) ParseFlags(ctx context.Context, args []string) (err e
 			})
 		}
 		c.Config.DBs = []*DBConfig{dbConfig}
-	} else {
-		if *configPath == "" {
-			*configPath = DefaultConfigPath()
-		}
-		if c.Config, err = ReadConfigFile(*configPath, !*noExpandEnv); err != nil {
-			return err
-		}
 	}
+
 	c.Config.ConfigPath = *configPath
 
 	// Override config exec command, if specified.
@@ -92,13 +99,13 @@ func (c *ReplicateCommand) ParseFlags(ctx context.Context, args []string) (err e
 }
 
 // Run loads all databases specified in the configuration.
-func (c *ReplicateCommand) Run() (err error) {
+func (c *ReplicateCommand) Run(ctx context.Context) (err error) {
 	// Display version information.
 	slog.Info("litestream", "version", Version, "level", c.Config.Logging.Level)
 
 	// Start MCP server if enabled
 	if c.Config.MCPAddr != "" {
-		c.MCP, err = NewMCP(context.Background(), c.Config.ConfigPath)
+		c.MCP, err = NewMCP(ctx, c.Config.ConfigPath)
 		if err != nil {
 			return err
 		}
@@ -110,7 +117,7 @@ func (c *ReplicateCommand) Run() (err error) {
 		slog.Error("no databases specified in configuration")
 	}
 
-	var dbs []*litestream.DB
+	dbs := make([]*litestream.DB, 0, len(c.Config.DBs))
 	for _, dbConfig := range c.Config.DBs {
 		db, err := NewDBFromConfig(dbConfig)
 		if err != nil {
@@ -129,30 +136,30 @@ func (c *ReplicateCommand) Run() (err error) {
 	if c.Config.Snapshot.Retention != nil {
 		c.Store.SnapshotRetention = *c.Config.Snapshot.Retention
 	}
-	if err := c.Store.Open(context.Background()); err != nil {
+	if err := c.Store.Open(ctx); err != nil {
 		return fmt.Errorf("cannot open store: %w", err)
 	}
 
 	// Notify user that initialization is done.
 	for _, db := range c.Store.DBs() {
 		r := db.Replica
 		slog.Info("initialized db", "path", db.Path())
-		slog := slog.With("type", r.Client.Type(), "sync-interval", r.SyncInterval)
+		slogWith := slog.With("type", r.Client.Type(), "sync-interval", r.SyncInterval)
 		switch client := r.Client.(type) {
 		case *file.ReplicaClient:
-			slog.Info("replicating to", "path", client.Path())
+			slogWith.Info("replicating to", "path", client.Path())
 		case *s3.ReplicaClient:
-			slog.Info("replicating to", "bucket", client.Bucket, "path", client.Path, "region", client.Region, "endpoint", client.Endpoint)
+			slogWith.Info("replicating to", "bucket", client.Bucket, "path", client.Path, "region", client.Region, "endpoint", client.Endpoint)
 		case *gs.ReplicaClient:
-			slog.Info("replicating to", "bucket", client.Bucket, "path", client.Path)
+			slogWith.Info("replicating to", "bucket", client.Bucket, "path", client.Path)
 		case *abs.ReplicaClient:
-			slog.Info("replicating to", "bucket", client.Bucket, "path", client.Path, "endpoint", client.Endpoint)
+			slogWith.Info("replicating to", "bucket", client.Bucket, "path", client.Path, "endpoint", client.Endpoint)
 		case *sftp.ReplicaClient:
-			slog.Info("replicating to", "host", client.Host, "user", client.User, "path", client.Path)
+			slogWith.Info("replicating to", "host", client.Host, "user", client.User, "path", client.Path)
 		case *nats.ReplicaClient:
-			slog.Info("replicating to", "bucket", client.BucketName, "url", client.URL)
+			slogWith.Info("replicating to", "bucket", client.BucketName, "url", client.URL)
 		default:
-			slog.Info("replicating to")
+			slogWith.Info("replicating to")
 		}
 	}
 
@@ -181,7 +188,7 @@ func (c *ReplicateCommand) Run() (err error) {
 			return fmt.Errorf("cannot parse exec command: %w", err)
 		}
 
-		c.cmd = exec.Command(execArgs[0], execArgs[1:]...)
+		c.cmd = exec.CommandContext(ctx, execArgs[0], execArgs[1:]...)
 		c.cmd.Env = os.Environ()
 		c.cmd.Stdout = os.Stdout
 		c.cmd.Stderr = os.Stderr
@@ -195,16 +202,16 @@ func (c *ReplicateCommand) Run() (err error) {
 }
 
 // Close closes all open databases.
-func (c *ReplicateCommand) Close() (err error) {
-	if e := c.Store.Close(); e != nil {
-		slog.Error("failed to close database", "error", e)
+func (c *ReplicateCommand) Close(ctx context.Context) error {
+	if err := c.Store.Close(ctx); err != nil {
+		slog.Error("failed to close database", "error", err)
 	}
 	if c.Config.MCPAddr != "" {
 		if err := c.MCP.Close(); err != nil {
 			slog.Error("error closing MCP server", "error", err)
 		}
 	}
-	return err
+	return nil
 }
 
 // Usage prints the help screen to STDOUT.

cmd/litestream/restore.go

@@ -50,7 +50,7 @@ func (c *RestoreCommand) Run(ctx context.Context, args []string) (err error) {
 		if *configPath != "" {
 			return fmt.Errorf("cannot specify a replica URL and the -config flag")
 		}
-		if r, err = c.loadFromURL(ctx, fs.Arg(0), *ifDBNotExists, &opt); err == errSkipDBExists {
+		if r, err = c.loadFromURL(ctx, fs.Arg(0), *ifDBNotExists, &opt); errors.Is(err, errSkipDBExists) {
 			slog.Info("database already exists, skipping")
 			return nil
 		} else if err != nil {
@@ -60,7 +60,7 @@ func (c *RestoreCommand) Run(ctx context.Context, args []string) (err error) {
 		if *configPath == "" {
 			*configPath = DefaultConfigPath()
 		}
-		if r, err = c.loadFromConfig(ctx, fs.Arg(0), *configPath, !*noExpandEnv, *ifDBNotExists, &opt); err == errSkipDBExists {
+		if r, err = c.loadFromConfig(ctx, fs.Arg(0), *configPath, !*noExpandEnv, *ifDBNotExists, &opt); errors.Is(err, errSkipDBExists) {
 			slog.Info("database already exists, skipping")
 			return nil
 		} else if err != nil {