Skip to content

Commit

Permalink
Wrap likely ABQ network errors to more easily identify as coming from…
Browse files Browse the repository at this point in the history
… ABQ connections. (#104)

* Wrap likely ABQ network errors to more easily identify as coming from ABQ connections.

* Fix socket opening for Ruby <3.0.

* Remove resolv_timeout for Ruby 2.6 support.

* Bump version 1.1.1

* Update test gemfiles.
  • Loading branch information
doxavore authored Feb 14, 2023
1 parent f4792c6 commit 045715a
Show file tree
Hide file tree
Showing 14 changed files with 88 additions and 38 deletions.
3 changes: 3 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,6 @@ RSpec/ExampleLength:

RSpec/MessageSpies:
EnforcedStyle: receive

RSpec/MultipleMemoizedHelpers:
Max: 10
10 changes: 5 additions & 5 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ GIT
PATH
remote: .
specs:
rspec-abq (1.1.0)
rspec-abq (1.1.1)
rspec-core (>= 3.5.0, < 3.13.0)

GEM
Expand Down Expand Up @@ -95,8 +95,8 @@ GEM
rubocop-ast (>= 1.24.1, < 2.0)
ruby-progressbar (~> 1.7)
unicode-display_width (>= 2.4.0, < 3.0)
rubocop-ast (1.24.1)
parser (>= 3.1.1.0)
rubocop-ast (1.26.0)
parser (>= 3.2.1.0)
rubocop-capybara (2.17.0)
rubocop (~> 1.41)
rubocop-performance (1.15.2)
Expand All @@ -123,8 +123,8 @@ GEM
simplecov_json_formatter (~> 0.1)
simplecov-html (0.12.3)
simplecov_json_formatter (0.1.4)
sorbet-runtime (0.5.10655)
standard (1.24.0)
sorbet-runtime (0.5.10662)
standard (1.24.3)
language_server-protocol (~> 3.17.0.2)
rubocop (= 1.44.1)
rubocop-performance (= 1.15.2)
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/rspec-3.10.gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ GIT
PATH
remote: ..
specs:
rspec-abq (1.1.0)
rspec-abq (1.1.1)
rspec-core (>= 3.5.0, < 3.13.0)

GEM
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/rspec-3.11.gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ GIT
PATH
remote: ..
specs:
rspec-abq (1.1.0)
rspec-abq (1.1.1)
rspec-core (>= 3.5.0, < 3.13.0)

GEM
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/rspec-3.12.gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ GIT
PATH
remote: ..
specs:
rspec-abq (1.1.0)
rspec-abq (1.1.1)
rspec-core (>= 3.5.0, < 3.13.0)

GEM
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/rspec-3.5.gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ GIT
PATH
remote: ..
specs:
rspec-abq (1.1.0)
rspec-abq (1.1.1)
rspec-core (>= 3.5.0, < 3.13.0)

GEM
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/rspec-3.6.gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ GIT
PATH
remote: ..
specs:
rspec-abq (1.1.0)
rspec-abq (1.1.1)
rspec-core (>= 3.5.0, < 3.13.0)

GEM
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/rspec-3.7.gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ GIT
PATH
remote: ..
specs:
rspec-abq (1.1.0)
rspec-abq (1.1.1)
rspec-core (>= 3.5.0, < 3.13.0)

GEM
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/rspec-3.8.gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ GIT
PATH
remote: ..
specs:
rspec-abq (1.1.0)
rspec-abq (1.1.1)
rspec-core (>= 3.5.0, < 3.13.0)

GEM
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/rspec-3.9.gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ GIT
PATH
remote: ..
specs:
rspec-abq (1.1.0)
rspec-abq (1.1.1)
rspec-core (>= 3.5.0, < 3.13.0)

GEM
Expand Down
55 changes: 35 additions & 20 deletions lib/rspec/abq.rb
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,20 @@ def self.setup_extensions_if_enabled!
Extensions.setup!
end

# Base ABQ error.
Error = Class.new(StandardError)

# raised when check_configuration fails
UnsupportedConfigurationError = Class.new(StandardError)
UnsupportedConfigurationError = Class.new(Error)

# Failed to connect and initialize handshake.
ConnectionFailed = Class.new(Error)

# Communication between abq sockets follows the following protocol:
# - The first 4 bytes an unsigned 32-bit integer (big-endian) representing
# the size of the rest of the message.
# - The rest of the message is a JSON-encoded payload.
ConnectionBroken = Class.new(Error)

# raises if RSpec is configured in a way that's incompatible with rspec-abq
def self.check_configuration!(config)
Expand Down Expand Up @@ -214,12 +226,16 @@ def self.fast_exit?

# Creates the socket to communicate with the worker and sends the worker the protocol
# @!visibility private
def self.socket
@socket ||= TCPSocket.new(*ENV[ABQ_SOCKET].split(":")).tap do |socket|
# Messages sent to/received from the ABQ worker should be done so ASAP.
# Since we're on a local network, we don't care about packet reduction here.
socket.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1)
protocol_write(NATIVE_RUNNER_SPAWNED_MESSAGE, socket)
def self.socket(connect_timeout: 5)
@socket ||= begin
Socket.tcp(*ENV[ABQ_SOCKET].split(":"), connect_timeout: connect_timeout).tap do |socket|
# Messages sent to/received from the ABQ worker should be done so ASAP.
# Since we're on a local network, we don't care about packet reduction here.
socket.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1)
protocol_write(NATIVE_RUNNER_SPAWNED_MESSAGE, socket)
end
rescue
raise ConnectionFailed, "Unable to connect to ABQ socket #{ENV[ABQ_SOCKET]}"
end
end

Expand All @@ -245,25 +261,18 @@ def self.fetch_next_example(message = protocol_read)
end
end

# Communication between abq sockets follows the following protocol:
# - The first 4 bytes an unsigned 32-bit integer (big-endian) representing
# the size of the rest of the message.
# - The rest of the message is a JSON-encoded payload.
class AbqConnBroken < StandardError
end

# Writes a message to an Abq socket using the 4-byte header protocol.
#
# @param socket [TCPSocket]
# @param msg
def self.protocol_write(msg, socket = Abq.socket)
json_msg = JSON.dump msg
begin
socket.write [json_msg.bytesize].pack("N")
socket.write json_msg
rescue
raise AbqConnBroken
end
socket.write [json_msg.bytesize].pack("N")
socket.write json_msg
rescue SystemCallError, IOError
raise ConnectionBroken
rescue
raise Error
end

# Writes a message to an Abq socket using the 4-byte header protocol.
Expand All @@ -273,10 +282,16 @@ def self.protocol_write(msg, socket = Abq.socket)
def self.protocol_read(socket = Abq.socket)
len_bytes = socket.read 4
return :abq_done if len_bytes.nil?

len = len_bytes.unpack1("N")
json_msg = socket.read len
return :abq_done if json_msg.nil?

JSON.parse json_msg
rescue SystemCallError, IOError
raise ConnectionBroken
rescue
raise Error
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rspec/abq/version.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module RSpec
module Abq
# current version!
VERSION = "1.1.0"
VERSION = "1.1.1"
end
end
2 changes: 1 addition & 1 deletion spec/NUM_TESTS
Original file line number Diff line number Diff line change
@@ -1 +1 @@
43
46
38 changes: 35 additions & 3 deletions spec/rspec/abq_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,14 @@ def stringify_keys(hash)
end

describe "socket communication", unless: RSpec::Abq.disable_tests_when_run_by_abq? do
host = "127.0.0.1"
let(:host) { "127.0.0.1" }
let(:port) { server.addr[1] }
let(:server) { TCPServer.new(host, 0) }
let(:client_sock) { TCPSocket.new(host, server.addr[1]) }
let(:client_sock) { TCPSocket.new(host, port) }
let(:server_sock) { server.accept }

around do |example|
EnvHelper.with_env("ABQ_SOCKET" => "#{host}:#{server.addr[1]}") do
EnvHelper.with_env("ABQ_SOCKET" => "#{host}:#{port}") do
example.call
end
RSpec::Abq.instance_eval { @socket = nil }
Expand All @@ -99,6 +100,15 @@ def stringify_keys(hash)
expect(server_sock.read(4).unpack1("N")).to eq(payload_length)
expect(server_sock.read(payload_length)).to eq(symbol_payload.to_json)
end

it "wraps errors with ConnectionBroken" do
socket = client_sock # touch before closing server
server.close

expect do
RSpec::Abq.protocol_write(symbol_payload, socket)
end.to raise_error(RSpec::Abq::ConnectionBroken)
end
end

describe ".protocol_read" do
Expand All @@ -109,6 +119,18 @@ def stringify_keys(hash)

expect(RSpec::Abq.protocol_read(server_sock)).to eq(JSON.parse(msg_payload))
end

it "wraps errors with ConnectionBroken" do
msg_payload = '{"a":1,"b":2}'
client_sock.write([msg_payload.length].pack("N"))
client_sock.write(msg_payload)

server_sock.close

expect do
RSpec::Abq.protocol_read(server_sock)
end.to raise_error(RSpec::Abq::ConnectionBroken)
end
end

describe ".socket" do
Expand All @@ -119,6 +141,16 @@ def stringify_keys(hash)
RSpec::Abq.protocol_write({"init_meta" => {"seed" => 4, "ordering_class" => "RSpec::Core::Ordering::Identity"}}, server_sock)
expect(RSpec::Abq.protocol_read(server_sock)).to(eq(stringify_keys(RSpec::Abq::NATIVE_RUNNER_SPAWNED_MESSAGE)))
end

it "fails with ConnectionFailed when connection times out" do
# Force this error to avoid flakiness.
allow(Socket).to receive(:tcp).with(host, port.to_s, connect_timeout: 0.001)
.and_raise(Errno::ETIMEDOUT, "forced error")

expect do
RSpec::Abq.socket(connect_timeout: 0.001)
end.to raise_error(RSpec::Abq::ConnectionFailed)
end
end
end

Expand Down

0 comments on commit 045715a

Please sign in to comment.