From b29675fead857cf6370d5ad70cac97f0d975ec3a Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sat, 23 Jul 2022 17:58:09 +0200 Subject: [PATCH 1/3] Remove error if there was no error anymore also add pod uri when logging offline pods ... just having a bunch of "OFFLINE" log messages doesn't help at all. --- app/models/pod.rb | 4 ++-- spec/models/pod_spec.rb | 30 ++++++++++++++++-------------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/app/models/pod.rb b/app/models/pod.rb index c64bf4ce2..08692f280 100644 --- a/app/models/pod.rb +++ b/app/models/pod.rb @@ -114,7 +114,7 @@ class Pod < ApplicationRecord def update_from_result(result) self.status = status_from_result(result) update_offline_since - logger.warn "OFFLINE #{result.failure_message}" if offline? + logger.warn "#{uri} OFFLINE: #{result.failure_message}" if offline? attributes_from_result(result) touch(:checked_at) @@ -125,7 +125,7 @@ class Pod < ApplicationRecord def attributes_from_result(result) self.ssl ||= result.ssl - self.error = result.failure_message[0..254] if result.error? + self.error = result.error? ? result.failure_message[0..254] : nil self.software = result.software_version[0..254] if result.software_version.present? self.response_time = result.rt end diff --git a/spec/models/pod_spec.rb b/spec/models/pod_spec.rb index a8001d3a8..6ad265b2f 100644 --- a/spec/models/pod_spec.rb +++ b/spec/models/pod_spec.rb @@ -143,31 +143,28 @@ describe Pod, type: :model do describe "#test_connection!" do before do @pod = FactoryGirl.create(:pod) - @result = double("result") @now = Time.zone.now - allow(@result).to receive(:rt) { 123 } - allow(@result).to receive(:software_version) { "diaspora a.b.c.d" } - allow(@result).to receive(:failure_message) { "hello error!" } + @result = ConnectionTester::Result.new + @result.rt = 123 + @result.software_version = "diaspora a.b.c.d" + @result.error = ConnectionTester::NetFailure.new("hello error!") expect(ConnectionTester).to receive(:check).at_least(:once).and_return(@result) end it "updates the connectivity values" do - allow(@result).to receive(:error) - allow(@result).to receive(:error?) + @result.error = nil @pod.test_connection! expect(@pod.status).to eq("no_errors") - expect(@pod.offline?).to be_falsy + expect(@pod.offline?).to be_falsey expect(@pod.response_time).to eq(123) expect(@pod.checked_at).to be_within(1.second).of @now end it "resets the scheduled_check flag" do - allow(@result).to receive(:error) - allow(@result).to receive(:error?) - @pod.update_column(:scheduled_check, true) + @pod.update(scheduled_check: true) @pod.test_connection! @@ -175,17 +172,22 @@ describe Pod, type: :model do end it "handles a failed check" do - expect(@result).to receive(:error?).at_least(:once) { true } - expect(@result).to receive(:error).at_least(:once) { ConnectionTester::NetFailure.new } @pod.test_connection! expect(@pod.offline?).to be_truthy expect(@pod.offline_since).to be_within(1.second).of @now end + it "removes the error message when there was no error" do + @pod.update(error: "old error message") + + @result.error = nil + @pod.test_connection! + + expect(@pod.error).to be_nil + end + it "preserves the original offline timestamp" do - expect(@result).to receive(:error?).at_least(:once) { true } - expect(@result).to receive(:error).at_least(:once) { ConnectionTester::NetFailure.new } @pod.test_connection! expect(@pod.offline_since).to be_within(1.second).of @now From 78b28c3d54b449ab1439d7dfa8fb2f6a20dfbe13 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sat, 23 Jul 2022 18:04:22 +0200 Subject: [PATCH 2/3] Handle nodeinfo timeouts gracefully some (especially bigger pods) are sometimes slow to respond with statistics, so lets handle that gracefully and not mark the pods as down. --- lib/connection_tester.rb | 6 +++--- spec/lib/connection_tester_spec.rb | 8 ++++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/connection_tester.rb b/lib/connection_tester.rb index 5957b80d5..810a3f2bd 100644 --- a/lib/connection_tester.rb +++ b/lib/connection_tester.rb @@ -129,14 +129,14 @@ class ConnectionTester version, url = ni_urls.max find_software_version(version, http.get(url).body) end - rescue Faraday::ClientError => e - raise HTTPFailure, "#{e.class}: #{e.message}" rescue NodeInfoFailure => e raise e - rescue JSON::Schema::ValidationError, JSON::Schema::SchemaError => e + rescue JSON::Schema::ValidationError, JSON::Schema::SchemaError, Faraday::TimeoutError => e raise NodeInfoFailure, "#{e.class}: #{e.message}" rescue JSON::JSONError => e raise NodeInfoFailure, e.message[0..255].encode(Encoding.default_external, undef: :replace) + rescue Faraday::ClientError => e + raise HTTPFailure, "#{e.class}: #{e.message}" rescue StandardError => e unexpected_error(e) end diff --git a/spec/lib/connection_tester_spec.rb b/spec/lib/connection_tester_spec.rb index 671fec860..a0dce4d4f 100644 --- a/spec/lib/connection_tester_spec.rb +++ b/spec/lib/connection_tester_spec.rb @@ -172,6 +172,14 @@ describe ConnectionTester do expect { tester.nodeinfo }.to raise_error(ConnectionTester::NodeInfoFailure) end + it "handles timeout gracefully" do + ni_wellknown = {links: [{rel: "http://nodeinfo.diaspora.software/ns/schema/1.0", href: "/nodeinfo/1.0"}]} + stub_request(:get, "#{url}#{ConnectionTester::NODEINFO_FRAGMENT}") + .to_return(status: 200, body: JSON.generate(ni_wellknown)) + stub_request(:get, "#{url}/nodeinfo/1.0").to_raise(Faraday::TimeoutError.new) + expect { tester.nodeinfo }.to raise_error(ConnectionTester::NodeInfoFailure) + end + it "handles a invalid jrd document gracefully" do invalid_wellknown = {links: {rel: "http://nodeinfo.diaspora.software/ns/schema/1.0", href: "/nodeinfo/1.0"}} stub_request(:get, "#{url}#{ConnectionTester::NODEINFO_FRAGMENT}") From 646685b42cd4cf14f6431633ef1b9f5c2d80ccfc Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sun, 24 Jul 2022 00:22:43 +0200 Subject: [PATCH 3/3] Handle Faraday::ServerError (for example 502) as HTTPFailure closes #8380 --- Changelog.md | 1 + lib/connection_tester.rb | 2 +- spec/lib/connection_tester_spec.rb | 5 +++++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index adc40086a..83d223c5f 100644 --- a/Changelog.md +++ b/Changelog.md @@ -12,6 +12,7 @@ * Update the suggested Ruby version to 2.7. If you run into trouble during the update and you followed our installation guides, run `rvm install 2.7`. [#8366](https://github.com/diaspora/diaspora/pull/8366) * Upgrade to bundler 2 [#8366](https://github.com/diaspora/diaspora/pull/8366) * Stop checking `/.well-known/host-meta`, check for `/.well-known/nodeinfo` instead [#8377](https://github.com/diaspora/diaspora/pull/8377) +* Handle NodeInfo timeouts gracefully [#8380](https://github.com/diaspora/diaspora/pull/8380) ## Bug fixes * Fix that no mails were sent after photo export [#8365](https://github.com/diaspora/diaspora/pull/8365) diff --git a/lib/connection_tester.rb b/lib/connection_tester.rb index 810a3f2bd..d66f418f8 100644 --- a/lib/connection_tester.rb +++ b/lib/connection_tester.rb @@ -109,7 +109,7 @@ class ConnectionTester raise NetFailure, e.message rescue Faraday::SSLError => e raise SSLFailure, e.message - rescue ArgumentError, Faraday::ClientError => e + rescue ArgumentError, Faraday::ClientError, Faraday::ServerError => e raise HTTPFailure, "#{e.class}: #{e.message}" rescue StandardError => e unexpected_error(e) diff --git a/spec/lib/connection_tester_spec.rb b/spec/lib/connection_tester_spec.rb index a0dce4d4f..d665dd717 100644 --- a/spec/lib/connection_tester_spec.rb +++ b/spec/lib/connection_tester_spec.rb @@ -94,6 +94,11 @@ describe ConnectionTester do expect { tester.request }.to raise_error(ConnectionTester::HTTPFailure) end + it "receives a 502 bad gateway" do + stub_request(:get, url).to_return(status: 502, body: "Bad Gateway!") + expect { tester.request }.to raise_error(ConnectionTester::HTTPFailure) + end + it "cannot connect" do stub_request(:get, url).to_raise(Faraday::ConnectionFailed.new("Error!")) expect { tester.request }.to raise_error(ConnectionTester::NetFailure)