Skip to content

Commit 9aa4d2a

Browse files
authored
Warn on partial UDP send failure, raise SendError when all fail (#23)
* Log UDP send failures as warnings, not errors A host with both A and AAAA records on a network with no route for one family (e.g. v4-only network with AAAA record) will fail the send for the unreachable family while the other family succeeds. The previous "Sparoid error sending ..." wording made the partial failure look fatal; reword to "Sparoid warn: skip <ip>: <reason>" to make clear it is recoverable, mirroring sparoid.rb#20. * Include hostname in UDP send warning * Suppress per-address UDP send errors when one family succeeds When a host resolves to both A and AAAA addresses but the network only routes one family, the unreachable family's send raises EHOSTUNREACH. Previously every per-addr failure was logged, which is noisy when the other family's send succeeded and the call as a whole worked. Collect errors and only log them if every address failed; include the original hostname alongside the IP for easier triage. * Extract send_errors_to_report helper and unit-test it The previous integration tests relied on "send to 0.0.0.0 fails with EHOSTUNREACH" to exercise the all-failed branch, which is true on macOS but not on the Linux CI runners where the kernel accepts the send. Extract the error-reporting decision into a pure helper that can be unit-tested with synthetic inputs, independent of OS networking behavior. * Raise SendError when no UDP send succeeds, warn on partial failure - Per-address failure with at least one success: STDERR warn line ("Sparoid warn: skip <host> (<ip>): <reason>") so v4-only networks don't make AAAA-resolved hosts log catastrophically. - Every address failed: raise Sparoid::Client::SendError with all per-address details. The CLI already catches and exits 1, so this surfaces a clear actionable failure instead of returning quietly and failing later in fdpass. * Emit partial-failure warnings via Log.warn instead of raw STDERR Apps embedding sparoid as a shard at scale (15k+ servers) need partial UDP send failures classified as WARN-level by their monitoring, not as ERROR-level (which most aggregators infer from STDERR by default). - Sparoid::Client::Log = ::Log.for(self) declares a log source so apps can filter sparoid output specifically. - udp_send emits per-address skips via Log.warn { ... }. - client-cli configures Log.setup_from_env with an STDERR backend (in :connect mode STDOUT is the unix-domain FD-passing channel and isn't safe for free-form output). * Address PR review: drop redundant prefix, test send-result branching - SendError no longer prepends 'Sparoid:' to the message — the CLI's rescue clause already prefixes 'Sparoid error:', so the previous format produced 'Sparoid error: Sparoid: failed to send...'. - Replace the format_send_errors formatting helper with a process_send_results helper that owns the partial-vs-total decision. This raises SendError directly when every send failed and returns the per-address partial-failure errors otherwise. - Specs now cover both branches plus the all-succeeded and empty-input cases, exercising the actual control flow rather than just message formatting.
1 parent 9a95e49 commit 9aa4d2a

3 files changed

Lines changed: 70 additions & 3 deletions

File tree

spec/sparoid_spec.cr

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,3 +142,43 @@ describe Sparoid::Server do
142142
s.try &.close
143143
end
144144
end
145+
146+
describe Sparoid::Client do
147+
describe ".process_send_results" do
148+
it "raises SendError when every send failed" do
149+
ip1 = Socket::IPAddress.new("1.1.1.1", 1)
150+
ip2 = Socket::IPAddress.new("2.2.2.2", 2)
151+
results = [
152+
{ip1, Exception.new("no route").as(Exception?)},
153+
{ip2, Exception.new("network down").as(Exception?)},
154+
]
155+
ex = expect_raises(Sparoid::Client::SendError) do
156+
Sparoid::Client.process_send_results("example.com", results)
157+
end
158+
ex.message.to_s.should contain("example.com")
159+
ex.message.to_s.should contain("1.1.1.1:1: no route")
160+
ex.message.to_s.should contain("2.2.2.2:2: network down")
161+
end
162+
163+
it "returns partial-failure errors when at least one send succeeded" do
164+
ip1 = Socket::IPAddress.new("1.1.1.1", 1)
165+
ip2 = Socket::IPAddress.new("2.2.2.2", 2)
166+
err = Exception.new("oops")
167+
results = [
168+
{ip1, nil.as(Exception?)},
169+
{ip2, err.as(Exception?)},
170+
]
171+
Sparoid::Client.process_send_results("example.com", results).should eq [{ip2, err}]
172+
end
173+
174+
it "returns nothing when all sends succeeded" do
175+
ip = Socket::IPAddress.new("1.1.1.1", 1)
176+
results = [{ip, nil.as(Exception?)}]
177+
Sparoid::Client.process_send_results("example.com", results).should be_empty
178+
end
179+
180+
it "returns nothing when there were no addresses" do
181+
Sparoid::Client.process_send_results("example.com", [] of {Socket::IPAddress, Exception?}).should be_empty
182+
end
183+
end
184+
end

src/client-cli.cr

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
1+
require "log"
12
require "option_parser"
23
require "./client"
34
require "./version"
45

6+
# Route logs to STDERR (in :connect mode STDOUT is the unix-domain socket used for FD passing).
7+
Log.setup_from_env(default_level: :info, backend: Log::IOBackend.new(STDERR))
8+
59
subcommand = :none
610
host = "0.0.0.0"
711
port = 8484

src/client.cr

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
require "log"
12
require "socket"
23
require "random/secure"
34
require "openssl/cipher"
@@ -11,6 +12,10 @@ require "wait_group"
1112

1213
module Sparoid
1314
class Client
15+
Log = ::Log.for(self)
16+
17+
class SendError < Exception; end
18+
1419
def self.new(config_path = "~/.sparoid.ini")
1520
key = ENV.fetch("SPAROID_KEY", "")
1621
hmac_key = ENV.fetch("SPAROID_HMAC_KEY", "")
@@ -70,25 +75,43 @@ module Sparoid
7075
exit 1 # only if all connects fails
7176
end
7277

73-
# Send to all resolved IPs for the hostname
78+
# Send to all resolved IPs for the hostname.
79+
# Per-address failures are logged as warnings if at least one address succeeded.
80+
# If every address fails, raises SendError.
7481
private def self.udp_send(host, port, key : String, hmac_key : String, public_ip : String? = nil) : Array(String)
7582
host_addresses = Socket::Addrinfo.udp(host, port)
76-
host_addresses.each do |addrinfo|
83+
results = host_addresses.map do |addrinfo|
7784
packages = generate_messages(addrinfo.ip_address, public_ip).map { |message| generate_package(key, hmac_key, message) }
7885
socket = UDPSocket.new(addrinfo.family)
86+
error = nil.as(Exception?)
7987
begin
8088
packages.each do |data|
8189
socket.send data, to: addrinfo.ip_address
8290
end
8391
rescue ex
84-
STDERR << "Sparoid error sending " << ex.inspect << "\n"
92+
error = ex
8593
ensure
8694
socket.close
8795
end
96+
{addrinfo.ip_address, error}
97+
end
98+
process_send_results(host, results).each do |ip, ex|
99+
Log.warn { "skip #{host} (#{ip}): #{ex.message}" }
88100
end
89101
host_addresses.map &.ip_address.address
90102
end
91103

104+
# Decide whether per-address send failures are partial (warn) or total (raise).
105+
# Returns the per-address errors to warn about. Raises SendError when every send failed.
106+
def self.process_send_results(host : String, results : Array({Socket::IPAddress, Exception?})) : Array({Socket::IPAddress, Exception})
107+
errors = results.compact_map { |ip, err| err.try { |e| {ip, e} } }
108+
if !results.empty? && errors.size == results.size
109+
details = errors.map { |ip, ex| "#{ip}: #{ex.message}" }.join("; ")
110+
raise SendError.new("failed to send to any address for #{host}: #{details}")
111+
end
112+
errors
113+
end
114+
92115
private def self.encrypt(key, hmac_key, data) : Bytes
93116
cipher = OpenSSL::Cipher.new("aes-256-cbc")
94117
cipher.encrypt

0 commit comments

Comments
 (0)