diff --git a/bin/vagrant b/bin/vagrant index 435f7e051..81501850d 100755 --- a/bin/vagrant +++ b/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 diff --git a/lib/vagrant/action/box/download.rb b/lib/vagrant/action/box/download.rb index b31a111c9..e9299e9d5 100644 --- a/lib/vagrant/action/box/download.rb +++ b/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 - @env.error!(:box_download_unknown_type) - return false - end + raise Errors::BoxDownloadUnknownType.new if !@downloader @downloader.prepare(@env["box"].uri) true diff --git a/lib/vagrant/action/warden.rb b/lib/vagrant/action/warden.rb index b1c69cc12..35b6943dd 100644 --- a/lib/vagrant/action/warden.rb +++ b/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 - @stack.unshift(@actions.shift).first.call(env) unless env.error? - - # if the call action returned prematurely with an error - begin_rescue(env) if env.error? + begin + # Call the next middleware in the sequence, appending to the stack + # of "recoverable" middlewares in case something goes wrong! + @stack.unshift(@actions.shift).first.call(env) + 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) diff --git a/lib/vagrant/errors.rb b/lib/vagrant/errors.rb index 6030a405c..d52473675 100644 --- a/lib/vagrant/errors.rb +++ b/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) diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 353236d10..e09441fb8 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -173,15 +173,16 @@ en: box: destroy: - destroying: Deleting box '%{name}'... + destroying: "Deleting box '%{name}'..." download: - with: Downloading with %{class}... - cleaning: Cleaning up downloaded box... - copying: Copying box to temporary location... + with: "Downloading with %{class}..." + cleaning: "Cleaning up downloaded box..." + 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: diff --git a/templates/strings.yml b/templates/strings.yml index 928130e11..7f06cf521 100644 --- a/templates/strings.yml +++ b/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 diff --git a/test/vagrant/action/box/download_test.rb b/test/vagrant/action/box/download_test.rb index 1b596a70b..404ce85c4 100644 --- a/test/vagrant/action/box/download_test.rb +++ b/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 @env.error? - assert_equal :box_download_unknown_type, @env.error.first + assert_raises(Vagrant::Errors::BoxDownloadUnknownType) { + @instance.instantiate_downloader + } end end diff --git a/test/vagrant/action/warden_test.rb b/test/vagrant/action/warden_test.rb index c4b81d6ba..6b8f5abfd 100644 --- a/test/vagrant/action/warden_test.rb +++ b/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 - @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 + should "begin recovery sequence when the called action raises an exception" 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