chore: ensure that errors aren't ignored via _ (#1829)

Forces use of a `//nolint:errcheck // <reason>` directive, so future me
doesn't wonder why I ignored the error.

.golangci.yml

@@ -85,6 +85,8 @@ linters-settings:
   gocritic:
     disabled-checks:
       - ifElseChain
+  errcheck:
+    check-blank: true
   depguard:
     rules:
       main:

authn/authn.go

@@ -140,7 +140,10 @@ func checkAuth(ctx context.Context, logger *log.Logger, endpoint *url.URL, creds
 	}
 	defer resp.Body.Close() //nolint:gosec
 	if resp.StatusCode != http.StatusOK {
-		body, _ := io.ReadAll(resp.Body)
+		body, err := io.ReadAll(resp.Body)
+		if err != nil {
+			return nil, fmt.Errorf("failed to read response body: %w", err)
+		}
 		logger.Tracef("Endpoint returned %d for authenticated request", resp.StatusCode)
 		logger.Tracef("Response headers: %s", resp.Header)
 		logger.Tracef("Response body: %s", body)

backend/controller/admin/admin.go

@@ -56,7 +56,10 @@ func (s *AdminService) ConfigList(ctx context.Context, req *connect.Request[ftlv
 			if err != nil {
 				return nil, err
 			}
-			cv, _ = json.Marshal(value)
+			cv, err = json.Marshal(value)
+			if err != nil {
+				return nil, fmt.Errorf("failed to marshal value for %s: %w", ref, err)
+			}
 		}
 
 		configs = append(configs, &ftlv1.ListConfigResponse_Config{
@@ -139,7 +142,10 @@ func (s *AdminService) SecretsList(ctx context.Context, req *connect.Request[ftl
 			if err != nil {
 				return nil, err
 			}
-			sv, _ = json.Marshal(value)
+			sv, err = json.Marshal(value)
+			if err != nil {
+				return nil, fmt.Errorf("failed to marshal value for %s: %w", ref, err)
+			}
 		}
 		secrets = append(secrets, &ftlv1.ListSecretsResponse_Secret{
 			RefPath: ref,

backend/controller/controller.go

@@ -106,7 +106,7 @@ func Start(ctx context.Context, config Config, runnerScaling scaling.RunnerScali
 	if config.NoConsole {
 		consoleHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			w.WriteHeader(http.StatusNotImplemented)
-			_, _ = w.Write([]byte("Console not installed."))
+			_, _ = w.Write([]byte("Console not installed.")) //nolint:errcheck
 		})
 	} else {
 		consoleHandler, err = frontend.Server(ctx, config.ContentTime, config.Bind, config.ConsoleURL)
@@ -1165,7 +1165,9 @@ func (s *Service) reconcileDeployments(ctx context.Context) (time.Duration, erro
 		lock.Unlock()
 	}
 
-	_ = wg.Wait()
+	if err := wg.Wait(); err != nil {
+		return 0, fmt.Errorf("failed to reconcile deployments: %w", err)
+	}
 	return time.Second, nil
 }
 
@@ -1543,7 +1545,7 @@ func (s *Service) watchModuleChanges(ctx context.Context, sendChange func(respon
 
 func (s *Service) getDeploymentLogger(ctx context.Context, deploymentKey model.DeploymentKey) *log.Logger {
 	attrs := map[string]string{"deployment": deploymentKey.String()}
-	if requestKey, _ := rpc.RequestKeyFromContext(ctx); requestKey.Ok() {
+	if requestKey, _ := rpc.RequestKeyFromContext(ctx); requestKey.Ok() { //nolint:errcheck // best effort?
 		attrs["request"] = requestKey.MustGet().String()
 	}
 

backend/controller/cronjobs/cronjobs.go

@@ -166,7 +166,7 @@ func (s *Service) NewCronJobsForModule(ctx context.Context, module *schemapb.Mod
 // the newly created cron jobs until other controllers have a chance to resync their list of jobs and start sharing responsibility of the new cron jobs.
 func (s *Service) CreatedOrReplacedDeloyment(ctx context.Context, newDeploymentKey model.DeploymentKey) {
 	// Rather than finding old/new cron jobs and updating our state, we can just resync the list of jobs
-	_ = s.syncJobsWithNewDeploymentKey(ctx, optional.Some(newDeploymentKey))
+	_ = s.syncJobsWithNewDeploymentKey(ctx, optional.Some(newDeploymentKey)) //nolint:errcheck // TODO(matt2e) is this valid?
 }
 
 // SyncJobs is run periodically via a scheduled task

backend/controller/cronjobs/cronjobs_utils_test.go

@@ -205,7 +205,7 @@ func newControllers(ctx context.Context, count int, dal DAL, clockFactory func()
 					Key: ctrl.key,
 				}
 			}))
-			_, _ = s.syncJobs(ctx)
+			_, _ = s.syncJobs(ctx) //nolint:errcheck
 		}()
 	}
 

backend/controller/dal/dal_test.go

@@ -361,7 +361,8 @@ func TestDAL(t *testing.T) {
 			{Message: optional.Some(Deployment{Language: "go", Module: "test", Schema: &schema.Module{Name: "test"}})},
 			{Message: optional.Some(Deployment{Language: "go", Module: "test", MinReplicas: 1, Schema: &schema.Module{Name: "test"}})},
 		}
-		_ = wg.Wait()
+		err = wg.Wait()
+		assert.NoError(t, err)
 		assert.Equal(t, expectedDeploymentChanges, deploymentChanges,
 			assert.Exclude[model.DeploymentKey](), assert.Exclude[time.Time](), assert.IgnoreGoStringer())
 	})

backend/controller/sql/conn.go

@@ -2,6 +2,7 @@ package sql
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	"github.com/jackc/pgx/v5"
@@ -86,7 +87,7 @@ func (t *Tx) Rollback(ctx context.Context) error {
 //	}
 func (t *Tx) CommitOrRollback(ctx context.Context, err *error) {
 	if *err != nil {
-		_ = t.Rollback(ctx)
+		*err = errors.Join(*err, t.Rollback(ctx))
 	} else {
 		*err = t.Commit(ctx)
 	}

backend/controller/sql/databasetesting/devel.go

@@ -60,8 +60,7 @@ func CreateForDevel(ctx context.Context, dsn string, recreate bool) (*pgxpool.Po
 		}
 	}
 
-	// PG doesn't support "IF NOT EXISTS" so instead we just ignore any error.
-	_, _ = conn.Exec(ctx, fmt.Sprintf("CREATE DATABASE %q", config.Database))
+	_, _ = conn.Exec(ctx, fmt.Sprintf("CREATE DATABASE %q", config.Database)) //nolint:errcheck // PG doesn't support "IF NOT EXISTS" so instead we just ignore any error.
 
 	err = sql.Migrate(ctx, dsn)
 	if err != nil {

backend/controller/sql/sqltest/testing.go

@@ -22,7 +22,7 @@ func OpenForTesting(ctx context.Context, t testing.TB) *pgxpool.Pool {
 	lockPath := filepath.Join(os.TempDir(), "ftl-db-test.lock")
 	release, err := flock.Acquire(ctx, lockPath, 10*time.Second)
 	assert.NoError(t, err)
-	t.Cleanup(func() { _ = release() })
+	t.Cleanup(func() { _ = release() }) //nolint:errcheck
 
 	testDSN := "postgres://localhost:15432/ftl-test?user=postgres&password=secret&sslmode=disable"
 	conn, err := databasetesting.CreateForDevel(ctx, testDSN, true)

backend/runner/runner.go

@@ -458,7 +458,7 @@ func (s *Service) streamLogsLoop(ctx context.Context, send func(request *ftlv1.S
 
 func (s *Service) getDeploymentLogger(ctx context.Context, deploymentKey model.DeploymentKey) *log.Logger {
 	attrs := map[string]string{"deployment": deploymentKey.String()}
-	if requestKey, _ := rpc.RequestKeyFromContext(ctx); requestKey.Ok() {
+	if requestKey, _ := rpc.RequestKeyFromContext(ctx); requestKey.Ok() { //nolint:errcheck // best effort
 		attrs["request"] = requestKey.MustGet().String()
 	}
 

backend/schema/fsm.go

@@ -46,7 +46,10 @@ func (f *FSM) TerminalStates() []*Ref {
 	}
 	for key := range all {
 		if _, ok := in[key]; !ok {
-			ref, _ := ParseRef(key)
+			ref, err := ParseRef(key)
+			if err != nil {
+				panic(err) // key must be valid
+			}
 			out = append(out, ref)
 		}
 	}

backend/schema/module.go

@@ -190,7 +190,7 @@ func (m *Module) Data() []*Data {
 // Imports returns the modules imported by this module.
 func (m *Module) Imports() []string {
 	imports := map[string]bool{}
-	_ = Visit(m, func(n Node, next func() error) error {
+	_ = Visit(m, func(n Node, next func() error) error { //nolint:errcheck
 		switch n := n.(type) {
 		case *Ref:
 			if n.Module != "" && n.Module != m.Name {

backend/schema/normalise.go

@@ -7,7 +7,7 @@ import (
 // Normalise clones and normalises (zeroes) positional information in schema Nodes.
 func Normalise[T Node](n T) T {
 	ni := reflect.DeepCopy(n)
-	_ = Visit(ni, func(n Node, next func() error) error {
+	_ = Visit(ni, func(n Node, next func() error) error { //nolint:errcheck
 		switch n := n.(type) {
 		case *Bool:
 			n.Bool = false

backend/schema/protobuf.go

@@ -95,8 +95,8 @@ func generateStruct(t reflect.Type, messages map[string]string) {
 		if aid == "-" || bid == "-" {
 			return strings.Compare(aid, bid)
 		}
-		an, _ := strconv.Atoi(aid)
-		bn, _ := strconv.Atoi(bid)
+		an, _ := strconv.Atoi(aid) //nolint:errcheck // 0 is fine
+		bn, _ := strconv.Atoi(bid) //nolint:errcheck // 0 is fine
 		return an - bn
 	})
 	// Filter out fields with protobuf tag "-"

backend/schema/validate.go

@@ -265,7 +265,7 @@ func ValidateModule(module *Module) error {
 	// Key is <type>:<name>
 	duplicateDecls := map[string]Decl{}
 
-	_ = Visit(module, func(n Node, next func() error) error {
+	_ = Visit(module, func(n Node, next func() error) error { //nolint:errcheck
 		if scoped, ok := n.(Scoped); ok {
 			pop := scopes
 			scopes = scopes.PushScope(scoped.Scope())

buildengine/build_kotlin.go

@@ -9,11 +9,12 @@ import (
 	"sort"
 	"strings"
 
-	"github.com/TBD54566975/scaffolder"
 	"github.com/beevik/etree"
 	sets "github.com/deckarep/golang-set/v2"
 	"golang.org/x/exp/maps"
 
+	"github.com/TBD54566975/scaffolder"
+
 	"github.com/TBD54566975/ftl"
 	"github.com/TBD54566975/ftl/backend/schema"
 	"github.com/TBD54566975/ftl/internal"
@@ -156,7 +157,7 @@ var scaffoldFuncs = scaffolder.FuncMap{
 	},
 	"imports": func(m *schema.Module) []string {
 		imports := sets.NewSet[string]()
-		_ = schema.VisitExcludingMetadataChildren(m, func(n schema.Node, next func() error) error {
+		_ = schema.VisitExcludingMetadataChildren(m, func(n schema.Node, next func() error) error { //nolint:errcheck
 			switch n.(type) {
 			case *schema.Data:
 				imports.Add("xyz.block.ftl.Data")

cmd/ftl/cmd_serve.go

@@ -53,8 +53,7 @@ func (s *serveCmd) Run(ctx context.Context, projConfig projectconfig.Config) err
 	if s.Background {
 		if s.Stop {
 			// allow usage of --background and --stop together to "restart" the background process
-			// ignore error here if the process is not running
-			_ = KillBackgroundServe(logger)
+			_ = KillBackgroundServe(logger) //nolint:errcheck // ignore error here if the process is not running
 		}
 
 		if err := runInBackground(logger); err != nil {
@@ -141,7 +140,9 @@ func (s *serveCmd) Run(ctx context.Context, projConfig projectconfig.Config) err
 }
 
 func runInBackground(logger *log.Logger) error {
-	if running, _ := isServeRunning(logger); running {
+	if running, err := isServeRunning(logger); err != nil {
+		return fmt.Errorf("failed to check if FTL is running: %w", err)
+	} else if running {
 		logger.Warnf(ftlRunningErrorMsg)
 		return nil
 	}
@@ -162,7 +163,10 @@ func runInBackground(logger *log.Logger) error {
 		return fmt.Errorf("failed to start background process: %w", err)
 	}
 
-	pidFilePath, _ := pidFilePath()
+	pidFilePath, err := pidFilePath()
+	if err != nil {
+		return fmt.Errorf("failed to get pid file path: %w", err)
+	}
 	if err := os.MkdirAll(filepath.Dir(pidFilePath), 0750); err != nil {
 		return fmt.Errorf("failed to create directory for pid file: %w", err)
 	}

cmd/ftl/main.go

@@ -133,7 +133,7 @@ func main() {
 		sig := <-sigch
 		logger.Debugf("FTL terminating with signal %s", sig)
 		cancel()
-		_ = syscall.Kill(-syscall.Getpid(), sig.(syscall.Signal)) //nolint:forcetypeassert
+		_ = syscall.Kill(-syscall.Getpid(), sig.(syscall.Signal)) //nolint:forcetypeassert,errcheck // best effort
 		os.Exit(0)
 	}()
 

common/plugin/serve.go

@@ -128,7 +128,7 @@ func Start[Impl any, Iface any, Config any](
 		sig := <-sigch
 		logger.Debugf("Terminated by signal %s", sig)
 		cancel()
-		_ = syscall.Kill(-syscall.Getpid(), sig.(syscall.Signal)) //nolint:forcetypeassert
+		_ = syscall.Kill(-syscall.Getpid(), sig.(syscall.Signal)) //nolint:forcetypeassert,errcheck // best effort
 		os.Exit(0)
 	}()
 

common/plugin/spawn.go

@@ -151,7 +151,7 @@ func Spawn[Client PingableClient](
 	defer func() {
 		if err != nil {
 			logger.Warnf("Plugin failed to start, terminating pid %d", cmd.Process.Pid)
-			_ = cmd.Kill(syscall.SIGTERM)
+			_ = cmd.Kill(syscall.SIGTERM) //nolint:errcheck // best effort
 		}
 	}()
 

frontend/local.go

@@ -16,7 +16,7 @@ import (
 	"github.com/TBD54566975/ftl/internal/log"
 )
 
-var proxyURL, _ = url.Parse("http://localhost:5173")
+var proxyURL, _ = url.Parse("http://localhost:5173") //nolint:errcheck
 var proxy = httputil.NewSingleHostReverseProxy(proxyURL)
 
 func Server(ctx context.Context, timestamp time.Time, publicURL *url.URL, allowOrigin *url.URL) (http.Handler, error) {

go-runtime/compile/build.go

@@ -262,7 +262,7 @@ var scaffoldFuncs = scaffolder.FuncMap{
 	},
 	"imports": func(m *schema.Module) map[string]string {
 		imports := map[string]string{}
-		_ = schema.VisitExcludingMetadataChildren(m, func(n schema.Node, next func() error) error {
+		_ = schema.VisitExcludingMetadataChildren(m, func(n schema.Node, next func() error) error { //nolint:errcheck
 			switch n := n.(type) {
 			case *schema.Ref:
 				if n.Module == "" || n.Module == m.Name {
@@ -273,7 +273,7 @@ var scaffoldFuncs = scaffolder.FuncMap{
 				for _, tp := range n.TypeParameters {
 					tpRef, err := schema.ParseRef(tp.String())
 					if err != nil {
-						return err
+						panic(err)
 					}
 					if tpRef.Module != "" && tpRef.Module != m.Name {
 						imports[path.Join("ftl", tpRef.Module)] = "ftl" + tpRef.Module
@@ -657,7 +657,7 @@ func updateTransitiveVisibility(d schema.Decl, module *schema.Module) {
 		return
 	}
 
-	_ = schema.Visit(d, func(n schema.Node, next func() error) error {
+	_ = schema.Visit(d, func(n schema.Node, next func() error) error { //nolint:errcheck
 		ref, ok := n.(*schema.Ref)
 		if !ok {
 			return next()

go-runtime/compile/schema.go

@@ -1551,7 +1551,7 @@ func (p *parseContext) getDeclForTypeName(name string) optional.Option[schema.De
 }
 
 func (p *parseContext) markAsExported(node schema.Node) {
-	_ = schema.Visit(node, func(n schema.Node, next func() error) error {
+	_ = schema.Visit(node, func(n schema.Node, next func() error) error { //nolint:errcheck
 		if decl, ok := n.(schema.Decl); ok {
 			switch decl := decl.(type) {
 			case *schema.Enum:

go-runtime/compile/schema_test.go

@@ -10,10 +10,11 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/TBD54566975/golang-tools/go/packages"
 	"github.com/alecthomas/assert/v2"
 	"github.com/alecthomas/participle/v2/lexer"
 
+	"github.com/TBD54566975/golang-tools/go/packages"
+
 	"github.com/TBD54566975/ftl/backend/schema"
 	extract "github.com/TBD54566975/ftl/go-runtime/schema"
 	"github.com/TBD54566975/ftl/internal/errors"
@@ -343,7 +344,7 @@ func TestExtractModuleSchemaParent(t *testing.T) {
 	assert.Equal(t, nil, r.Errors, "expected no schema errors")
 	actual := schema.Normalise(r.Module)
 	expected := `module parent {
-		export typealias ChildAlias String 
+		export typealias ChildAlias String
 
 		export data ChildStruct {
 			name parent.ChildAlias?
@@ -379,7 +380,7 @@ func TestExtractModulePubSub(t *testing.T) {
         export data PayinEvent {
         	name String
         }
-        
+
 		export verb broadcast(Unit) Unit
 
         verb payin(Unit) Unit
@@ -498,12 +499,12 @@ func TestErrorReporting(t *testing.T) {
 		t.SkipNow()
 	}
 
-	// prebuild so we have external_module.go for pubsub module, but ignore these initial errors
-	_ = prebuildTestModule(t, "testdata/failing", "testdata/pubsub")
+	_ = prebuildTestModule(t, "testdata/failing", "testdata/pubsub") //nolint:errcheck // prebuild so we have external_module.go for pubsub module, but ignore these initial errors
 
 	ctx := log.ContextWithNewDefaultLogger(context.Background())
-	pwd, _ := os.Getwd()
-	err := exec.Command(ctx, log.Debug, "testdata/failing", "go", "mod", "tidy").RunBuffered(ctx)
+	pwd, err := os.Getwd()
+	assert.NoError(t, err)
+	err = exec.Command(ctx, log.Debug, "testdata/failing", "go", "mod", "tidy").RunBuffered(ctx)
 	assert.NoError(t, err)
 	r, err := ExtractModuleSchema("testdata/failing", &schema.Schema{})
 	assert.NoError(t, err)
@@ -578,8 +579,9 @@ func TestValidationFailures(t *testing.T) {
 		t.SkipNow()
 	}
 	ctx := log.ContextWithNewDefaultLogger(context.Background())
-	pwd, _ := os.Getwd()
-	err := exec.Command(ctx, log.Debug, "testdata/validation", "go", "mod", "tidy").RunBuffered(ctx)
+	pwd, err := os.Getwd()
+	assert.NoError(t, err)
+	err = exec.Command(ctx, log.Debug, "testdata/validation", "go", "mod", "tidy").RunBuffered(ctx)
 	assert.NoError(t, err)
 	_, err = ExtractModuleSchema("testdata/validation", &schema.Schema{})
 	assert.Error(t, err)