From 978cc0cfca1100391259a5011e36a7a5bd61fcbd Mon Sep 17 00:00:00 2001
From: Gerhard Schlager <mail@gerhard-schlager.at>
Date: Thu, 10 Jan 2019 15:38:31 +0100
Subject: [PATCH] FIX: Remove expired subscription for push notifications

All other errors get logged, but do not stop the system from sending further push notifications.
---
 app/services/push_notification_pusher.rb | 52 ++++++++++++++----------
 1 file changed, 31 insertions(+), 21 deletions(-)

diff --git a/app/services/push_notification_pusher.rb b/app/services/push_notification_pusher.rb
index d397e13039e..3a633788652 100644
--- a/app/services/push_notification_pusher.rb
+++ b/app/services/push_notification_pusher.rb
@@ -2,25 +2,25 @@ require_dependency 'webpush'
 
 class PushNotificationPusher
   def self.push(user, payload)
+    message = {
+      title: I18n.t(
+        "discourse_push_notifications.popup.#{Notification.types[payload[:notification_type]]}",
+        site_title: SiteSetting.title,
+        topic: payload[:topic_title],
+        username: payload[:username]
+      ),
+      body: payload[:excerpt],
+      badge: get_badge,
+      icon: ActionController::Base.helpers.image_url("push-notifications/#{Notification.types[payload[:notification_type]]}.png"),
+      tag: "#{Discourse.current_hostname}-#{payload[:topic_id]}",
+      base_url: Discourse.base_url,
+      url: payload[:post_url],
+      hide_when_active: true
+    }
+
     subscriptions(user).each do |subscription|
       subscription = JSON.parse(subscription.data)
-
-      message = {
-        title: I18n.t(
-          "discourse_push_notifications.popup.#{Notification.types[payload[:notification_type]]}",
-          site_title: SiteSetting.title,
-          topic: payload[:topic_title],
-          username: payload[:username]
-        ),
-        body: payload[:excerpt],
-        badge: get_badge,
-        icon: ActionController::Base.helpers.image_url("push-notifications/#{Notification.types[payload[:notification_type]]}.png"),
-        tag: "#{Discourse.current_hostname}-#{payload[:topic_id]}",
-        base_url: Discourse.base_url,
-        url: payload[:post_url],
-        hide_when_active: true
-      }
-      send_notification user, subscription, message
+      send_notification(user, subscription, message)
     end
   end
 
@@ -54,7 +54,7 @@ class PushNotificationPusher
         tag: "#{Discourse.current_hostname}-subscription"
       }
 
-      send_notification user, subscription, message
+      send_notification(user, subscription, message)
     end
   end
 
@@ -74,7 +74,7 @@ class PushNotificationPusher
 
   def self.send_notification(user, subscription, message)
     begin
-      response = Webpush.payload_send(
+      Webpush.payload_send(
         endpoint: subscription["endpoint"],
         message: message.to_json,
         p256dh: subscription.dig("keys", "p256dh"),
@@ -85,8 +85,18 @@ class PushNotificationPusher
           private_key: SiteSetting.vapid_private_key
         }
       )
-    rescue Webpush::InvalidSubscription => e
-      unsubscribe user, subscription
+    rescue Webpush::ExpiredSubscription
+      unsubscribe(user, subscription)
+    rescue Webpush::ResponseError => e
+      Discourse.warn_exception(
+        e,
+        message: "Failed to send push notification",
+        env: {
+          user_id: user.id,
+          endpoint: subscription["endpoint"],
+          message: message.to_json
+        }
+      )
     end
   end
 end