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:
		
							parent
							
								
									3434547038
								
							
						
					
					
						commit
						83b444ddab
					
				
					 2 changed files with 3 additions and 20 deletions
				
			
		| 
						 | 
					@ -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)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in a new issue