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

Users receiving duplicate notifications #123

Closed
ibushong opened this issue May 29, 2015 · 21 comments
Closed

Users receiving duplicate notifications #123

ibushong opened this issue May 29, 2015 · 21 comments

Comments

@ibushong
Copy link

I'm not sure how this is possible because I thought Apple prevented this...but for some reason, after implementing this library, users are receiving multiple instances of push notifications.

In my first implementation, I looped over a few thousand users, using the enhanced setting. A bunch of our users got hundreds of repeating notifications and had to uninstall the app to stop it. Not a good experience. Looking through the logs, it appears that when PyAPNs went back to re-send dropped notifications (due to an invalid token earlier on), it resends notifications that were already sent. Now this is probably due to some implementation error of mine instead of a library issue. But everything seems pretty straightforward, so I'm not sure what could be causing this.

Since then, I've broken up our mass pushes into smaller chucks, but still some users report getting 2-3 duplicate notifications.

Has anyone else experienced this? Any tips for debugging this? (Hard to reproduce for debugging because I need a large audience and that only happens in production)

I guess I could always switch to the non-enhanced versions right? That would definitely prevent this issue, with the tradeoff of taking longer (due to the blocking socket).

@jimhorng
Copy link
Collaborator

@ibushong,

  1. Can you please paste the code snippet of how to use PyAPNs, as completed as possible, so that folks here can troubleshoot easier.

  2. Yes, you can switch to non-enhance mode anytime :)

@tigerhy1
Copy link

tigerhy1 commented Jun 9, 2015

I experience the the same situation. and my code like:

message = Payload(
    alert = alert,
    badge = badge,
    sound = sound,
    custom = coustomDefined)

self.apns.gateway_server.send_notification(token_hex, message)

and user received hundreds of duplicated messages.
I think there are some wrong with my implementation, who can explain it.

@jimhorng
Copy link
Collaborator

jimhorng commented Jun 9, 2015

@tigerhy1 ,

  1. Could you provide more code snippet of how APNS object is instantiate?
  2. Do you use enhanced mode?
    If so, do you have unique identifier for each message? as refer to document
    Thanks :)

@ibushong
Copy link
Author

ibushong commented Jun 9, 2015

Sorry for the delay. Here is my code:

def send_ios_push_notification(self, tokens, msg, custom_info=None, sound='default', badge=1):
        if custom_info is None:
            custom_info = {}
        use_sandbox = getenv() != "production"

        apns = APNs(use_sandbox=use_sandbox, cert_file=self.cert, enhanced=True)
        apns.gateway_server.register_response_listener(push_response_listener)

        payload = Payload(alert=msg, badge=badge, sound=sound, custom=custom_info)

        # Convert tokens to hex (some are stored in base64) and make sure they are valid
        hex_tokens = []
        for token in tokens:
            try:
                # Determine if token is hex or base64
                int(token, 16)
            except ValueError:
                hex_token = base64.decodestring(token).encode("hex")
            else:
                hex_token = token

            hex_tokens.append(hex_token)

        expires = time.time() + 60 * 60 * 24
        for hex_token in hex_tokens:
            identifier = random.getrandbits(32)
            apns.gateway_server.send_notification(hex_token, payload, identifier, expires)

Note that initially, I set had been setting the identifier to 0, since I didn't care about identifying dropped messages. But then after looking at the PyAPNs source, I saw that the identifier was used internally to figure out which messages to resend, so I could definitely see this being the reason for repeat notifications (IMO, this should be make clearer in the documentation).

Additionally, I realized I can't used non-enhanced mode, because then I have to wait a few seconds between notifications to see if it was successful or not. Or else I will loose a bunch of notifications I send behind a failed one. Right?

Basically I just want to send a lot of notifications, but I don't care about re-trying failed ones. It's really annoying that Apple drops the connection on a failed push. I just want it to silently ignore them.

@jimhorng
Copy link
Collaborator

jimhorng commented Jun 9, 2015

@ibushong

I saw that the identifier was used internally to figure out which messages to resend, so I could definitely see this being the reason for repeat notifications (IMO, this should be make clearer in the documentation).

Yes, you are right, while the document might states clearer, the current statement is
"Send a notification. note that identifer is the information to indicate which message has error in error-response payload." which somehow indirectly indicate the message identifier should be unique or otherwise it cannot be identified.

Additionally, I realized I can't used non-enhanced mode, because then I have to wait a few seconds between notifications to see if it was successful or not. Or else I will loose a bunch of notifications I send behind a failed one. Right?

yes, in non-enhanced mode APNS will not send you error-response when failed.

Basically I just want to send a lot of notifications, but I don't care about re-trying failed ones. It's really annoying that Apple drops the connection on a failed push. I just want it to silently ignore them.

So you should still use enhanced mode, the re-send is NOT re-trying the failed ones, it is resending all notification sent between the last "failed" notification and the one before APNS drops the connection, which are all dropped silently.

@tigerhy1
Copy link

I guess I know the reason why this happens:
I ain't using identifier for message, and I use enhanced mode:

self.apns = APNs(use_sandbox=sandBox, cert_file=perm, enhanced=True)

question:
if enhanced=True, and using unique identifier for each message, there would be no duplicated messages for sure?

I feel that I should read through the source code to make sure no duplicated messages will be send.
And it's my fault that not reading the doc throughfully, but could the code add some constraints to protect the user from using it in the wrong way?

Thanks :)

@jimhorng
Copy link
Collaborator

@tigerhy1 ,

if enhanced=True, and using unique identifier for each message, there would be no duplicated messages for sure?

Yes, it should be, otherwise please fire a bug :)

but could the code add some constraints to protect the user from using it in the wrong way?

It's a good intention, however as my current knowledge, it can only check identifier's uniqueness on runtime during sending lots of message and might not trustworthy, and, it might not help much. Please provide good suggestion or PR for discussion :)

@ngonpham
Copy link

Follow @jimhorng instruction to create random identifier. However, there is another problem, APNS sometimes returns identifier 0, then cause duplicated messages again. So I temporarily fix by adding "identifier > 0" condition. Wonder if there is a better fix for this?

                            if 8 == command and identifier > 0: # there is error response from APNS
                                error_response = (status, identifier)
                                if self._apns_connection._response_listener:
                                    self._apns_connection._response_listener(Util.convert_error_response_to_dict(error_response))
                                _logger.info("got error-response from APNS:" + str(error_response))
                                self._apns_connection._disconnect()
                                self._resend_notifications_by_id(identifier)

@jimhorng
Copy link
Collaborator

@ngonpham ,
BTW, can you deterministically reproduce the scenario that APNS returns identifier 0 when you never specify a identifier=0 message to it? if so, maybe its a bug from APNS can the library needs to workaround through it :(

@ngonpham
Copy link

@jimhorng Yes, always return in my case. (8, 8, 0) are returned for

command, status, identifier = unpack(ERROR_RESPONSE_FORMAT, buff)

@ibushong
Copy link
Author

@ngonpham excited to see some more data here! May I ask how you're testing/reproducing this? Are you using the sandbox or live server?

@ngonpham
Copy link

@ibushong I'm using live, here is the code

apns = APNs(use_sandbox=False, cert_file=..., key_file=..., enhanced=True)
apns.gateway_server.register_response_listener(invalid_token_listener)
try:
    identifier = random.getrandbits(32)
    ios_token[identifier] = reg_id
    apns.gateway_server.send_notification(reg_id, payload, identifier=identifier)
except Exception, e:
    logging.exception(e)

@ibushong
Copy link
Author

Is that running inside of a loop? Or you are able to reproduce it with only one push?

@ngonpham
Copy link

It ran in a loop @ibushong

@vhquang
Copy link

vhquang commented Nov 12, 2015

We ran into the same problem recently.
@jimhorng

can you deterministically reproduce the scenario that APNS returns identifier 0 when you never specify a identifier=0 message to it?

I think it is already set in the code:
https://github.com/djacobs/PyAPNs/blob/7df928626/apns.py#L520

So:
apns.gateway_server.send_notification(token, payload)
will have identifier default to 0, which is really bad in enhance mode.

@jimhorng
Copy link
Collaborator

@vhquang
Thanks for pointing this out and provided solution :)
AFAIK, identifier in apns world is for identifying which message is failed, so if library implicitly generate the identifier without letting user code to know, this is kind of meaningless that user never know which message failed, the same applies to current default value identifier=0.
So I am thinking maybe we should remove default value of identifier and force user to specify one for themselves?
However, timestamp is better of default identifier than 0 that I admitted, let's see @djacobs or other user/contributors thoughts :)

@vhquang
Copy link

vhquang commented Nov 13, 2015

I thought about set identifier as require parameter too. But it would be a breaking change for current users in non-enhance mode. Definitely interested in collecting more input from other people.

@djacobs
Copy link
Owner

djacobs commented Nov 13, 2015

I haven't looked deeply at this yet, but it feels like something that documentation could improve, no?

@vhquang
Copy link

vhquang commented Nov 13, 2015

@djacobs , I saw a PR for that, opened by @jimhorng some while ago #128

@pcompassion
Copy link

pcompassion commented Jul 13, 2017

I just had this problem with enhanced=True and unique identifier (I set identifier with django model's id field)

..... sad thing is I can't figure out why it happened...

I am using 3d40f97 commit.

@vhquang
Copy link

vhquang commented Jul 13, 2017

As far as I remember, enhanced mode send messages out in batch, and require each message to have unique identifier. When you have use set that identifier with django's model, it is possible that by the time you send out the message, your model is not saved yet. Thus you did not have the id by the time the message sent out. Try saving your model first.
Other than that, we would need more context on how you use the library.

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

7 participants