This is an automated email from the ASF dual-hosted git repository.
jensg pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/thrift.git
The following commit(s) were added to refs/heads/master by this push:
new 8ae80a7 THRIFT-4999: Raise proper exception on transport error
Client: ruby Patch: Grégoire Seux
8ae80a7 is described below
commit 8ae80a7f8466e5c340388fcb1d797dc3779d9f80
Author: Grégoire Seux <[email protected]>
AuthorDate: Thu Nov 7 11:33:58 2019 +0100
THRIFT-4999: Raise proper exception on transport error
Client: ruby
Patch: Grégoire Seux
This closes #1924
Before this patch, any error on the http layer was ignored and usually
seen by the user as ProtocolException instead of TransportException
---
.../lib/thrift/transport/http_client_transport.rb | 2 ++
lib/rb/spec/http_client_spec.rb | 22 ++++++++++++++++++++++
2 files changed, 24 insertions(+)
diff --git a/lib/rb/lib/thrift/transport/http_client_transport.rb
b/lib/rb/lib/thrift/transport/http_client_transport.rb
index 5c1dd5c..c84304d 100644
--- a/lib/rb/lib/thrift/transport/http_client_transport.rb
+++ b/lib/rb/lib/thrift/transport/http_client_transport.rb
@@ -47,6 +47,8 @@ module Thrift
http.use_ssl = @url.scheme == 'https'
http.verify_mode = @ssl_verify_mode if @url.scheme == 'https'
resp = http.post(@url.request_uri, @outbuf, @headers)
+ raise TransportException.new(TransportException::UNKNOWN,
"#{self.class.name} Could not connect to #{@url}, HTTP status code
#{resp.code.to_i}") unless (200..299).include?(resp.code.to_i)
+
data = resp.body
data = Bytes.force_binary_encoding(data)
@inbuf = StringIO.new data
diff --git a/lib/rb/spec/http_client_spec.rb b/lib/rb/spec/http_client_spec.rb
index df472ab..292c752 100644
--- a/lib/rb/spec/http_client_spec.rb
+++ b/lib/rb/spec/http_client_spec.rb
@@ -45,6 +45,7 @@ describe 'Thrift::HTTPClientTransport' do
expect(http).to receive(:post).with("/path/to/service?param=value",
"a test frame", {"Content-Type"=>"application/x-thrift"}) do
double("Net::HTTPOK").tap do |response|
expect(response).to receive(:body).and_return "data"
+ expect(response).to receive(:code).and_return "200"
end
end
end
@@ -65,6 +66,7 @@ describe 'Thrift::HTTPClientTransport' do
expect(http).to receive(:post).with("/path/to/service?param=value",
"test", headers) do
double("Net::HTTPOK").tap do |response|
expect(response).to receive(:body).and_return "data"
+ expect(response).to receive(:code).and_return "200"
end
end
end
@@ -86,6 +88,24 @@ describe 'Thrift::HTTPClientTransport' do
expect(@client.instance_variable_get(:@outbuf)).to
eq(Thrift::Bytes.empty_byte_buffer)
end
+ it 'should raise TransportError on HTTP failures' do
+ @client.write "test"
+
+ expect(Net::HTTP).to receive(:new).with("my.domain.com", 80) do
+ double("Net::HTTP").tap do |http|
+ expect(http).to receive(:use_ssl=).with(false)
+ expect(http).to receive(:post).with("/path/to/service?param=value",
"test", {"Content-Type"=>"application/x-thrift"}) do
+ double("Net::HTTPOK").tap do |response|
+ expect(response).not_to receive(:body)
+ expect(response).to receive(:code).at_least(:once).and_return
"503"
+ end
+ end
+ end
+ end
+
+ expect { @client.flush }.to raise_error(Thrift::TransportException)
+ end
+
end
describe 'ssl enabled' do
@@ -107,6 +127,7 @@ describe 'Thrift::HTTPClientTransport' do
"Content-Type" => "application/x-thrift") do
double("Net::HTTPOK").tap do |response|
expect(response).to receive(:body).and_return "data"
+ expect(response).to receive(:code).and_return "200"
end
end
end
@@ -128,6 +149,7 @@ describe 'Thrift::HTTPClientTransport' do
"Content-Type" => "application/x-thrift") do
double("Net::HTTPOK").tap do |response|
expect(response).to receive(:body).and_return "data"
+ expect(response).to receive(:code).and_return "200"
end
end
end
