Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ios - if one DeviceToken are wrong no push are sent #713

Open
jsundstr opened this issue May 31, 2016 · 21 comments
Open

ios - if one DeviceToken are wrong no push are sent #713

jsundstr opened this issue May 31, 2016 · 21 comments

Comments

@jsundstr
Copy link

v.4.0.10.0

Problem
If I send a batch of notifications and one of the DeviceToken are wrong the correct ones dont sends,

Try it
send a batch with two DeviceToken one wrong and one correct, and you dont get the correct pushnotification either

Exception
2016-05-31 17:18:14. [DEBUG] Scaled Changed to: 1
2016-05-31 17:18:14. [INFO] Stopping: Waiting on Tasks
2016-05-31 17:18:14. [INFO] Waiting on all tasks 2
2016-05-31 17:18:15. [INFO] APNS-Client[1]: Sending Batch ID=1, Count=2
2016-05-31 17:18:15. [ERROR] APNS-CLIENT[1]: Send Batch Error: Batch ID=1, Error=PushSharp.Core.NotificationException: Invalid DeviceToken
at PushSharp.Apple.ApnsNotification.ToBytes()
at PushSharp.Apple.ApnsConnection.createBatch(List`1 toSend)
at PushSharp.Apple.ApnsConnection.d__21.MoveNext()
2016-05-31 17:18:15. [INFO] APNS-Client[1]: Sent Batch, waiting for possible response...
2016-05-31 17:18:15. [ERROR] APNS-Client[1]: Reader Exception: System.NullReferenceException: Object reference not set to an instance of an object.
at PushSharp.Apple.ApnsConnection.d__23.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task)
at PushSharp.Apple.ApnsConnection.d__21.MoveNext()
2016-05-31 17:18:15. [INFO] APNS-Client[1]: Done Reading for Batch ID=1, reseting batch timer...
Apple Notification Failed: ID=1, Code=ConnectionError
Apple Notification Failed: ID=2, Code=ConnectionError
2016-05-31 17:18:15. [INFO] All Tasks Finished
2016-05-31 17:18:15. [INFO] Passed WhenAll
2016-05-31 17:18:15. [INFO] Broker IsCompleted
2016-05-31 17:18:15. [DEBUG] Broker Task Ended
2016-05-31 17:18:15. [INFO] Stopping: Done Waiting on Tasks

Code
// Wire up events
apnsBroker.OnNotificationFailed += (notification, aggregateEx) =>
{
string error = string.Empty;
aggregateEx.Handle(ex =>
{

                            //See what kind of exception it was to further diagnose
                            if (ex is ApnsNotificationException)
                            {
                                var notificationException = (ApnsNotificationException)ex;

                                //Deal with the failed notification
                                var apnsNotification = notificationException.Notification;
                                var statusCode = notificationException.ErrorStatusCode;

                                error = $"Apple Notification Failed: ID={apnsNotification.Identifier}, Code={statusCode}";
                                upd_failed(notification, error);
                                Console.WriteLine(error);

                            }
                            else
                            {
                                //Inner exception might hold more useful information like an ApnsConnectionException 
                                error = $"Apple Notification Failed for some unknown reason : {ex.InnerException}";
                                upd_failed(notification, error);
                                Console.WriteLine(error);
                            }

                            // Mark it as handled
                            return true;
                        });
                    };
                    apnsBroker.OnNotificationSucceeded += (notification) =>
                    {
                        Console.WriteLine("Apple Notification Sent!");
                        upd_sent(notification);
                    };


                    // Start the broker
                    apnsBroker.Start();

                foreach (XmlNode node_ios in list_ios)
                {

                        string device_token = node_ios.Attributes.GetNamedItem("device_token").Value;
                        string json = jsonAPNS(node_ios);

                        apnsBroker.QueueNotification(new ApnsNotification
                        {
                            DeviceToken = device_token,
                            Payload = JObject.Parse(json)
                        });

                }
                    // Stop the broker, wait for it to finish   
                    // This isn't done after every message, but after you're
                    // done with the broker
                    apnsBroker.Stop();
@bernik1980
Copy link

Can confirm that, did cost me some time some days ago.

Sending one push after another now until this is fixed.

@jsundstr
Copy link
Author

Same for me sending one push at time, very time consuming...

@JulianPasque
Copy link

Same problem here.

@rid00z
Copy link

rid00z commented Jun 27, 2016

How do you send one at time rather than using the queue?

@jsundstr
Copy link
Author

I,m on vaccation. Back on July 11th

Regards,
Johan Sundström

@JulianPasque
Copy link

I can't access my code at the moment. But I think there was a method SetScale to send one at time.

@bernik1980
Copy link

Any news on this? Its really a show-stopper. Sending 150 notifications to iOS devices will take minutes sending one after another.

@wmlai29
Copy link

wmlai29 commented Sep 23, 2016

Any news on this?
(>"<) Same problem here.

@sasirekhak
Copy link

Any updates on this?? I have Same problem

@h0useRus
Copy link

Hi, we faced with same Invalid Token problem, please help with it

@alr
Copy link

alr commented Oct 28, 2016

Same problem, any news on this?

@petr-pokorny-1
Copy link

petr-pokorny-1 commented Nov 9, 2016

I am using the following workaround:

var config = new ApnsConfiguration(eApnsEnvironment, certificatePath, apnsCertPassword);
// This is a temporary solution for this issue:
// https://github.com/Redth/PushSharp/issues/713
config.InternalBatchSize = 1;

Which bypases the Apns connection batches and sends notifications one by one.

The problem is caused by the following code in ApnsConnection.cs, line 160. Foreach loop sets all items in the batch failed.

       ...
            } catch (Exception ex) {
                Log.Error ("APNS-CLIENT[{0}]: Send Batch Error: Batch ID={1}, Error={2}", id, batchId, ex);
                foreach (var n in toSend)
                    n.CompleteFailed (new ApnsNotificationException (ApnsNotificationErrorStatusCode.ConnectionError, n.Notification, ex));
            }
     ...

@lepertFLA
Copy link

lepertFLA commented Nov 17, 2016

Looking at the current Reader code.. it does send the InvalidToken exception to us and then re-enqueues ALL the ones AFTER the failed one in the batch... and thus tries the later ones again. I found setting the batch size to 1 is not a good solution for a system that needs to send many requests at once. Still trying to see what the root issue is...

I did change the code in the connection send.... to handle a bad socket and retry better... not sure if this helps

                    bool isConnected = true;
                    try {
                        // See if we need to connect
                        if (!socketCanWrite () || i > 0)
                            await connect ();
                    }
                    catch (Exception ex) when (i != Configuration.InternalBatchFailureRetryCount)
                    {
                        isConnected = false;
                        Log.Info("APNS-CLIENT[{0}]: Socket Connection Exception Occurred, Retrying Batch: Batch ID={1}, Error={2}", id, batchId, ex);
                    }
                    finally {
                        connectingSemaphore.Release ();
                    }
                    if(isConnected)
                    {
                        try
                        {
                            await networkStream.WriteAsync(data, 0, data.Length).ConfigureAwait(false);
                            break;
                        }
                        catch (Exception ex) when (i != Configuration.InternalBatchFailureRetryCount)
                        {
                            Log.Info("APNS-CLIENT[{0}]: Retrying Batch: Batch ID={1}, Error={2}", id, batchId, ex);
                        }
                    }

@ShalvaHMO
Copy link

ShalvaHMO commented Feb 6, 2017

This is a temporary change to code for sending all Succeed Notifications in batch.
Works for me. ApnsConnection.cs, line 160

            catch (Exception ex) {
            Log.Error ("APNS-CLIENT[{0}]: Send Batch Error: Batch ID={1}, Error={2}", id, batchId, ex);
            #region Temporary CHANGES
            var errorNotificationToSend = new List<CompletableApnsNotification>();
            foreach (var notificationItem in toSend)
            {
                if (!notificationItem.Notification.IsDeviceRegistrationIdValid())
                {
                    errorNotificationToSend.Add(notificationItem);
                }
                else
                {
                    notifications.Enqueue(notificationItem);
                }
            }
            #endregion Temporary CHANGES

            foreach (var n in errorNotificationToSend)
            {
                n.CompleteFailed(new ApnsNotificationException(ApnsNotificationErrorStatusCode.ConnectionError, n.Notification, ex));
            }

@jekcom
Copy link

jekcom commented May 21, 2017

same here

@hidim
Copy link

hidim commented Jul 21, 2017

Still, there are no updates :(

@idealz
Copy link

idealz commented Sep 14, 2017

for the time being i am just doing a check on each device before adding it to queue
var notification = new ApnsNotification
{
DeviceToken = device.DeviceToken,
Payload = JObject.Parse(JsonConvert.SerializeObject(jsonObject))
};
try
{
var bytes = notification.ToBytes();
if (bytes.Length > 0)
{
apnsBroker.QueueNotification(notification);
}
}
catch
{
_appDeviceService.InvalidateToken(device.DeviceToken);
}

@umutbebek
Copy link

umutbebek commented Nov 24, 2017

Opps. I tried to open a lot of brokers to APN but it seems it blocks you (your IP etc) if you that. So do not try to open a lot of connections :)

I simply removed invalidTokens at once from db if encounter any, and set config.InternalBatchSize to 99. That seems to solve to problem and sended a thousand notifications in instant. I would update here later if our user count increases :)

@AndrewFahlgren
Copy link

I made a pull request to fix this issue. If you are still experiencing this issue you can try my code to fix it.

pull request: #877

@kalpesh-jariwala
Copy link

kalpesh-jariwala commented Feb 17, 2021

If I send a batch of notifications and one of the DeviceToken are wrong the correct ones dont sends. . I am using demo code of push sharp for batch of notifications. example, I am send 100 device tokens and there is in the list first and last are correct. so i am getting only first pushnotification and last(100) not getting. Please let me know what was the solution of this problem.

@AndrewFahlgren
Copy link

@kalpesh-jariwala #885 was created to solve this issue. I am not sure why it hasn't been merged but you could download or fork https://github.com/chillNZ/PushSharp and build it yourself

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests