Re: "unicorn -D" always returns 0 "success" (even when failed to load)

[Eric Wong <normalperson@yhbt.net>]

Iñaki Baz Castillo <ibc@aliax.net> wrote:
> El Lunes, 28 de Diciembre de 2009, Eric Wong escribió:
> > > It's really awesome! I've tested it and it works great, and avoids the 
> > > sleeping stuff and a new commandlline option. Congratulations :)
> > 
> > The current version is actually slightly buggy in that it leaks
> > the pipe descriptor...
> 
> I've detected some other "issue":
> 
> Imagine port 80 is used by other application (as apache) and Unicorn is 
> configured to also bind in same port.
> 
> 
> When running it in foreground all is great:
> 
>   ERROR -- : adding listener failed addr=0.0.0.0:80 (in use)
>   ERROR -- : retrying in 3 seconds (1 tries left)
>   ERROR -- : adding listener failed addr=0.0.0.0:80 (in use)
>   ERROR -- : retrying in 3 seconds (0 tries left)
>   ERROR -- : adding listener failed addr=0.0.0.0:80 (in use)
>   /usr/local/lib/ruby1.9/site_ruby/1.9.1/unicorn/socket_helper.rb:110:in  
>   `initialize': Address already in use - bind(2) (Errno::EADDRINUSE)
> 
> 
> But when running in background an ugly error is displayed:
> 
>   /usr/local/lib/ruby1.9/site_ruby/1.9.1/unicorn/launcher.rb:45:in `sysread':
>   end of file reached (EOFError)
>     from /usr/local/lib/ruby1.9/site_ruby/1.9.1/unicorn/launcher.rb:45:in
>     `daemonize!'
> 
> 
> In both cases $? is 1 (error) but in the last case the error message is not 
> very useful.
> 
> I suspect what is happening: the master tries several times to bind and after 
> N retries it terminates (so it closes the pipe and grandparent gets EOFError).
> 
> As a suggestion, could the grandparent rescue such exception and display some 
> kind of error message?:
> 
>   "The master couldn't be started. Inspect the log error or run in foreground"

Thanks, pushed out to the ready_pipe branch of unicorn.git

>From 52eee4e424198a3c80793ee9c930fd3bb0285168 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Mon, 28 Dec 2009 11:16:00 -0800
Subject: [PATCH] launcher: descriptive error message on startup failure
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Rather than erroring out with a non-descript EOFError,
show a warning message telling users to check the logs
instead.

Reported-by: Iñaki Baz Castillo mid=200912281350.44760.ibc@aliax.net
---
 lib/unicorn/launcher.rb |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/unicorn/launcher.rb b/lib/unicorn/launcher.rb
index 0ea836b..1871420 100644
--- a/lib/unicorn/launcher.rb
+++ b/lib/unicorn/launcher.rb
@@ -42,8 +42,11 @@ class Unicorn::Launcher
 
       if grandparent == $$
         # this will block until HttpServer#join runs (or it dies)
-        master_pid = rd.readpartial(16).to_i
-        exit!(1) unless master_pid > 1
+        master_pid = (rd.readpartial(16) rescue nil).to_i
+        unless master_pid > 1
+          warn "master failed to start, check stderr log for details"
+          exit!(1)
+        end
         exit 0
       else # unicorn master process
         options[:ready_pipe] = wr
-- 
Eric Wong
_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying