From e0cc910f77235d49f7e1d3dac66d4defbe8872d1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= <mitr@redhat.com>
Date: Tue, 22 Feb 2022 01:11:26 +0100
Subject: [PATCH] Consolidate the error handling cleanup into a defer
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

... so that we don't repeat it all over the place.

Introduce a pretty ugly cleanupFailureContext variable
for that purpose; still, it's better than copy&pasting everything.

This will be even more useful soon.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 layers.go | 39 +++++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/layers.go b/layers.go
index 89198c0782..be3bf920f0 100644
--- a/layers.go
+++ b/layers.go
@@ -796,40 +796,43 @@ func (r *layerStore) Put(id string, parentLayer *Layer, names []string, mountLab
 		layer.Flags[flag] = value
 	}
 	layer.Flags[incompleteFlag] = true
+
+	succeeded := false
+	cleanupFailureContext := ""
+	defer func() {
+		if !succeeded {
+			// On any error, try both removing the driver's data as well
+			// as the in-memory layer record.
+			if err2 := r.Delete(layer.ID); err2 != nil {
+				if cleanupFailureContext == "" {
+					cleanupFailureContext = "unknown: cleanupFailureContext not set at the failure site"
+				}
+				logrus.Errorf("While recovering from a failure (%s), error deleting layer %#v: %v", cleanupFailureContext, layer.ID, err2)
+			}
+		}
+	}()
+
 	err = r.Save()
 	if err != nil {
-		// We don't have a presistent record of this layer, but
-		// try to remove both the driver’s data as well as
-		// the in-memory layer record.
-		if err2 := r.Delete(layer.ID); err2 != nil {
-			logrus.Errorf("While recovering from a failure saving incomplete layer metadata, error deleting layer %#v: %v", id, err2)
-		}
+		cleanupFailureContext = "saving incomplete layer metadata"
 		return nil, -1, err
 	}
 	if diff != nil {
 		size, err = r.applyDiffWithOptions(layer.ID, moreOptions, diff)
 		if err != nil {
-			if err2 := r.Delete(layer.ID); err2 != nil {
-				// Either a driver error or an error saving.
-				// We now have a layer that's been marked for
-				// deletion but which we failed to remove.
-				logrus.Errorf("While recovering from a failure applying layer diff, error deleting layer %#v: %v", layer.ID, err2)
-			}
+			cleanupFailureContext = "applying layer diff"
 			return nil, -1, err
 		}
 	}
 	delete(layer.Flags, incompleteFlag)
 	err = r.Save()
 	if err != nil {
-		if err2 := r.Delete(layer.ID); err2 != nil {
-			// Either a driver error or an error saving.
-			// We now have a layer that's been marked for
-			// deletion but which we failed to remove.
-			logrus.Errorf("While recovering from a failure applying layer diff, error deleting layer %#v: %v", layer.ID, err2)
-		}
+		cleanupFailureContext = "saving finished layer"
 		return nil, -1, err
 	}
+
 	layer = copyLayer(layer)
+	succeeded = true
 	return layer, size, err
 }