diff options
| -rw-r--r-- | app/services/subscribe_service.rb | 16 | ||||
| -rw-r--r-- | app/services/unsubscribe_service.rb | 4 | ||||
| -rw-r--r-- | spec/services/subscribe_service_spec.rb | 5 | ||||
| -rw-r--r-- | spec/services/unsubscribe_service_spec.rb | 37 |
4 files changed, 52 insertions, 10 deletions
diff --git a/app/services/subscribe_service.rb b/app/services/subscribe_service.rb index 138718f14..1e7984a7f 100644 --- a/app/services/subscribe_service.rb +++ b/app/services/subscribe_service.rb @@ -8,28 +8,28 @@ class SubscribeService < BaseService response = subscription.subscribe if response_failed_permanently?(response) - # An error in the 4xx range (except for 429, which is rate limiting) - # means we're not allowed to subscribe. Fail and move on + # We're not allowed to subscribe. Fail and move on. account.secret = '' account.save! elsif response_successful?(response) - # Anything in the 2xx range means the subscription will be confirmed - # asynchronously, we've done what we needed to do + # The subscription will be confirmed asynchronously. account.save! else - # What's left is the 5xx range and 429, which means we need to retry - # at a later time. Fail loudly! + # The response was either a 429 rate limit, or a 5xx error. + # We need to retry at a later time. Fail loudly! raise "Subscription attempt failed for #{account.acct} (#{account.hub_url}): HTTP #{response.code}" end end private + # Any response in the 3xx or 4xx range, except for 429 (rate limit) def response_failed_permanently?(response) - response.code > 299 && response.code < 500 && response.code != 429 + (response.status.redirect? || response.status.client_error?) && !response.status.too_many_requests? end + # Any response in the 2xx range def response_successful?(response) - response.code > 199 && response.code < 300 + response.status.success? end end diff --git a/app/services/unsubscribe_service.rb b/app/services/unsubscribe_service.rb index 1a951d1b4..6db8dbdc4 100644 --- a/app/services/unsubscribe_service.rb +++ b/app/services/unsubscribe_service.rb @@ -5,8 +5,8 @@ class UnsubscribeService < BaseService subscription = account.subscription(api_subscription_url(account.id)) response = subscription.unsubscribe - unless response.successful? - Rails.logger.debug "PuSH unsubscribe for #{account.acct} failed: #{response.message}" + unless response.status.success? + Rails.logger.debug "PuSH unsubscribe for #{account.acct} failed: #{response.status}" end account.secret = '' diff --git a/spec/services/subscribe_service_spec.rb b/spec/services/subscribe_service_spec.rb index 01cdb29c0..5db91ad99 100644 --- a/spec/services/subscribe_service_spec.rb +++ b/spec/services/subscribe_service_spec.rb @@ -35,4 +35,9 @@ RSpec.describe SubscribeService do stub_request(:post, 'http://hub.example.com/').to_return(status: 503) expect { subject.call(account) }.to raise_error(/Subscription attempt failed/) end + + it 'fails loudly if rate limited' do + stub_request(:post, 'http://hub.example.com/').to_return(status: 429) + expect { subject.call(account) }.to raise_error(/Subscription attempt failed/) + end end diff --git a/spec/services/unsubscribe_service_spec.rb b/spec/services/unsubscribe_service_spec.rb new file mode 100644 index 000000000..c81772037 --- /dev/null +++ b/spec/services/unsubscribe_service_spec.rb @@ -0,0 +1,37 @@ +require 'rails_helper' + +RSpec.describe UnsubscribeService do + let(:account) { Fabricate(:account, username: 'bob', domain: 'example.com', hub_url: 'http://hub.example.com') } + subject { UnsubscribeService.new } + + it 'removes the secret and resets expiration on account' do + stub_request(:post, 'http://hub.example.com/').to_return(status: 204) + subject.call(account) + account.reload + + expect(account.secret).to be_blank + expect(account.subscription_expires_at).to be_blank + end + + it 'logs error on subscription failure' do + logger = stub_logger + stub_request(:post, 'http://hub.example.com/').to_return(status: 404) + subject.call(account) + + expect(logger).to have_received(:debug).with(/unsubscribe for bob@example.com failed/) + end + + it 'logs error on connection failure' do + logger = stub_logger + stub_request(:post, 'http://hub.example.com/').to_raise(HTTP::Error) + subject.call(account) + + expect(logger).to have_received(:debug).with(/PuSH subscription request for bob@example.com could not be made due to HTTP or SSL error/) + end + + def stub_logger + double(debug: nil).tap do |logger| + allow(Rails).to receive(:logger).and_return(logger) + end + end +end |