Rack response needs to be finished when returned from the Rack application

kpumuk · Jens-G · commit 5cb828f4b1a6 · 2025-12-02T23:13:18.000+01:00
This merge request addresses the following test failures: ``` 1) Thrift::ThinHTTPServer::RackApplication 404 response receives a non-POST Failure/Error: expect(last_response.status).to be 404 expected #<Integer:809> => 404 got #<Integer:1001> => 500 Compared using equal?, which compares object identity, but expected and actual are not the same object. Use `expect(actual).to eq(expected)` if you don't care about object identity in this example. # ./spec/thin_http_server_spec.rb:102:in 'block (3 levels) in <top (required)>' 2) Thrift::ThinHTTPServer::RackApplication 404 response receives a header other than application/x-thrift Failure/Error: expect(last_response.status).to be 404 expected #<Integer:809> => 404 got #<Integer:1001> => 500 Compared using equal?, which compares object identity, but expected and actual are not the same object. Use `expect(actual).to eq(expected)` if you don't care about object identity in this example. # ./spec/thin_http_server_spec.rb:108:in 'block (3 levels) in <top (required)>' 3) Thrift::ThinHTTPServer::RackApplication 200 response status code 200 Failure/Error: expect(last_response.ok?).to be_truthy expected: truthy value got: false # ./spec/thin_http_server_spec.rb:135:in 'block (3 levels) in <top (required)>' ``` From the [Rack documentation](https://rack.github.io/rack/2.2/Rack/Response.html), > Your application’s call should end returning [Response#finish](https://rack.github.io/rack/2.2/Rack/Response.html#method-i-finish). Additionally: * using identity checks on integers produces weird expectation "expected #<Integer:809> => 404", which is not necessary. Switched to using `eq` matcher * `be_truthy` matcher is overly generic and identity matching on `true`/`false` is preferred: "expected true" output will be displayed on failure
diff --git a/lib/rb/lib/thrift/server/thin_http_server.rb b/lib/rb/lib/thrift/server/thin_http_server.rb
@@ -60,9 +60,9 @@ def self.for(path, processor, protocol_factory)
             run lambda { |env|
               request = Rack::Request.new(env)
               if RackApplication.valid_thrift_request?(request)
-                RackApplication.successful_request(request, processor, protocol_factory)
+                RackApplication.successful_request(request, processor, protocol_factory).finish
               else
-                RackApplication.failed_request
+                RackApplication.failed_request.finish
               end
             }
           end
diff --git a/lib/rb/spec/thin_http_server_spec.rb b/lib/rb/spec/thin_http_server_spec.rb
@@ -99,13 +99,13 @@ def app
     it 'receives a non-POST' do
       header('Content-Type', "application/x-thrift")
       get "/"
-      expect(last_response.status).to be 404
+      expect(last_response.status).to eq 404
     end
 
     it 'receives a header other than application/x-thrift' do
       header('Content-Type', "application/json")
       post "/"
-      expect(last_response.status).to be 404
+      expect(last_response.status).to eq 404
     end
 
   end
@@ -132,10 +132,9 @@ def app
     it 'status code 200' do
       header('Content-Type', "application/x-thrift")
       post "/"
-      expect(last_response.ok?).to be_truthy
+      expect(last_response.ok?).to be true
     end
 
   end
 
 end
-
