Immediately set the deferred status to succeeded after receiving a se…

…rver exception.

This is needed so that the error is returned as soon as possible instead of having to wait until all the hooks (on_headers, on_body, response) have been executed (for nothing).

examples/error.rb

@@ -0,0 +1,32 @@
+#!/usr/bin/env ruby
+$:<< '../lib' << 'lib'
+
+require 'goliath'
+
+class Error < Goliath::API
+  include Goliath::Validation
+  MAX_SIZE = 10
+  TEST_FILE = "/tmp/goliath-test-error.log"
+
+  def on_headers(env, headers)
+    env.logger.info 'received headers: ' + headers.inspect
+    env['async-headers'] = headers
+    raise Goliath::Validation::NotImplementedError.new("Can't handle requests with X-Crash: true.") if env['HTTP_X_CRASH'] && env['HTTP_X_CRASH'] == 'true'
+  end
+
+  def on_body(env, data)
+    env.logger.info 'received data: ' + data
+    (env['async-body'] ||= '') << data
+    size = env['async-body'].size
+    raise Goliath::Validation::BadRequestError.new("Payload size can't exceed #{MAX_SIZE} bytes. Received #{size.inspect} bytes.") if size >= MAX_SIZE
+  end
+
+  def on_close(env)
+    env.logger.info 'closing connection'
+  end
+
+  def response(env) 
+    File.open(TEST_FILE, "w+") { |f| f << "response that should not be here"}
+    [200, {}, "OK"]
+  end
+end

lib/goliath/request.rb

@@ -200,6 +200,17 @@ def server_exception(e)
       headers['Content-Length'] = body.bytesize.to_s
       @env[:terminate_connection] = true
       post_process([status, headers, body])
+      # Pass the request status to succeeded, so that the callback declared in
+      # #post_process is executed as soon as possible, and not after the whole
+      # response has been generated. For example, if you wanted to abort the
+      # request in a #on_headers or #on_body hook (through exception raising),
+      # the previous behaviour still went through all the hooks and the
+      # #response method before returning the error. Now the error fires as
+      # soon as possible. Note that #on_body and #response hooks may still be
+      # executed if your machine is very fast or the request is very small,
+      # but at least for long requests you don't get stuck until the full
+      # request has been received.
+      succeed
     end
 
     # Used to determine if the connection should  be kept open

spec/integration/error_spec.rb

@@ -0,0 +1,50 @@
+require 'spec_helper'
+require File.join(File.dirname(__FILE__), '../../', 'examples/error')
+
+describe Error do
+  let(:err) { Proc.new { fail "API request failed" } }
+
+  after do
+    File.unlink(Error::TEST_FILE) if File.exist?(Error::TEST_FILE)
+  end
+
+  it "should return OK" do
+    with_api(Error) do
+      get_request({}, err) do |c|
+        c.response.should == "OK"
+      end
+    end
+  end
+
+  # The following two tests are very brittle, since they depend on the speed
+  # of the machine executing the test and the size of the incoming data
+  # packets. I hope someone more knowledgeable will be able to refactor these
+  # ;-)
+  it 'fails without going in the response method if exception is raised in on_header hook' do
+    with_api(Error) do
+      request_data = {
+        :body => (["abcd"] * 200_000).join,
+        :head => {'X-Crash' => 'true'}
+      }
+
+      post_request(request_data, err) do |c|
+        c.response.should == "{\"error\":\"Can't handle requests with X-Crash: true.\"}"
+        File.exist?("/tmp/goliath-test-error.log").should be_false
+      end
+    end
+  end
+
+  it 'fails without going in the response method if exception is raised in on_body hook' do
+    with_api(Error) do
+      request_data = {
+        :body => (["abcd"] * 200_000).join
+      }
+
+      post_request(request_data, err) do |c|
+        c.response.should =~ /Payload size can't exceed 10 bytes/
+        File.exist?("/tmp/goliath-test-error.log").should be_false
+      end
+    end
+  end
+
+end