transactions: Log error cases

While we already log a transaction that is being started as well as a
successful transaction, we do not yet any error cases. Refactor the code
to put verification of transactions into its own function to unify all
current error cases into a single path and log any returned errors.

1 files changed, 28 insertions, 15 deletions

diff --git a/internal/praefect/transactions/manager.go b/internal/praefect/transactions/manager.go
index 2a9b85382..d0a5a11df 100644
--- a/internal/praefect/transactions/manager.go
+++ b/internal/praefect/transactions/manager.go
@@ -76,6 +76,26 @@ func (mgr *Manager) cancelTransaction(transactionID uint64) {
 	delete(mgr.transactions, transactionID)
 }
 
+func (mgr *Manager) verifyTransaction(transactionID uint64, node string, hash []byte) error {
+	// While the reference updates hash is not used yet, we already verify
+	// it's there. At a later point, the hash will be used to verify that
+	// all voting nodes agree on the same updates.
+	if len(hash) != sha1.Size {
+		return helper.ErrInvalidArgumentf("invalid reference hash: %q", hash)
+	}
+
+	transaction, ok := mgr.transactions[transactionID]
+	if !ok {
+		return helper.ErrNotFound(fmt.Errorf("no such transaction: %d", transactionID))
+	}
+
+	if transaction != node {
+		return helper.ErrInternalf("invalid node for transaction: %q", node)
+	}
+
+	return nil
+}
+
 // StartTransaction is called by a client who's starting a reference
 // transaction. As we currently only have primary nodes which perform reference
 // transactions, this function doesn't yet do anything of interest but will
@@ -92,26 +112,19 @@ func (mgr *Manager) StartTransaction(ctx context.Context, transactionID uint64,
 		"hash":           hex.EncodeToString(hash),
 	}).Debug("StartTransaction")
 
-	// While the reference updates hash is not used yet, we already verify
-	// it's there. At a later point, the hash will be used to verify that
-	// all voting nodes agree on the same updates.
-	if len(hash) != sha1.Size {
-		return helper.ErrInvalidArgumentf("invalid reference hash: %q", hash)
-	}
-
-	transaction, ok := mgr.transactions[transactionID]
-	if !ok {
-		return helper.ErrNotFound(fmt.Errorf("no such transaction: %d", transactionID))
-	}
-
-	if transaction != node {
-		return helper.ErrInternalf("invalid node for transaction: %q", node)
+	if err := mgr.verifyTransaction(transactionID, node, hash); err != nil {
+		mgr.log(ctx).WithFields(logrus.Fields{
+			"transaction_id": transactionID,
+			"node":           node,
+			"hash":           hex.EncodeToString(hash),
+		}).WithError(err).Error("StartTransaction: transaction invalid")
+		return err
 	}
 
 	mgr.log(ctx).WithFields(logrus.Fields{
 		"transaction_id": transactionID,
 		"hash":           hex.EncodeToString(hash),
-	}).Debug("CommitTransaction")
+	}).Debug("StartTransaction: transaction committed")
 
 	return nil
 }