@@ -138,7 +138,12 @@ class AbstractServer # on a white list of file descriptors. That proved to be way too fragile: # too many file descriptors are being left open even though they shouldn't # be. So now we close file descriptors based on a black list. - file_descriptors_to_leave_open = [0, 1, 2, @child_socket.fileno] + # + # Note that STDIN, STDOUT and STDERR may be temporarily set to + # different file descriptors than 0, 1 and 2, e.g. in unit tests. + # We don't want to close these either. + file_descriptors_to_leave_open = [0, 1, 2, @child_socket.fileno, + fileno_of(STDIN), fileno_of(STDOUT), fileno_of(STDERR)].compact.uniq NativeSupport.close_all_file_descriptors(file_descriptors_to_leave_open) # In addition to closing the file descriptors, one must also close # the associated IO objects. This is to prevent IO.close from @@ -295,6 +300,12 @@ protected def quit_main @done = true end + + def fileno_of(io) + return io.fileno + rescue + return nil + end private # Reset all signal handlers to default. This is called in the child process, lib/phusion_passenger/abstract_server.rb @@ -72,7 +72,7 @@ class ApplicationSpawner Process.waitpid(pid) rescue nil channel = MessageChannel.new(a) - unmarshal_and_raise_errors(channel, "rack") + unmarshal_and_raise_errors(channel, !!options["print_exceptions"], "rack") # No exception was raised, so spawning succeeded. pid, socket_name, socket_type = channel.read lib/phusion_passenger/rack/application_spawner.rb @@ -94,6 +94,10 @@ class ApplicationSpawner < AbstractServer # string if the application is deployed on the root URI. It must not # equal the empty string. # + # - +print_exceptions+: + # Whether exceptions that have occurred during application initialization + # should be printed to STDERR. The default is true. + # # All other options will be passed on to RequestHandler. def initialize(app_root, options = {}) super() @@ -105,6 +109,7 @@ class ApplicationSpawner < AbstractServer @environment = @options["environment"] @encoded_environment_variables = @options["environment_variables"] @base_uri = @options["base_uri"] if @options["base_uri"] && @options["base_uri"] != "/" + @print_exceptions = @options["print_exceptions"] self.max_idle_time = DEFAULT_APP_SPAWNER_MAX_IDLE_TIME assert_valid_app_root(@app_root) define_message_handler(:spawn_application, :handle_spawn_application) @@ -186,7 +191,7 @@ class ApplicationSpawner < AbstractServer Process.waitpid(pid) rescue nil channel = MessageChannel.new(a) - unmarshal_and_raise_errors(channel) + unmarshal_and_raise_errors(channel, @print_exceptions) # No exception was raised, so spawning succeeded. pid, socket_name, socket_type = channel.read @@ -207,7 +212,7 @@ class ApplicationSpawner < AbstractServer def start super begin - unmarshal_and_raise_errors(server) + unmarshal_and_raise_errors(server, @print_exceptions) rescue IOError, SystemCallError, SocketError => e stop raise Error, "The application spawner server exited unexpectedly: #{e}" lib/phusion_passenger/railz/application_spawner.rb @@ -58,6 +58,9 @@ class FrameworkSpawner < AbstractServer # this version is actually installed. # - <tt>:vendor</tt>: The directory to the vendor Rails framework to use. This is # usually something like "/webapps/foo/vendor/rails". + # - <tt>:print_framework_loading_exceptions</tt>: + # Whether exceptions that have occurred while loading the Ruby on Rails framework + # should be printed to STDERR. The default is true. # # It is not allowed to specify both +version+ and +vendor+. # @@ -72,6 +75,11 @@ class FrameworkSpawner < AbstractServer end @version = options[:version] @vendor = options[:vendor] + if options.has_key?(:print_framework_loading_exceptions) + @print_framework_loading_exceptions = options[:print_framework_loading_exceptions] + else + @print_framework_loading_exceptions = true + end if !@version && !@vendor raise ArgumentError, "Either the 'version' or the 'vendor' option must specified" elsif @version && @vendor @@ -87,7 +95,7 @@ class FrameworkSpawner < AbstractServer # Overrided from AbstractServer#start. # # May raise these additional exceptions: - # - FrameworkInitError: The specified Ruby on Rails framework could not be loaded. + # - FrameworkInitError: An error occurred while loading the specified Ruby on Rails framework. # - FrameworkSpawner::Error: The FrameworkSpawner server exited unexpectedly. def start super @@ -109,6 +117,9 @@ class FrameworkSpawner < AbstractServer "#{child_exception.class} (#{child_exception.message})" end options = { :vendor => @vendor, :version => @version } + if @print_framework_loading_exceptions + print_exception(self.class.to_s, child_exception) + end raise FrameworkInitError.new(message, child_exception, options) end rescue IOError, SystemCallError, SocketError @@ -122,23 +133,7 @@ class FrameworkSpawner < AbstractServer # When successful, an Application object will be returned, which represents # the spawned RoR application. # - # The following options are allowed: - # - +lower_privilege+ and +lowest_user+: - # If +lower_privilege+ is true, then ApplicationSpawner will attempt to - # switch to the user who owns the application's <tt>config/environment.rb</tt>, - # and to the default group of that user. - # - # If that user doesn't exist on the system, or if that user is root, - # then ApplicationSpawner will attempt to switch to the username given by - # +lowest_user+ (and to the default group of that user). - # If +lowest_user+ doesn't exist either, or if switching user failed - # (because the current process does not have the privilege to do so), - # then ApplicationSpawner will continue without reporting an error. - # - # - +environment+: - # Allows one to specify the RAILS_ENV environment to use. - # - # All other options will be passed on to ApplicationSpawner and RequestHandler. + # All options accepted by ApplicationSpawner.new and RequestHandler.new are accepted. # # FrameworkSpawner will internally cache the code of applications, in order to # speed up future spawning attempts. This implies that, if you've changed @@ -148,7 +143,7 @@ class FrameworkSpawner < AbstractServer # # Raises: # - AbstractServer::ServerNotStarted: The FrameworkSpawner server hasn't already been started. - # - InvalidAppRoot: +app_root+ doesn't appear to be a valid Ruby on Rails application root. + # - InvalidPath: +app_root+ doesn't appear to be a valid Ruby on Rails application root. # - AppInitError: The application raised an exception or called exit() during startup. # - ApplicationSpawner::Error: The ApplicationSpawner server exited unexpectedly. # - FrameworkSpawner::Error: The FrameworkSpawner server exited unexpectedly. @@ -156,8 +151,12 @@ class FrameworkSpawner < AbstractServer assert_valid_app_root(app_root) options = sanitize_spawn_options(options) options["app_root"] = app_root + # No need for the ApplicationSpawner to print exceptions. All + # exceptions raised by the ApplicationSpawner are sent back here, + # so we just need to decide here whether we want to print it. + print_exceptions = options["print_exceptions"] + options["print_exceptions"] = false - exception_to_propagate = nil begin server.write("spawn_application", *options.to_a.flatten) result = server.read @@ -166,8 +165,10 @@ class FrameworkSpawner < AbstractServer end if result[0] == 'exception' e = unmarshal_exception(server.read_scalar) - if e.respond_to?(:child_exception) && e.child_exception - #print_exception(self.class.to_s, e.child_exception) + if print_exceptions && e.respond_to?(:child_exception) && e.child_exception + print_exception(self.class.to_s, e.child_exception) + elsif print_exceptions + print_exception(self.class.to_s, e) end raise e else @@ -276,10 +277,7 @@ private end def handle_spawn_application(*options) - options = Hash[*options] - options["lower_privilege"] = options["lower_privilege"] == "true" - options["app_spawner_timeout"] = options["app_spawner_timeout"].to_i - options["memory_limit"] = options["memory_limit"].to_i + options = sanitize_spawn_options(Hash[*options]) app = nil app_root = options["app_root"] @@ -293,10 +291,10 @@ private spawner.start spawner end - rescue ArgumentError, AppInitError, ApplicationSpawner::Error => e + rescue InvalidPath, AppInitError, ApplicationSpawner::Error => e client.write('exception') client.write_scalar(marshal_exception(e)) - if e.child_exception.is_a?(LoadError) + if e.respond_to?(:child_exception) && e.child_exception.is_a?(LoadError) # A source file failed to load, maybe because of a # missing gem. If that's the case then the sysadmin # will install probably the gem. So we clear RubyGems's lib/phusion_passenger/railz/framework_spawner.rb @@ -97,7 +97,7 @@ class SpawnManager < AbstractServer # # Other options are: # - # ['lower_privilege', 'lowest_user', 'environment', 'environment_variables' and 'base_uri'] + # ['lower_privilege', 'lowest_user', 'environment', 'environment_variables', 'base_uri' and 'print_exceptions'] # See Railz::ApplicationSpawner.new for an explanation of these options. # # ['app_type'] @@ -234,7 +234,11 @@ private else key = "version:#{framework_version}" create_spawner = proc do - Railz::FrameworkSpawner.new(:version => framework_version) + framework_options = { :version => framework_version } + if options.has_key?(:print_framework_loading_exceptions) + framework_options[:print_framework_loading_exceptions] = options[:print_framework_loading_exceptions] + end + Railz::FrameworkSpawner.new(framework_options) end spawner_timeout = options["framework_spawner_timeout"] end lib/phusion_passenger/spawn_manager.rb @@ -154,10 +154,10 @@ protected # # +current_location+ is a string which describes where the code is # currently at. Usually the current class name will be enough. - def print_exception(current_location, exception) + def print_exception(current_location, exception, destination = STDERR) if !exception.is_a?(SystemExit) - STDERR.puts(exception.backtrace_string(current_location)) - STDERR.flush + destination.puts(exception.backtrace_string(current_location)) + destination.flush if destination.respond_to?(:flush) end end @@ -264,13 +264,24 @@ protected # received information, then an appropriate exception will be # raised. # + # If <tt>print_exception</tt> evaluates to true, then the + # exception message and the backtrace will also be printed. + # Where it is printed to depends on the type of + # <tt>print_exception</tt>: + # - If it responds to #puts, then the exception information will + # be printed using this method. + # - If it responds to #to_str, then the exception information + # will be appended to the file whose filename equals the return + # value of the #to_str call. + # - Otherwise, it will be printed to STDERR. + # # Raises: # - AppInitError: this class wraps the exception information # received through the channel. # - IOError, SystemCallError, SocketError: these errors are # raised if an error occurred while receiving the information # through the channel. - def unmarshal_and_raise_errors(channel, app_type = "rails") + def unmarshal_and_raise_errors(channel, print_exception = nil, app_type = "rails") args = channel.read if args.nil? raise EOFError, "Unexpected end-of-file detected." @@ -279,8 +290,7 @@ protected if status == 'exception' child_exception = unmarshal_exception(channel.read_scalar) stderr = channel.read_scalar - #print_exception(self.class.to_s, child_exception) - raise AppInitError.new( + exception = AppInitError.new( "Application '#{@app_root}' raised an exception: " << "#{child_exception.class} (#{child_exception.message})", child_exception, @@ -289,9 +299,25 @@ protected elsif status == 'exit' child_exception = unmarshal_exception(channel.read_scalar) stderr = channel.read_scalar - raise AppInitError.new("Application '#{@app_root}' exited during startup", + exception = AppInitError.new("Application '#{@app_root}' exited during startup", child_exception, app_type, stderr.empty? ? nil : stderr) + else + exception = nil + end + + if print_exception && exception + if print_exception.respond_to?(:puts) + print_exception(self.class.to_s, child_exception, print_exception) + elsif print_exception.respond_to?(:to_str) + filename = print_exception.to_str + File.open(filename, 'a') do |f| + print_exception(self.class.to_s, child_exception, f) + end + else + print_exception(self.class.to_s, child_exception) + end end + raise exception if exception end # Lower the current process's privilege to the owner of the given file. @@ -340,6 +366,10 @@ protected end end + def to_boolean(value) + return !(value.nil? || value == false || value == "false") + end + def sanitize_spawn_options(options) defaults = { "lower_privilege" => true, @@ -348,12 +378,15 @@ protected "app_type" => "rails", "spawn_method" => "smart-lv2", "framework_spawner_timeout" => -1, - "app_spawner_timeout" => -1 + "app_spawner_timeout" => -1, + "print_exceptions" => true } options = defaults.merge(options) - options["lower_privilege"] = options["lower_privilege"].to_s == "true" + options["lower_privilege"] = to_boolean(options["lower_privilege"]) options["framework_spawner_timeout"] = options["framework_spawner_timeout"].to_i options["app_spawner_timeout"] = options["app_spawner_timeout"].to_i + # Force this to be a boolean for easy use with Utils#unmarshal_and_raise_errors. + options["print_exceptions"] = to_boolean(options["print_exceptions"]) return options end lib/phusion_passenger/utils.rb @@ -19,7 +19,7 @@ describe PhusionPassenger::Rack::ApplicationSpawner do it "propagates exceptions in application startup" do File.prepend("#{@stub.app_root}/config.ru", "raise StandardError, 'foo'\n") - spawn = lambda { spawn(@stub.app_root) } + spawn = lambda { spawn(@stub.app_root, "print_exceptions" => false) } spawn.should raise_error(StandardError) end test/ruby/rack/application_spawner_spec.rb @@ -97,8 +97,8 @@ describe FrameworkSpawner do end end - def load_nonexistant_framework - spawner = FrameworkSpawner.new(:version => "1.9.827") + def load_nonexistant_framework(options = {}) + spawner = FrameworkSpawner.new(options.merge(:version => "1.9.827")) begin spawner.start ensure test/ruby/rails/framework_spawner_spec.rb @@ -1,3 +1,5 @@ +require 'stringio' + shared_examples_for "handling errors in application initialization" do before :each do @stub = setup_rails_stub('foobar') @@ -11,8 +13,8 @@ shared_examples_for "handling errors in application initialization" do it "raises an AppInitError if the spawned app raises a standard exception during startup" do File.prepend(@stub.environment_rb, "raise 'This is a dummy exception.'\n") begin - spawn_stub_application(@stub).close - violated "Spawning the application should have raised an InitializationError." + spawn_stub_application(@stub, "print_exceptions" => false).close + violated "Spawning the application should have raised an AppInitError." rescue AppInitError => e e.child_exception.message.should == "This is a dummy exception." end @@ -26,8 +28,8 @@ shared_examples_for "handling errors in application initialization" do raise MyError, "This is a custom exception." }) begin - spawn_stub_application(@stub).close - violated "Spawning the application should have raised an InitializationError." + spawn_stub_application(@stub, "print_exceptions" => false).close + violated "Spawning the application should have raised an AppInitError." rescue AppInitError => e e.child_exception.message.should == "This is a custom exception. (MyError)" end @@ -36,17 +38,70 @@ shared_examples_for "handling errors in application initialization" do it "raises an AppInitError if the spawned app calls exit() during startup" do File.prepend(@stub.environment_rb, "exit\n") begin - spawn_stub_application(@stub).close - violated "Spawning the application should have raised an InitializationError." + spawn_stub_application(@stub, "print_exceptions" => false).close + violated "Spawning the application should have raised an AppInitError." rescue AppInitError => e e.child_exception.class.should == SystemExit end end + + it "prints the exception to STDERR if the spawned app raised an error" do + old_stderr = STDERR + file = File.new('output.tmp', 'w+') + begin + Object.send(:remove_const, "STDERR") rescue nil + Object.const_set("STDERR", file) + + File.prepend(@stub.environment_rb, "raise 'This is a dummy exception.'\n") + block = lambda do + spawn_stub_application(@stub).close + end + block.should raise_error(AppInitError) + + file.rewind + data = file.read + data.should =~ /spawn_stub_application/ + data.should =~ /spawner_error_handling_spec\.rb/ + ensure + Object.send(:remove_const, "STDERR") rescue nil + Object.const_set("STDERR", old_stderr) + file.close rescue nil + File.unlink('output.tmp') rescue nil + end + end end shared_examples_for "handling errors in framework initialization" do include Utils + it "raises FrameworkInitError if the framework could not be loaded" do - lambda { load_nonexistant_framework.close }.should raise_error(FrameworkInitError) + block = lambda do + load_nonexistant_framework(:print_framework_loading_exceptions => false).close + end + block.should raise_error(FrameworkInitError) + end + + it "prints the exception to STDERR if the framework could not be loaded" do + old_stderr = STDERR + file = File.new('output.tmp', 'w+') + begin + Object.send(:remove_const, "STDERR") rescue nil + Object.const_set("STDERR", file) + + block = lambda do + load_nonexistant_framework.close + end + block.should raise_error(FrameworkInitError) + + file.rewind + data = file.read + data.should =~ /load_nonexistant_framework/ + data.should =~ /spawner_error_handling_spec\.rb/ + ensure + Object.send(:remove_const, "STDERR") rescue nil + Object.const_set("STDERR", old_stderr) + file.close rescue nil + File.unlink('output.tmp') rescue nil + end end end test/ruby/rails/spawner_error_handling_spec.rb @@ -1,5 +1,6 @@ require 'support/config' require 'support/test_helper' +require 'phusion_passenger/application' require 'phusion_passenger/spawn_manager' require 'ruby/abstract_server_spec' @@ -183,7 +184,7 @@ describe SpawnManager do end end - def load_nonexistant_framework + def load_nonexistant_framework(extra_options = {}) # Prevent detect_framework_version from raising VersionNotFound Application.instance_eval do alias orig_detect_framework_version detect_framework_version @@ -194,7 +195,7 @@ describe SpawnManager do begin File.write(@stub.environment_rb, "RAILS_GEM_VERSION = '1.9.827'") @stub.dont_use_vendor_rails - return spawn_stub_application(@stub) + return spawn_stub_application(@stub, extra_options) ensure Application.instance_eval do alias detect_framework_version orig_detect_framework_version test/ruby/spawn_manager_spec.rb @@ -164,6 +164,53 @@ describe Utils do end end + describe "#unmarshal_and_raise_errors" do + before :each do + @a, @b = IO.pipe + @report_channel = MessageChannel.new(@a) + report_app_init_status(MessageChannel.new(@b)) do + raise StandardError, "Something went wrong!" + end + end + + after :each do + @a.close rescue nil + @b.close rescue nil + end + + it "prints the exception information to the 'print_exception' argument using #puts, if 'print_exception' responds to that" do + buffer = StringIO.new + lambda { unmarshal_and_raise_errors(@report_channel, buffer) }.should raise_error(AppInitError) + buffer.string.should =~ /Something went wrong!/ + buffer.string.should =~ /utils\.rb/ + buffer.string.should =~ /utils_spec\.rb/ + end + + it "appends the exception information to the file pointed to by 'print_exception', if 'print_exception' responds to #to_str" do + begin + lambda { unmarshal_and_raise_errors(@report_channel, "exception.txt") }.should raise_error(AppInitError) + data = File.read('exception.txt') + data.should =~ /Something went wrong!/ + data.should =~ /utils\.rb/ + data.should =~ /utils_spec\.rb/ + ensure + File.unlink('exception.txt') rescue nil + end + end + end + + specify "#to_boolean works" do + to_boolean(nil).should be_false + to_boolean(false).should be_false + to_boolean(true).should be_true + to_boolean(1).should be_true + to_boolean(0).should be_true + to_boolean("").should be_true + to_boolean("true").should be_true + to_boolean("false").should be_false + to_boolean("bla bla").should be_true + end + describe "#passenger_tmpdir" do before :each do @old_passenger_tmpdir = Utils.passenger_tmpdir test/ruby/utils_spec.rb 237c6e04dd6518cfa86b2204fa43e6d0c14bcd82 Hongli Lai (Phusion) hongli@phusion.nl http://github.com/FooBarWidget/passenger/commit/ac4a005fbb860cddecab46e38d79de8e8c1677d5 ac4a005fbb860cddecab46e38d79de8e8c1677d5 2009-06-04T13:53:17-07:00 2009-05-29T07:50:15-07:00 When an application fails to start, log the exception it to STDERR (which is usually the web server's log file). Fixes issue #210. 4f604c6a54f1219feeb5e12c59ddb27663664830 Hongli Lai (Phusion) hongli@phusion.nl