Catch or explicitly ignore unhandled errors (#104)

Signed-off-by: Geoff Franks <[email protected]> Co-authored-by: Amelia Downs <[email protected]>
cloudfoundry · Sep 26, 2024 · 155fe39 · 155fe39
1 parent 4b574f9
commit 155fe39
Show file tree

Hide file tree

Showing 13 changed files with 56 additions and 16 deletions.
diff --git a/cmd/bbs/main.go b/cmd/bbs/main.go
@@ -383,7 +383,10 @@ func main() {
 
 	err = <-monitor.Wait()
 	if sqlConn != nil {
-		sqlConn.Close()
+		closeErr := sqlConn.Close()
+		if closeErr != nil {
+			logger.Error("failed-to-close-sql-conn", closeErr)
+		}
 	}
 	if err != nil {
 		logger.Error("exited-with-failure", err)

diff --git a/cmd/bbs/testrunner/runner.go b/cmd/bbs/testrunner/runner.go
@@ -30,7 +30,7 @@ func New(binPath string, bbsConfig config.BBSConfig) *ginkgomon.Runner {
 		StartCheck:        "bbs.started",
 		StartCheckTimeout: 1 * time.Minute,
 		Cleanup: func() {
-			// do not use Expect otherwise a race condition will happen
+			// #nosec G104 - do not use Expect otherwise a race condition will happen
 			os.RemoveAll(f.Name())
 		},
 	})

diff --git a/controllers/actual_lrp_lifecycle_controller.go b/controllers/actual_lrp_lifecycle_controller.go
@@ -148,7 +148,10 @@ func (h *ActualLRPLifecycleController) StartActualLRP(ctx context.Context,
 	suspect := findWithPresence(lrps, models.ActualLRP_Suspect)
 
 	if evacuating != nil {
-		h.evacuationDB.RemoveEvacuatingActualLRP(ctx, logger, &evacuating.ActualLRPKey, &evacuating.ActualLRPInstanceKey)
+		err = h.evacuationDB.RemoveEvacuatingActualLRP(ctx, logger, &evacuating.ActualLRPKey, &evacuating.ActualLRPInstanceKey)
+		if err != nil {
+			logger.Error("failed-to-remove-evacuating-actual-lrp", err, lager.Data{"instance-guid": evacuating.ActualLRPInstanceKey})
+		}
 		newLRPs = eventCalculator.RecordChange(evacuating, nil, newLRPs)
 	}
 

diff --git a/controllers/task_controller.go b/controllers/task_controller.go
@@ -131,7 +131,7 @@ func (c *TaskController) CancelTask(ctx context.Context, logger lager.Logger, ta
 		return err
 	}
 	logger.Info("start-rep-cancel-task", lager.Data{"task_guid": taskGUID})
-	repClient.CancelTask(logger, taskGUID)
+	err = repClient.CancelTask(logger, taskGUID)
 	if err != nil {
 		logger.Error("failed-rep-cancel-task", err)
 		// don't return an error, the rep will converge later

diff --git a/db/migrations/1474993971_encrypt_routes.go b/db/migrations/1474993971_encrypt_routes.go
@@ -72,7 +72,10 @@ func (e *EncryptRoutes) Up(tx *sql.Tx, logger lager.Logger) error {
 		}
 		routeDataMap[processGuid] = routeData
 	}
-	rows.Close()
+	err = rows.Close()
+	if err != nil {
+		logger.Error("failed-to-close-row", err)
+	}
 
 	for pGuid, rData := range routeDataMap {
 		encodedData, err := e.encoder.Encode(rData)

diff --git a/db/migrations/1722634733_split_metric_tags.go b/db/migrations/1722634733_split_metric_tags.go
@@ -101,7 +101,10 @@ func (e *SplitMetricTags) Up(tx *sql.Tx, logger lager.Logger) error {
 		}
 		metricTagsMap[processGuid] = metricTags
 	}
-	rows.Close()
+	err = rows.Close()
+	if err != nil {
+		logger.Error("failed-to-close-row", err)
+	}
 
 	for pGuid, metricTags := range metricTagsMap {
 		updateQuery := "UPDATE desired_lrps SET metric_tags = ? WHERE process_guid = ?"

diff --git a/db/sqldb/desired_lrp_db.go b/db/sqldb/desired_lrp_db.go
@@ -535,7 +535,10 @@ func (db *SQLDB) fetchDesiredLRPs(ctx context.Context, logger lager.Logger, rows
 	}
 
 	if len(guids) > 0 {
-		db.deleteInvalidLRPs(ctx, logger, queryable, guids...)
+		deleteErr := db.deleteInvalidLRPs(ctx, logger, queryable, guids...)
+		if deleteErr != nil {
+			logger.Error("failed-to-delete-invalid-lrps", deleteErr, lager.Data{"guid": guids})
+		}
 	}
 
 	if err := rows.Err(); err != nil {
@@ -548,7 +551,10 @@ func (db *SQLDB) fetchDesiredLRPs(ctx context.Context, logger lager.Logger, rows
 func (db *SQLDB) fetchDesiredLRP(ctx context.Context, logger lager.Logger, scanner helpers.RowScanner, queryable helpers.Queryable) (*models.DesiredLRP, error) {
 	lrp, guid, err := db.fetchDesiredLRPInternal(logger, scanner)
 	if err == models.ErrDeserialize {
-		db.deleteInvalidLRPs(ctx, logger, queryable, guid)
+		deleteErr := db.deleteInvalidLRPs(ctx, logger, queryable, guid)
+		if deleteErr != nil {
+			logger.Error("failed-to-delete-invalid-lrp", deleteErr, lager.Data{"guid": guid})
+		}
 	}
 	return lrp, err
 }

diff --git a/db/sqldb/task_db.go b/db/sqldb/task_db.go
@@ -478,10 +478,16 @@ func (db *SQLDB) fetchTasks(ctx context.Context, logger lager.Logger, rows *sql.
 		err = rows.Err()
 	}
 
-	rows.Close()
+	closeErr := rows.Close()
+	if closeErr != nil {
+		logger.Debug("failed-to-close-row", lager.Data{"error": closeErr})
+	}
 
 	if len(invalidGuids) > 0 {
-		db.deleteInvalidTasks(ctx, logger, queryable, invalidGuids...)
+		deleteErr := db.deleteInvalidTasks(ctx, logger, queryable, invalidGuids...)
+		if deleteErr != nil {
+			logger.Error("failed-to-delete-invalid-task", err, lager.Data{"guids": invalidGuids})
+		}
 	}
 
 	return tasks, validGuids, len(invalidGuids), err
@@ -490,7 +496,10 @@ func (db *SQLDB) fetchTasks(ctx context.Context, logger lager.Logger, rows *sql.
 func (db *SQLDB) fetchTask(ctx context.Context, logger lager.Logger, scanner helpers.RowScanner, queryable helpers.Queryable) (*models.Task, error) {
 	task, guid, err := db.fetchTaskInternal(logger, scanner)
 	if err == models.ErrDeserialize {
-		db.deleteInvalidTasks(ctx, logger, queryable, guid)
+		deleteErr := db.deleteInvalidTasks(ctx, logger, queryable, guid)
+		if deleteErr != nil {
+			logger.Error("failed-to-delete-invalid-task", err, lager.Data{"guid": guid})
+		}
 	}
 	return task, err
 }

diff --git a/encryptor/encryptor.go b/encryptor/encryptor.go
@@ -77,7 +77,10 @@ func (m Encryptor) Run(signals <-chan os.Signal, ready chan<- struct{}) error {
 		if err != nil {
 			logger.Error("encryption-failed", err)
 		} else {
-			m.db.SetEncryptionKeyLabel(context.Background(), logger, m.keyManager.EncryptionKey().Label())
+			err = m.db.SetEncryptionKeyLabel(context.Background(), logger, m.keyManager.EncryptionKey().Label())
+			if err != nil {
+				return err
+			}
 		}
 
 		totalTime := m.clock.Since(encryptionStart)

diff --git a/handlers/events_handlers.go b/handlers/events_handlers.go
@@ -80,6 +80,7 @@ func streamEventsToResponse(logger lager.Logger, w http.ResponseWriter, eventCha
 	eventID := 0
 	done := make(chan bool, 1)
 	go func() {
+		// #nosec G104 - ignore errors when reading hijacked HTTP requests so we don't spam our logs during a DoS
 		rw.ReadFrom(conn)
 		done <- true
 	}()

diff --git a/handlers/handlers.go b/handlers/handlers.go
@@ -204,6 +204,6 @@ func writeResponse(w http.ResponseWriter, message proto.Message) {
 	w.Header().Set("Content-Length", strconv.Itoa(len(responseBytes)))
 	w.Header().Set("Content-Type", "application/x-protobuf")
 	w.WriteHeader(http.StatusOK)
-
+	// #nosec G104 - ignore errors when writing HTTP responses so we don't spam our logs during a DoS
 	w.Write(responseBytes)
 }
diff --git a/metrics/bbs_election_metron_notifier.go b/metrics/bbs_election_metron_notifier.go
@@ -32,7 +32,10 @@ func (notifier BBSElectionMetronNotifier) Run(signals <-chan os.Signal, ready ch
 	logger.Info("started")
 	defer logger.Info("finished")
 
-	notifier.metronClient.SendMetric(bbsMasterElectedMetric, 1)
+	err := notifier.metronClient.SendMetric(bbsMasterElectedMetric, 1)
+	if err != nil {
+		logger.Debug("failed-to-emit-bbs-master-elected-metric", lager.Data{"error": err})
+	}
 
 	<-signals
 	return nil

diff --git a/metrics/request_stat_metron_notifier.go b/metrics/request_stat_metron_notifier.go
@@ -67,12 +67,18 @@ func (notifier *RequestStatMetronNotifier) Run(signals <-chan os.Signal, ready c
 		case <-notifier.ticker.C():
 			add := atomic.SwapUint64(&notifier.requestCount, 0)
 			logger.Info("adding-counter", lager.Data{"add": add})
-			notifier.metronClient.IncrementCounterWithDelta(requestCounter, add)
+			metricErr := notifier.metronClient.IncrementCounterWithDelta(requestCounter, add)
+			if metricErr != nil {
+				logger.Debug("failed-to-emit-request-counter", lager.Data{"error": metricErr})
+			}
 
 			latency := notifier.ReadAndResetLatency()
 			if latency != 0 {
 				logger.Info("sending-latency", lager.Data{"latency": latency})
-				notifier.metronClient.SendDuration(requestLatencyDuration, latency)
+				metricErr := notifier.metronClient.SendDuration(requestLatencyDuration, latency)
+				if metricErr != nil {
+					logger.Debug("failed-to-emit-request-latency-metric", lager.Data{"error": metricErr})
+				}
 			}
 		case <-signals:
 			return nil