Added additional error handling for invalid apns device registration ids - #877
Added additional error handling for invalid apns device registration ids#877AndrewFahlgren wants to merge 3 commits into
Conversation
…ids. Now when an error occurs during connection we are checking each notification for a bad decive registration id and if we find any we requeue the notifications and send InvalidToken errors for each one.
vladgaman
commented
Jul 10, 2018
This is not bad, but then again this method IsDeviceRegistrationIdValid() all is doing is checking it with a very simple regex: @"^[0-9A-F]+$" and this means if i have this registration id "AAA" then it will pass the regex thinking is a valid id, but in reality it is not. This method IsDeviceRegistrationIdValid() would have been a lot more useful if it actually checked if the registration id is an existing one. |
Agreed with @vladgaman - this is an inadequate solution due to the fact ApnsNotification::ToBytes can raise a NotificationException for a number of reasons that aren't due to the regex failing such as:
In the cases above this code will get stuck in an infinite loop, attempting to resend the batch and failing over and over again. I've made a pull request #885 which simply handles the NotificationException in createBatch, calling the fail event and skipping the notifications which fail any of the possible validation exceptions from ToBytes, which avoids this issue. |
Update fork from main repository
Now when an error occurs during connection we are checking each notification for a bad device registration id and if we find any we re-queue the notifications and send InvalidToken errors for each one.
This code was adapted from code written by @ShalvaHMO here: #713
This code was written to solve issue: #713