From f32aabe823274b0ce87020a0e2a8fbd3b1aa80a9 Mon Sep 17 00:00:00 2001
From: Christopher Horrell <christopher@horrell.ca>
Date: Tue, 10 Feb 2026 17:46:50 -0500
Subject: [PATCH] Add comprehensive error handling to image lifecycle

Improvements to create_image:
- Catch and report Docker build failures with clear error messages
- Re-raise exception to fail tests appropriately

Improvements to delete_image:
- Guard against nil @image (if build failed)
- Wrap container cleanup in error handling to continue if issues occur
- Catch individual container cleanup failures and continue
- Handle NotFoundError if image already removed
- Always attempt image removal even if container cleanup fails
- Add informative status messages for debugging

Benefits:
- Tests fail gracefully with clear error messages
- Cleanup attempts even after failures (prevents resource leaks)
- Better visibility into what's happening during cleanup
- Prevents cascading failures from cleanup issues

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
---
 spec/spec_helper.rb | 51 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 37 insertions(+), 14 deletions(-)

diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index e9f4514..ccb7016 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -1,7 +1,13 @@
 module Helpers
   def create_image(version)
     puts "Building image..."
-    @image = Docker::Image.build_from_dir("#{version}/")
+    
+    begin
+      @image = Docker::Image.build_from_dir("#{version}/")
+    rescue Docker::Error::DockerError => e
+      puts "Failed to build Docker image: #{e.message}"
+      raise
+    end
 
     set :os, :family => 'debian'
     set :backend, :docker
@@ -11,25 +17,42 @@ def create_image(version)
   end
   
   def delete_image
+    return unless @image
+    
     puts "Deleting image..."
+    
     # Stop and remove only containers created from this image
-    Docker::Container.all(:all => true).each do |container|
-      container_image = container.info['Image']
-      container_image_id = container.info['ImageID']
-      
-      # Match image IDs - container IDs may have sha256: prefix and be full hashes
-      # while @image.id is the short ID
-      if container_image&.include?(@image.id) || container_image_id&.include?(@image.id)
-        begin
-          container.stop unless container.info['State'] == 'exited'
-          container.delete(:force => true)
-        rescue Docker::Error::NotModifiedError
-          # Container already stopped, ignore
+    begin
+      Docker::Container.all(:all => true).each do |container|
+        container_image = container.info['Image']
+        container_image_id = container.info['ImageID']
+        
+        # Match image IDs - container IDs may have sha256: prefix and be full hashes
+        # while @image.id is the short ID
+        if container_image&.include?(@image.id) || container_image_id&.include?(@image.id)
+          begin
+            container.stop unless container.info['State'] == 'exited'
+            container.delete(:force => true)
+          rescue Docker::Error::NotModifiedError
+            # Container already stopped, ignore
+          rescue Docker::Error::DockerError => e
+            puts "Warning: Failed to cleanup container #{container.id[0..11]}: #{e.message}"
+          end
         end
       end
+    rescue Docker::Error::DockerError => e
+      puts "Warning: Error during container cleanup: #{e.message}"
     end
     
-    @image.remove(:force => true)
+    # Always attempt to remove the image, even if container cleanup failed
+    begin
+      @image.remove(:force => true)
+      puts "Image removed successfully"
+    rescue Docker::Error::NotFoundError
+      puts "Image already removed"
+    rescue Docker::Error::DockerError => e
+      puts "Warning: Failed to remove image: #{e.message}"
+    end
   end
 end