From f1a43f2319c0d9fde36ded04f839ecbd1e6448d7 Mon Sep 17 00:00:00 2001 From: Ted Johansson Date: Tue, 25 Jul 2023 15:01:02 +0800 Subject: [PATCH] DEV: Handle SSL errors in push notification pusher (#22771) We're seeing unhandled errors in production when web push notifications are failing with an SSL error. This is happening for a few users, but generating a large amount of log noise due to the sheer number of notifications. This adds handling of SSL errors in two places: 1. In FinalDestination::HTTP, this is handled the same as a timeout error, and gives a chance to recover. 2. In PushNotificationPusher. This will cause the notification to retry a number of times, and if it keeps failing, disable push notifications for the user. (Existing behaviour.) I wanted to wrap the SSL error in e.g. WebPush::RequestError, but the gem doesn't have request error handling, so didn't want to have the freedom patch diverge from the gem as well. Instead just propagating the raw SSL error. --- app/services/push_notification_pusher.rb | 2 ++ lib/final_destination/http.rb | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/services/push_notification_pusher.rb b/app/services/push_notification_pusher.rb index 736f3ea2bf6..0c6891d8591 100644 --- a/app/services/push_notification_pusher.rb +++ b/app/services/push_notification_pusher.rb @@ -157,6 +157,8 @@ class PushNotificationPusher end rescue Timeout::Error => e handle_generic_error(subscription, e, user, endpoint, message) + rescue OpenSSL::SSL::SSLError => e + handle_generic_error(subscription, e, user, endpoint, message) end end diff --git a/lib/final_destination/http.rb b/lib/final_destination/http.rb index ffceff99deb..4973b149a33 100644 --- a/lib/final_destination/http.rb +++ b/lib/final_destination/http.rb @@ -23,7 +23,7 @@ class FinalDestination::HTTP < Net::HTTP @open_timeout = remaining_time return super - rescue SystemCallError, Net::OpenTimeout => e + rescue OpenSSL::SSL::SSLError, SystemCallError, Net::OpenTimeout => e debug "[FinalDestination] Error connecting to #{ip}... #{e.message}" was_last_attempt = index == ips.length - 1 raise if was_last_attempt