Begin moving from error_and_exit to exceptions. Fail hard!

bin/vagrant

@@ -10,5 +10,6 @@ rescue Vagrant::Errors::VagrantError => e
   opts = { :_translate => false, :_prefix => false }
   env.ui.error e.message, opts
   env.ui.error e.backtrace.join("\n"), opts if ENV["VAGRANT_DEBUG"]
-  exit e.status_code
+  exit e.status_code if e.respond_to?(:status_code)
+  exit 999 # An error occurred with no status code defined
 end

lib/vagrant/action/box/download.rb

@@ -20,8 +20,6 @@ module Vagrant
           @env = env
 
           download if instantiate_downloader
-          return if env.error?
-
           @app.call(@env)
 
           recover(env) # called in both cases to cleanup workspace
@@ -35,10 +33,7 @@ module Vagrant
             end
           end
 
-          if !@downloader
+          raise Errors::BoxDownloadUnknownType.new if !@downloader
-            @env.error!(:box_download_unknown_type)
-            return false
-          end
 
           @downloader.prepare(@env["box"].uri)
           true

lib/vagrant/action/warden.rb

@@ -1,5 +1,11 @@
 module Vagrant
   class Action
+    # The action warden is a middleware which injects itself between
+    # every other middleware, watching for exceptions which are raised
+    # and performing proper cleanup on every action by calling the {#recover}
+    # method. The warden therefore allows middlewares to not worry about
+    # exceptional events, and by providing a simple callback, can clean up
+    # in any erroneous case.
     class Warden
       include Util
       attr_accessor :actions, :stack
@@ -12,11 +18,16 @@ module Vagrant
       def call(env)
         return if @actions.empty?
 
-        # If the previous action passes and environment error on
+        begin
-        @stack.unshift(@actions.shift).first.call(env) unless env.error?
+          # Call the next middleware in the sequence, appending to the stack
-
+          # of "recoverable" middlewares in case something goes wrong!
-        # if the call action returned prematurely with an error
+          @stack.unshift(@actions.shift).first.call(env)
-        begin_rescue(env) if env.error?
+        rescue
+          # Something went horribly wrong. Start the rescue chain then
+          # reraise the exception to properly kick us out of limbo here.
+          begin_rescue(env)
+          raise
+        end
       end
 
       def begin_rescue(env)
@@ -25,9 +36,6 @@ module Vagrant
         end
 
         exit if env.interrupted?
-
-        # Erroneous environment resulted. Properly display error message.
-        error_and_exit(*env.error)
       end
 
       def finalize_action(action, env)

lib/vagrant/errors.rb

@@ -1,3 +1,6 @@
+# This file contains all of the internal errors in Vagrant's core
+# commands, actions, etc.
+
 module Vagrant
   module Errors
     # Main superclass of any errors in Vagrant. This provides some
@@ -6,12 +9,15 @@ module Vagrant
     # error code, and the error key is used as a default message from
     # I18n.
     class VagrantError < StandardError
+      DEFAULT_NAMESPACE = "vagrant.errors"
+
       def self.status_code(code = nil)
         define_method(:status_code) { code }
       end
 
-      def self.error_key(key=nil)
+      def self.error_key(key=nil, namespace=nil)
         define_method(:error_key) { key }
+        define_method(:error_namespace) { namespace } if namespace
       end
 
       def initialize(message=nil, *args)
@@ -22,7 +28,8 @@ module Vagrant
       protected
 
       def translate_error(key, opts=nil)
-        I18n.t("vagrant.errors.#{key}", opts)
+        namespace = respond_to?(:error_namespace) ? error_namespace : DEFAULT_NAMESPACE
+        I18n.t("#{namespace}.#{key}", opts)
       end
     end
 
@@ -31,6 +38,11 @@ module Vagrant
       error_key(:base_vm_not_found)
     end
 
+    class BoxDownloadUnknownType < VagrantError
+      status_code(13)
+      error_key(:unknown_type, "vagrant.actions.box.download")
+    end
+
     class BoxNotFound < VagrantError
       status_code(2)
       error_key(:box_not_found)

templates/locales/en.yml

@@ -173,15 +173,16 @@ en:
 
       box:
         destroy:
-          destroying: Deleting box '%{name}'...
+          destroying: "Deleting box '%{name}'..."
         download:
-          with: Downloading with %{class}...
+          with: "Downloading with %{class}..."
-          cleaning: Cleaning up downloaded box...
+          cleaning: "Cleaning up downloaded box..."
-          copying: Copying box to temporary location...
+          copying: "Copying box to temporary location..."
+          unknown_type: "Unknown or unsupported URI type given for box download."
         unpackage:
-          extracting: Extracting box...
+          extracting: "Extracting box..."
         verify:
-          verifying: Verifying box...
+          verifying: "Verifying box..."
 
       general:
         package:

templates/strings.yml

@@ -10,8 +10,6 @@
 :box_already_exists: |-
   This box appears to already exist! Please call `vagrant box remove <%= box_name %>`
   and then try to add it again.
-:box_download_unknown_type: |-
-  Unknown URI type for box download.
 :box_download_http_socket_error: |-
   An error occurred while trying to download the specified box. This most
   often happens if there is no internet connection or the address is

test/vagrant/action/box/download_test.rb

@@ -35,15 +35,6 @@ class DownloadBoxActionTest < Test::Unit::TestCase
         @instance.expects(:recover).with(@env).in_sequence(seq)
         @instance.call(@env)
       end
-
-      should "halt the chain if downloader instantiation fails" do
-        seq = sequence("seq")
-        @env.error!(:foo)
-        @instance.expects(:instantiate_downloader).in_sequence(seq).returns(false)
-        @instance.expects(:download).never
-        @app.expects(:call).with(@env).never
-        @instance.call(@env)
-      end
     end
 
     context "instantiating downloader" do
@@ -56,9 +47,9 @@ class DownloadBoxActionTest < Test::Unit::TestCase
 
       should "error environment if URI is invalid for any downloaders" do
         @env["box"].uri = "foobar"
-        assert !@instance.instantiate_downloader
+        assert_raises(Vagrant::Errors::BoxDownloadUnknownType) {
-        assert @env.error?
+          @instance.instantiate_downloader
-        assert_equal :box_download_unknown_type, @env.error.first
+        }
       end
     end
 

test/vagrant/action/warden_test.rb

@@ -55,32 +55,17 @@ class ActionWardenTest < Test::Unit::TestCase
       @instance.call(new_env)
     end
 
-    should "begin recover on environment error" do
+    should "begin recovery sequence when the called action raises an exception" do
-      @instance.expects(:begin_rescue)
-      @instance.actions << lambda {}
-      @instance.actions.first.expects(:call).never
-      @instance.call(new_env_with_error)
-    end
-
-    should "not call the next action on env err" do
-      action = mock('action')
-      action.expects(:call).never
-      @instance.actions << action
-      @instance.expects(:begin_rescue)
-      @instance.call(new_env_with_error)
-    end
-
-    should "call begin recover when the called action returns with an env error" do
       class Foo
         def initialize(*args); end
         def call(env)
-          return env.error!(:foo)
+          raise "An exception"
         end
       end
 
       @instance.actions << Foo.new
       @instance.expects(:begin_rescue)
-      @instance.call(new_env)
+      assert_raises(RuntimeError) { @instance.call(new_env) }
     end
 
     def new_env_with_error