From 657496b5a9488b904166c33764500b364e024679 Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Sun, 14 May 2017 03:22:48 +0200
Subject: [PATCH] =?UTF-8?q?Do=20not=20cancel=20PuSH=20subscriptions=20afte?=
=?UTF-8?q?r=20encountering=20"permanent"=20error=E2=80=A6=20(#3046)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* Do not cancel PuSH subscriptions after encountering "permanent" error response
After talking with MMN about it, turns out some servers/php setups do
return 4xx errors while rebooting, so this anti-feature that was meant
to take load off of the hub is doing more harm than good in terms of
breaking subscriptions
* Update delivery_worker.rb
---
app/workers/pubsubhubbub/delivery_worker.rb | 14 +++-----------
spec/workers/pubsubhubbub/delivery_worker_spec.rb | 9 ---------
2 files changed, 3 insertions(+), 20 deletions(-)
diff --git a/app/workers/pubsubhubbub/delivery_worker.rb b/app/workers/pubsubhubbub/delivery_worker.rb
index 708a5fb9f..981838f33 100644
--- a/app/workers/pubsubhubbub/delivery_worker.rb
+++ b/app/workers/pubsubhubbub/delivery_worker.rb
@@ -23,13 +23,9 @@ class Pubsubhubbub::DeliveryWorker
def process_delivery
payload_delivery
- if response_successful?
- subscription.touch(:last_successful_delivery_at)
- elsif response_failed_permanently?
- subscription.destroy!
- else
- raise "Delivery failed for #{subscription.callback_url}: HTTP #{payload_delivery.code}"
- end
+ raise "Delivery failed for #{subscription.callback_url}: HTTP #{payload_delivery.code}" unless response_successful?
+
+ subscription.touch(:last_successful_delivery_at)
end
def payload_delivery
@@ -82,10 +78,6 @@ class Pubsubhubbub::DeliveryWorker
OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha1'), subscription.secret, payload)
end
- def response_failed_permanently?
- payload_delivery.code > 299 && payload_delivery.code < 500 && payload_delivery.code != 429
- end
-
def response_successful?
payload_delivery.code > 199 && payload_delivery.code < 300
end
diff --git a/spec/workers/pubsubhubbub/delivery_worker_spec.rb b/spec/workers/pubsubhubbub/delivery_worker_spec.rb
index ec1e319d5..081dfa41c 100644
--- a/spec/workers/pubsubhubbub/delivery_worker_spec.rb
+++ b/spec/workers/pubsubhubbub/delivery_worker_spec.rb
@@ -22,15 +22,6 @@ describe Pubsubhubbub::DeliveryWorker do
expect(subscription.reload.last_successful_delivery_at).to be_within(2).of(2.days.ago)
end
- it 'destroys subscription when request fails permanently' do
- subscription = Fabricate(:subscription)
-
- stub_request_to_respond_with(subscription, 404)
- subject.perform(subscription.id, payload)
-
- expect { subscription.reload }.to raise_error(ActiveRecord::RecordNotFound)
- end
-
it 'raises when request fails' do
subscription = Fabricate(:subscription)