cleanup and refactor error handling

Make sure app errors get logged correctly, and we no longer
return a 500 response when a client EOFs the write end (but not
the read end) of a connection.

4 files changed, 93 insertions, 2 deletions

diff --git a/t/t0000-simple-http.sh b/t/t0000-simple-http.sh
index 173281b..6f4d738 100755
--- a/t/t0000-simple-http.sh
+++ b/t/t0000-simple-http.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 . ./test-lib.sh
-t_plan 24 "simple HTTP connection keepalive/pipelining tests for $model"
+t_plan 25 "simple HTTP connection keepalive/pipelining tests for $model"
 
 t_begin "checking for config.ru for $model" && {
         tbase=simple-http_$model.ru
@@ -21,6 +21,17 @@ t_begin "single request" && {
         curl -sSfv http://$listen/
 }
 
+t_begin "handles client EOF gracefully" && {
+        printf 'GET / HTTP/1.1\r\nHost: example.com\r\n\r\n' | \
+                socat - TCP4:$listen > $tmp
+        dbgcat tmp
+        if grep 'HTTP.* 500' $tmp
+        then
+                die "500 error returned on client shutdown(SHUT_WR)"
+        fi
+        check_stderr
+}
+
 dbgcat r_err
 
 t_begin "two requests with keepalive" && {
diff --git a/t/t0001-unix-http.sh b/t/t0001-unix-http.sh
index 59f996a..e9bc919 100755
--- a/t/t0001-unix-http.sh
+++ b/t/t0001-unix-http.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 . ./test-lib.sh
-t_plan 18 "simple HTTP connection keepalive/pipelining tests for $model"
+t_plan 19 "simple HTTP connection keepalive/pipelining tests for $model"
 
 t_begin "checking for config.ru for $model" && {
         tbase=simple-http_$model.ru
@@ -23,6 +23,17 @@ t_begin "single TCP request" && {
         curl -sSfv http://$listen/
 }
 
+t_begin "handles client EOF gracefully" && {
+        printf 'GET / HTTP/1.1\r\nHost: example.com\r\n\r\n' | \
+                socat - UNIX:$unix_socket > $tmp
+        dbgcat tmp
+        if grep 'HTTP.* 500' $tmp
+        then
+                die "500 error returned on client shutdown(SHUT_WR)"
+        fi
+        check_stderr
+}
+
 dbgcat r_err
 
 t_begin "pipelining partial requests" && {
diff --git a/t/t0009-broken-app.sh b/t/t0009-broken-app.sh
new file mode 100644
index 0000000..b5aae4e
--- /dev/null
+++ b/t/t0009-broken-app.sh
@@ -0,0 +1,56 @@
+#!/bin/sh
+. ./test-lib.sh
+
+t_plan 9 "graceful handling of broken apps for $model"
+
+t_begin "setup and start" && {
+        rainbows_setup
+        rainbows -D t0009.ru -c $unicorn_config $model 1
+        rainbows_wait_start
+}
+
+t_begin "normal response is alright" && {
+        test xOK = x"$(curl -sSf http://$listen/)"
+}
+
+t_begin "app raised exception" && {
+        curl -sSf http://$listen/raise 2> $tmp || :
+        grep -F 500 $tmp
+        > $tmp
+}
+
+t_begin "app exception logged and backtrace not swallowed" && {
+        grep -F 'app error' $r_err
+        grep -A1 -F 'app error' $r_err | tail -1 | grep t0009.ru:
+        dbgcat r_err
+        > $r_err
+}
+
+t_begin "trigger bad response" && {
+        curl -sSf http://$listen/nil 2> $tmp || :
+        grep -F 500 $tmp
+        > $tmp
+}
+
+t_begin "app exception logged" && {
+        grep -F 'app error' $r_err
+        > $r_err
+}
+
+t_begin "normal responses alright afterwards" && {
+        > $tmp
+        curl -sSf http://$listen/ >> $tmp &
+        curl -sSf http://$listen/ >> $tmp &
+        curl -sSf http://$listen/ >> $tmp &
+        curl -sSf http://$listen/ >> $tmp &
+        wait
+        test xOK = x$(sort < $tmp | uniq)
+}
+
+t_begin "teardown" && {
+        kill $rainbows_pid
+}
+
+t_begin "check stderr" && check_stderr
+
+t_done
diff --git a/t/t0009.ru b/t/t0009.ru
new file mode 100644
index 0000000..bc7cb0b
--- /dev/null
+++ b/t/t0009.ru
@@ -0,0 +1,13 @@
+#\-E none
+# we do not want Rack::Lint or anything to protect us
+use Rack::ContentLength
+use Rack::ContentType, "text/plain"
+map "/" do
+  run lambda { |env| [ 200, {}, [ "OK\n" ] ] }
+end
+map "/raise" do
+  run lambda { |env| raise "BAD" }
+end
+map "/nil" do
+  run lambda { |env| nil }
+end