Do not cancel PuSH subscriptions after encountering "permanent" error… (#3046)
* 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
This commit is contained in:
		| @@ -23,13 +23,9 @@ class Pubsubhubbub::DeliveryWorker | |||||||
|   def process_delivery |   def process_delivery | ||||||
|     payload_delivery |     payload_delivery | ||||||
|  |  | ||||||
|     if response_successful? |     raise "Delivery failed for #{subscription.callback_url}: HTTP #{payload_delivery.code}" unless response_successful? | ||||||
|       subscription.touch(:last_successful_delivery_at) |  | ||||||
|     elsif response_failed_permanently? |     subscription.touch(:last_successful_delivery_at) | ||||||
|       subscription.destroy! |  | ||||||
|     else |  | ||||||
|       raise "Delivery failed for #{subscription.callback_url}: HTTP #{payload_delivery.code}" |  | ||||||
|     end |  | ||||||
|   end |   end | ||||||
|  |  | ||||||
|   def payload_delivery |   def payload_delivery | ||||||
| @@ -82,10 +78,6 @@ class Pubsubhubbub::DeliveryWorker | |||||||
|     OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha1'), subscription.secret, payload) |     OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha1'), subscription.secret, payload) | ||||||
|   end |   end | ||||||
|  |  | ||||||
|   def response_failed_permanently? |  | ||||||
|     payload_delivery.code > 299 && payload_delivery.code < 500 && payload_delivery.code != 429 |  | ||||||
|   end |  | ||||||
|  |  | ||||||
|   def response_successful? |   def response_successful? | ||||||
|     payload_delivery.code > 199 && payload_delivery.code < 300 |     payload_delivery.code > 199 && payload_delivery.code < 300 | ||||||
|   end |   end | ||||||
|   | |||||||
| @@ -22,15 +22,6 @@ describe Pubsubhubbub::DeliveryWorker do | |||||||
|       expect(subscription.reload.last_successful_delivery_at).to be_within(2).of(2.days.ago) |       expect(subscription.reload.last_successful_delivery_at).to be_within(2).of(2.days.ago) | ||||||
|     end |     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 |     it 'raises when request fails' do | ||||||
|       subscription = Fabricate(:subscription) |       subscription = Fabricate(:subscription) | ||||||
|  |  | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user