From 8b43d6bf9c3c85b093ae9581d7400776aaa97322 Mon Sep 17 00:00:00 2001 From: Sorin Davidoi Date: Wed, 26 Jul 2017 16:14:39 +0200 Subject: fix(web_push_notification_worker): Guard against deleted notifications (#4379) --- app/workers/web_push_notification_worker.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'app/workers') diff --git a/app/workers/web_push_notification_worker.rb b/app/workers/web_push_notification_worker.rb index da4043ddb..51b8daae7 100644 --- a/app/workers/web_push_notification_worker.rb +++ b/app/workers/web_push_notification_worker.rb @@ -9,6 +9,8 @@ class WebPushNotificationWorker session_activation = SessionActivation.find(session_activation_id) notification = Notification.find(notification_id) + return if session_activation.nil? || notification.nil? + begin session_activation.web_push_subscription.push(notification) rescue Webpush::InvalidSubscription, Webpush::ExpiredSubscription => e -- cgit From 994d948c3947b75140c2a2a7de5b81713c7b8ea3 Mon Sep 17 00:00:00 2001 From: Clworld Date: Thu, 27 Jul 2017 07:38:20 +0900 Subject: Add callback_url/acct information for Sidekiq PuSH workers Exception. (#4281) * Add destination informations to exception on SubscribeWorker and DeliveryWorker. * Simplify delivery error message. * Prevent changing Exception type... * fix typo. --- app/lib/exceptions.rb | 10 +++++----- app/workers/pubsubhubbub/delivery_worker.rb | 2 ++ app/workers/pubsubhubbub/subscribe_worker.rb | 2 ++ 3 files changed, 9 insertions(+), 5 deletions(-) (limited to 'app/workers') diff --git a/app/lib/exceptions.rb b/app/lib/exceptions.rb index 34d84a34f..b2489711d 100644 --- a/app/lib/exceptions.rb +++ b/app/lib/exceptions.rb @@ -8,11 +8,11 @@ module Mastodon class UnexpectedResponseError < Error def initialize(response = nil) - @response = response - end - - def to_s - "#{@response.uri} returned code #{@response.code}" + if response.respond_to? :uri + super("#{response.uri} returned code #{response.code}") + else + super + end end end end diff --git a/app/workers/pubsubhubbub/delivery_worker.rb b/app/workers/pubsubhubbub/delivery_worker.rb index 035a59048..88645cf33 100644 --- a/app/workers/pubsubhubbub/delivery_worker.rb +++ b/app/workers/pubsubhubbub/delivery_worker.rb @@ -16,6 +16,8 @@ class Pubsubhubbub::DeliveryWorker @subscription = Subscription.find(subscription_id) @payload = payload process_delivery unless blocked_domain? + rescue => e + raise e.class, "Delivery failed for #{subscription&.callback_url}: #{e.message}" end private diff --git a/app/workers/pubsubhubbub/subscribe_worker.rb b/app/workers/pubsubhubbub/subscribe_worker.rb index 6865e7136..9178079d4 100644 --- a/app/workers/pubsubhubbub/subscribe_worker.rb +++ b/app/workers/pubsubhubbub/subscribe_worker.rb @@ -22,5 +22,7 @@ class Pubsubhubbub::SubscribeWorker account = Account.find(account_id) logger.debug "PuSH re-subscribing to #{account.acct}" ::SubscribeService.new.call(account) + rescue => e + raise e.class, "Subscribe failed for #{account&.acct}: #{e.message}" end end -- cgit From 6e186b9c77e7f23da6b46b901aace1c0b3b163ad Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 28 Jul 2017 17:21:14 +0200 Subject: When PuSH subscribe attempts are exhausted, unsubscribe (#4422) --- app/workers/pubsubhubbub/subscribe_worker.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'app/workers') diff --git a/app/workers/pubsubhubbub/subscribe_worker.rb b/app/workers/pubsubhubbub/subscribe_worker.rb index 9178079d4..7560c2671 100644 --- a/app/workers/pubsubhubbub/subscribe_worker.rb +++ b/app/workers/pubsubhubbub/subscribe_worker.rb @@ -3,7 +3,7 @@ class Pubsubhubbub::SubscribeWorker include Sidekiq::Worker - sidekiq_options queue: 'push', retry: 10, unique: :until_executed + sidekiq_options queue: 'push', retry: 10, unique: :until_executed, dead: false sidekiq_retry_in do |count| case count @@ -18,6 +18,12 @@ class Pubsubhubbub::SubscribeWorker end end + sidekiq_retries_exhausted do |msg, _e| + account = Account.find(msg['args'].first) + logger.error "PuSH subscription attempts for #{account.acct} exhausted. Unsubscribing" + ::UnsubscribeService.new.call(account) + end + def perform(account_id) account = Account.find(account_id) logger.debug "PuSH re-subscribing to #{account.acct}" -- cgit From 4e2f2fab73d6632a2c5f8ff3d5ba26140747cd60 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 28 Jul 2017 17:21:28 +0200 Subject: Fix guard clause in WebPushNotificationWorker (#4421) --- app/workers/web_push_notification_worker.rb | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) (limited to 'app/workers') diff --git a/app/workers/web_push_notification_worker.rb b/app/workers/web_push_notification_worker.rb index 51b8daae7..eacea04c3 100644 --- a/app/workers/web_push_notification_worker.rb +++ b/app/workers/web_push_notification_worker.rb @@ -7,18 +7,19 @@ class WebPushNotificationWorker def perform(session_activation_id, notification_id) session_activation = SessionActivation.find(session_activation_id) - notification = Notification.find(notification_id) + notification = Notification.find(notification_id) - return if session_activation.nil? || notification.nil? + return if session_activation.web_push_subscription.nil? || notification.activity.nil? - begin - session_activation.web_push_subscription.push(notification) - rescue Webpush::InvalidSubscription, Webpush::ExpiredSubscription => e - # Subscription expiration is not currently implemented in any browser - session_activation.web_push_subscription.destroy! - session_activation.update!(web_push_subscription: nil) + session_activation.web_push_subscription.push(notification) + rescue Webpush::InvalidSubscription, Webpush::ExpiredSubscription + # Subscription expiration is not currently implemented in any browser - raise e - end + session_activation.web_push_subscription.destroy! + session_activation.update!(web_push_subscription: nil) + + true + rescue ActiveRecord::RecordNotFound + true end end -- cgit From 3e7a541e09ac875be21bea3cf8a444e3228d2a4c Mon Sep 17 00:00:00 2001 From: Yamagishi Kazutoshi Date: Mon, 31 Jul 2017 22:19:13 +0900 Subject: Change RuboCop rules to loose (#4464) --- .rubocop.yml | 9 +++++---- app/lib/emoji.rb | 2 +- app/services/batched_remove_status_service.rb | 2 +- app/workers/pubsubhubbub/distribution_worker.rb | 4 ++-- 4 files changed, 9 insertions(+), 8 deletions(-) (limited to 'app/workers') diff --git a/.rubocop.yml b/.rubocop.yml index 1cbdadd49..ae3697174 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -27,6 +27,7 @@ Metrics/AbcSize: Max: 100 Metrics/BlockLength: + Max: 35 Exclude: - 'lib/tasks/**/*' @@ -35,10 +36,10 @@ Metrics/BlockNesting: Metrics/ClassLength: CountComments: false - Max: 200 + Max: 300 Metrics/CyclomaticComplexity: - Max: 15 + Max: 25 Metrics/LineLength: AllowURI: true @@ -53,11 +54,11 @@ Metrics/ModuleLength: Max: 200 Metrics/ParameterLists: - Max: 4 + Max: 5 CountKeywordArgs: true Metrics/PerceivedComplexity: - Max: 10 + Max: 20 Rails: Enabled: true diff --git a/app/lib/emoji.rb b/app/lib/emoji.rb index 6de70e9b4..45b7f53de 100644 --- a/app/lib/emoji.rb +++ b/app/lib/emoji.rb @@ -6,7 +6,7 @@ class Emoji include Singleton def initialize - data = Oj.load(File.open(File.join(Rails.root, 'lib', 'assets', 'emoji.json'))) + data = Oj.load(File.open(Rails.root.join('lib', 'assets', 'emoji.json'))) @map = {} diff --git a/app/services/batched_remove_status_service.rb b/app/services/batched_remove_status_service.rb index b462154ae..ab810c628 100644 --- a/app/services/batched_remove_status_service.rb +++ b/app/services/batched_remove_status_service.rb @@ -90,7 +90,7 @@ class BatchedRemoveStatusService < BaseService key = FeedManager.instance.key(:home, follower_id) originals = statuses.reject(&:reblog?) - reblogs = statuses.reject { |s| !s.reblog? } + reblogs = statuses.select(&:reblog?) # Quickly remove all originals redis.pipelined do diff --git a/app/workers/pubsubhubbub/distribution_worker.rb b/app/workers/pubsubhubbub/distribution_worker.rb index ce467d18b..ea246128d 100644 --- a/app/workers/pubsubhubbub/distribution_worker.rb +++ b/app/workers/pubsubhubbub/distribution_worker.rb @@ -14,7 +14,7 @@ class Pubsubhubbub::DistributionWorker @subscriptions = active_subscriptions.to_a distribute_public!(stream_entries.reject(&:hidden?)) - distribute_hidden!(stream_entries.reject { |s| !s.hidden? }) + distribute_hidden!(stream_entries.select(&:hidden?)) end private @@ -35,7 +35,7 @@ class Pubsubhubbub::DistributionWorker @payload = OStatus::AtomSerializer.render(OStatus::AtomSerializer.new.feed(@account, stream_entries)) @domains = @account.followers.domains - Pubsubhubbub::DeliveryWorker.push_bulk(@subscriptions.reject { |s| !allowed_to_receive?(s.callback_url, s.domain) }) do |subscription| + Pubsubhubbub::DeliveryWorker.push_bulk(@subscriptions.select { |s| allowed_to_receive?(s.callback_url, s.domain) }) do |subscription| [subscription.id, @payload] end end -- cgit