Use new protocol, handles disconnects, uses timeout

[roncohen]

I've modified @adamri's proposal. It will now correctly handle the case where Apple does not respond to a message and it will reconnect when we get disconnected.

It will throw exceptions according to the newer APN protocol so you get immediate feedback.

[simonwhitaker]

Thanks both. Do either of you have any idea when (if ever) a provider might want to use simple format notifications? Apple's docs say "the enhanced format is recommended for most providers" but they don't expand on that. It does however imply that for some providers the simple format is still recommended, in which case this should be an option in PyAPNs.

[roncohen]

Hard to tell why you'd stick with the old format.

After applying this pull request, the code will behave significantly different from previously, effectively breaking backwards compatibility, e.g. can throw exceptions such as InvalidTokenError when send_notification is called. In this light, we should make it clear to people that if they upgrade, they are probably going to have to start catching some exceptions.

However, I do believe it's a very significant improvement over the current state. For one: As far as I can tell, if an invalid token is sent using the old protocol, the connection is still closed. Without this fix, it will fail when you try to send the next notification.

[simonwhitaker]

I agree that the enhanced format is an improvement, but as the simple format is still supported by Apple I think it would be remiss to remove it from PyAPNs. So ideally I'd like to see a parameter added to the constructor to control the format in use. (Defaulting enhanced mode to True would seem appropriate.)

Is that something you'd be interested in adding in?

[adamrimon]

My guess is that Apple supports the simple format for
backward-compatibility. I really can't see any reason a new server would
use it.

Anyway, adding a parameter makes the API less simple and clear. If
supporting the old method seems important to you, maybe we could add another method...?
send_simple_notification?

On Tue, Oct 23, 2012 at 5:55 PM, Simon Whitaker [email protected]:

I agree that the enhanced format is an improvement, but as the simple
format is still supported by Apple I think it would be remiss to remove it
from PyAPNs. So ideally I'd like to see a parameter added to the
constructor to control the format in use. (Defaulting enhanced mode to True
would seem appropriate.)

Is that something you'd be interested in adding in?

—
Reply to this email directly or view it on GitHubhttps://github.com/simonwhitaker/PyAPNs/pull/23#issuecomment-9707249.

[roncohen]

I agree with @adamri. Adding a method might be the way to go, but I'd rather see the old protocol ditched, for simplicity and conciseness.

[mrkhingston]

Thanks for the good work @simonwhitaker, @adamri and @roncohen. Not sure what's happening with resolving these pull requests and whether this pull request or @minorblend 's will be accepted. Personally I prefer this one as it doesn't require the caller to remember old pushes sent if it wants to re-send a failed push.

I forked and made a couple of changes to this part of the code which I think might be worth including in this pull request. See: https://github.com/vardr/PyAPNs/commit/9c114174063f49d5d6552bc671b7254e453a252b

Changed it to catch all exceptions so that no matter the write error, we will always reconnect before trying again. Specifically this simplifies my handling of recovery from connection errors / timeouts caused by intermittent network failures. (Without this, when the network comes back I just keep getting Broken Pipe errors from write() calls)

Also I believe that if an SSL error / timeout happens then the error should be re-raised so that the calling code is aware that it's notification has most likely not been sent and it should try to re-send it's notification. Seems possible that a message might be delivered to the Apple server and then the timeout happen before we receive a response - but for my use case it's much better to re-send the push and have it perhaps received twice rather than not received at all. Might be much the same for any caller that cares enough to re-send failed pushes.

[simonwhitaker]

Hey guys - apologies for leaving this so long. I don't use PyAPNs myself any longer, and I'm afraid I don't have the spare cycles to give it the attention it needs.

Would any of you be interested in taking over as the maintainer of this project?

[thetylerhayes]

Was this ever merged then?

[djacobs]

I can check this out shortly, thanks for the ping.

[Ethan-Zhang]

for non-blocking, we can use async apns client base on tornado tornado-apns

[ExplodingCabbage]

I think this was obsoleted by #71 and can be closed?