You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by je...@apache.org on 2019/11/21 21:44:10 UTC
[thrift] branch master updated: THRIFT-4999: Raise proper exception on transport error Client: ruby Patch: Grégoire Seux
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 <g....@criteo.com>
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