#694060 - Handle image import errors

[Matt Wagner]

This patch fixes #694060, in which attempting to import an EC2 AMI that
didn't exist raised an exception that was not handled. The existing code
was not catching the exception properly, and the exception it was trying
to show contained multiple lines of debugging info. This should clean it
up.

Unfortunately, the test I added isn't able to test the exact failure (it
uses mock, not ec2), but it should still apply.

-- Matt

[PATCH] Graceful handling of errors when importing non-existent image

[Matt Wagner]

---
 .../image_factory/image_imports_controller.rb      |    5 +++--
 src/features/image_import.feature                  |   13 +++++++++++--
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/src/app/controllers/image_factory/image_imports_controller.rb b/src/app/controllers/image_factory/image_imports_controller.rb
index 1d52891..87128af 100644
--- a/src/app/controllers/image_factory/image_imports_controller.rb
+++ b/src/app/controllers/image_factory/image_imports_controller.rb
@@ -11,9 +11,10 @@ class ImageFactory::ImageImportsController < ApplicationController
       flash[:notice]="Image successfully imported"
       redirect_to image_factory_templates_path
       kick_condor
-    rescue => e
+    rescue Exception => e
       init_provider_vars
-      flash.now[:error]=e.message
+      # The full message may be multiple lines, including the actual request, so only include the first line:
+      flash.now[:error]="Could not import image: #{e.message.split("\n").first}"
       Rails.logger.error([e.message,e.backtrace].join("\n  "))
       render :new
     end
diff --git a/src/features/image_import.feature b/src/features/image_import.feature
index 8fc6f7a..56302cf 100644
--- a/src/features/image_import.feature
+++ b/src/features/image_import.feature
@@ -12,15 +12,18 @@ Feature: Import Images
     And There is a mock pulp repository
     When I go to the new image factory image import page
     Then I should be on the new image factory image import page
-    When I select "provider1" from "provider_account_id"
-    And I fill in "ami_id" with "img1"
+

Scenario: Import a new image
+    When I select "provider1" from "provider_account_id"
+    And I fill in "ami_id" with "img1"
     Given I press "Import"
     Then I should see "Image successfully imported"
     And I should be on the image factory templates page

Scenario: Import an already imported image
+    When I select "provider1" from "provider_account_id"
+    And I fill in "ami_id" with "img1"
     Given I press "Import"
     When I go to the new image factory image import page
     When I select "provider1" from "provider_account_id"
@@ -28,3 +31,9 @@ Feature: Import Images
     And I press "Import"
     Then I should see "Image 'img1' is already imported"
     And I should be on the image factory image imports page
+
+  Scenario: Import a non-existent image
+    When I select "provider1" from "provider_account_id"
+    And I fill in "ami_id" with "does-not-exist"
+    Given I press "Import"
+    Then I should see "Could not import image"

[PATCH] Graceful handling of errors when importing non-existent image

[Richard Su]

On 04/25/2011 11:32 AM, Matt Wagner wrote:
> .../image_factory/image_imports_controller.rb      |    5 +++--
>   src/features/image_import.feature                  |   13 +++++++++++--
>   2 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/src/app/controllers/image_factory/image_imports_controller.rb b/src/app/controllers/image_factory/image_imports_controller.rb
> index 1d52891..87128af 100644
> --- a/src/app/controllers/image_factory/image_imports_controller.rb
> +++ b/src/app/controllers/image_factory/image_imports_controller.rb
> @@ -11,9 +11,10 @@ class ImageFactory::ImageImportsController<  ApplicationController
>         flash[:notice]="Image successfully imported"
>         redirect_to image_factory_templates_path
>         kick_condor
> rescue =>  e
> rescue Exception =>  e
>    init_provider_vars
>  flash.now[:error]=e.message
>  # The full message may be multiple lines, including the actual request, so only include the first line:
>  flash.now[:error]="Could not import image: #{e.message.split("\n").first}"
>   Rails.logger.error([e.message,e.backtrace].join("\n  "))
>   render :new
> end
>
> diff --git a/src/features/image_import.feature b/src/features/image_import.feature
> index 8fc6f7a..56302cf 100644
> --- a/src/features/image_import.feature
> +++ b/src/features/image_import.feature
> @@ -12,15 +12,18 @@ Feature: Import Images
>       And There is a mock pulp repository
>       When I go to the new image factory image import page
>       Then I should be on the new image factory image import page
> When I select "provider1" from "provider_account_id"
> And I fill in "ami_id" with "img1"
>
> Scenario: Import a new image
>
> When I select "provider1" from "provider_account_id"
>
> And I fill in "ami_id" with "img1"
>  Given I press "Import"
>  Then I should see "Image successfully imported"
>  And I should be on the image factory templates page
>
> Scenario: Import an already imported image
>
> When I select "provider1" from "provider_account_id"
>
> And I fill in "ami_id" with "img1"
>  Given I press "Import"
>  When I go to the new image factory image import page
>  When I select "provider1" from "provider_account_id"
>
> @@ -28,3 +31,9 @@ Feature: Import Images
>       And I press "Import"
>       Then I should see "Image 'img1' is already imported"
>       And I should be on the image factory image imports page
> Scenario: Import a non-existent image
> When I select "provider1" from "provider_account_id"
> And I fill in "ami_id" with "does-not-exist"
> Given I press "Import"
> Then I should see "Could not import image"

Ack.