From 43b4f2e20b68fb9df4cf23c0e0bb9bddc2a1a570 Mon Sep 17 00:00:00 2001 From: Kyle McNamara Date: Mon, 3 Sep 2018 13:07:37 +1200 Subject: [PATCH 1/3] Updated ApnsConnection createBatch Fixed one ApnsNotification failing validation in ToBytes() causing the whole batch to fail - now triggers CompleteFailed for the notifications that fail validation, skips them and continues to send the rest. --- PushSharp.Apple/ApnsConnection.cs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/PushSharp.Apple/ApnsConnection.cs b/PushSharp.Apple/ApnsConnection.cs index 371151b3..ebf41b4c 100644 --- a/PushSharp.Apple/ApnsConnection.cs +++ b/PushSharp.Apple/ApnsConnection.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.IO; using System.Net.Sockets; using System.Net.Security; @@ -185,9 +185,20 @@ byte[] createBatch (List toSend) var batchData = new List (); // Add all the frame data - foreach (var n in toSend) - batchData.AddRange (n.Notification.ToBytes ()); - + foreach (var n in toSend) { + try { + batchData.AddRange(n.Notification.ToBytes ()); + } catch (NotificationException ex) { + // If a notification fails validation, simply fail it and skip + Log.Info("APNS-CLIENT[{0}]: Notification failed validation, failing notification {1}. Batch ID={2}, Error={3}", id, n.Notification.Identifier, batchId, ex); + + // Raise the failed event for this notification so the caller knows it wasn't sent + n.CompleteFailed(new ApnsNotificationException(ApnsNotificationErrorStatusCode.ConnectionError, n.Notification, ex)); + + // Keep adding the other notifications to the batch data to send as we still want to send the others (provided they pass validation themselves) + } + } + return batchData.ToArray (); } From bc2b0eccd30d98ecddefc96a3d504dec70b73a7f Mon Sep 17 00:00:00 2001 From: Kyle McNamara Date: Mon, 3 Sep 2018 15:15:39 +1200 Subject: [PATCH 2/3] Fixed bug in ApnsConnection Fixed bug introduced in commit 43b4f2e20b68fb9df4cf23c0e0bb9bddc2a1a570 where the notifications that failed validation were being added to the list of sent notifications, which causes the response processing code to attempt to CompleteSuccessfully them when another notification is returned from apple with a failure. --- PushSharp.Apple/ApnsConnection.cs | 32 ++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/PushSharp.Apple/ApnsConnection.cs b/PushSharp.Apple/ApnsConnection.cs index ebf41b4c..8cc1eb39 100644 --- a/PushSharp.Apple/ApnsConnection.cs +++ b/PushSharp.Apple/ApnsConnection.cs @@ -153,14 +153,24 @@ async Task SendBatch () } } - foreach (var n in toSend) - sent.Add (new SentNotification (n)); + foreach (var n in toSend) { + if (n.State == NotificationState.Failed) + continue; + + sent.Add(new SentNotification(n)); + } } } catch (Exception ex) { Log.Error ("APNS-CLIENT[{0}]: Send Batch Error: Batch ID={1}, Error={2}", id, batchId, ex); - foreach (var n in toSend) + foreach (var n in toSend) { + // If the notification has already failed, don't fail it again - we want the original error intact (and CompleteFailed raises an exception if called twice on the same notification) + if (n.State == NotificationState.Failed) + continue; + + // Fail the notification. n.CompleteFailed (new ApnsNotificationException (ApnsNotificationErrorStatusCode.ConnectionError, n.Notification, ex)); + } } Log.Info ("APNS-Client[{0}]: Sent Batch, waiting for possible response...", id); @@ -184,16 +194,17 @@ byte[] createBatch (List toSend) var batchData = new List (); + List failedValidation = new List(); // Add all the frame data foreach (var n in toSend) { try { - batchData.AddRange(n.Notification.ToBytes ()); + batchData.AddRange (n.Notification.ToBytes ()); } catch (NotificationException ex) { // If a notification fails validation, simply fail it and skip Log.Info("APNS-CLIENT[{0}]: Notification failed validation, failing notification {1}. Batch ID={2}, Error={3}", id, n.Notification.Identifier, batchId, ex); // Raise the failed event for this notification so the caller knows it wasn't sent - n.CompleteFailed(new ApnsNotificationException(ApnsNotificationErrorStatusCode.ConnectionError, n.Notification, ex)); + n.CompleteFailed (new ApnsNotificationException (ApnsNotificationErrorStatusCode.ConnectionError, n.Notification, ex)); // Keep adding the other notifications to the batch data to send as we still want to send the others (provided they pass validation themselves) } @@ -444,6 +455,8 @@ public CompletableApnsNotification (ApnsNotification notification) public ApnsNotification Notification { get; private set; } + public NotificationState State { get; private set; } + TaskCompletionSource completionSource; public Task WaitForComplete () @@ -454,14 +467,23 @@ public Task WaitForComplete () public void CompleteSuccessfully () { completionSource.SetResult (null); + State = NotificationState.Successful; } public void CompleteFailed (Exception ex) { completionSource.SetResult (ex); + State = NotificationState.Failed; } } + public enum NotificationState + { + Pending, + Successful, + Failed + } + /// /// Using IOControl code to configue socket KeepAliveValues for line disconnection detection(because default is toooo slow) /// From 65856e8667ce35ccd4ee1c3958311ba1857a2424 Mon Sep 17 00:00:00 2001 From: Kyle McNamara Date: Mon, 3 Sep 2018 15:19:11 +1200 Subject: [PATCH 3/3] Added minor comments to ApnsConnection --- PushSharp.Apple/ApnsConnection.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/PushSharp.Apple/ApnsConnection.cs b/PushSharp.Apple/ApnsConnection.cs index 8cc1eb39..689968c5 100644 --- a/PushSharp.Apple/ApnsConnection.cs +++ b/PushSharp.Apple/ApnsConnection.cs @@ -154,6 +154,7 @@ async Task SendBatch () } foreach (var n in toSend) { + // If the notification failed validation before being sent (i.e. it was skipped) if (n.State == NotificationState.Failed) continue; @@ -164,7 +165,8 @@ async Task SendBatch () } catch (Exception ex) { Log.Error ("APNS-CLIENT[{0}]: Send Batch Error: Batch ID={1}, Error={2}", id, batchId, ex); foreach (var n in toSend) { - // If the notification has already failed, don't fail it again - we want the original error intact (and CompleteFailed raises an exception if called twice on the same notification) + // If the notification has already failed, don't fail it again (it's possible it failed validation already) + // - we want the original error intact (and CompleteFailed raises an exception if called twice on the same notification) if (n.State == NotificationState.Failed) continue; @@ -194,7 +196,6 @@ byte[] createBatch (List toSend) var batchData = new List (); - List failedValidation = new List(); // Add all the frame data foreach (var n in toSend) { try {