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

What's the correct way to use PushManager.shutdown()? #2

Closed
Raibaz opened this issue Sep 2, 2013 · 6 comments
Closed

What's the correct way to use PushManager.shutdown()? #2

Raibaz opened this issue Sep 2, 2013 · 6 comments
Labels

Comments

@Raibaz
Copy link

Raibaz commented Sep 2, 2013

README.md says "When you're done sending push notifications, make sure to shut down the PushManager", but there is no way to know when the sending is done, since enqueueing a notification and its actual sending aren't synchronous.

Shutting down the push manager immediately after enqueueing sometimes results in notifications not being sent, so I was wondering, what's the correct moment to invoke shutdown() and, in general, to use the PushManager?

I currently implemented a helper class that uses it as a singleton and basically never shuts it down, but I guess it would be better to shut it down when the notifications queue is empty and restart it when a new notification needs to be sent; however, the API doesn't have a method to get the queue status.

What am I missing about the "correct" way to use it, then?

@jchambers
Copy link
Owner

…it would be better to shut it down when the notifications queue is empty and restart it when a new notification needs to be sent.

Actually, Apple's docs suggest keeping the connection open even when you're not using it:

Keep your connections with APNs open across multiple notifications; don’t repeatedly open and close connections. APNs treats rapid connection and disconnection as a denial-of-service attack. You should leave a connection open unless you know it will be idle for an extended period of time—for example, if you only send notifications to your users once a day it is ok to use a new connection each day.

You're right, though, that there isn't currently a way to know when all notifications have been sent. Generally, our expectation has been that the connection will be open for a long time and shutdown timing won't be a high-precision affair (i.e. you'll be doing it as part of a larger application shutdown). The README is a bit confusing in that regard, though; you're right that it makes it sound like we're encouraging you to shut down the PushManager whenever the queue is empty, and I'll fix that shortly.

Could you tell us a bit more about your use case? Is this a thing where we're just setting expectations poorly in the README, or do you have some functional need that's not being met by the design of the library itself?

@Raibaz
Copy link
Author

Raibaz commented Sep 2, 2013

I guess I was just confused by the README, what you say about keeping the connection open makes perfect sense.

Anyway, IMHO having some methods to query the state of the pushManager and the notifications queue may come in handy, I'll try forking and implementing them later on.

@jchambers
Copy link
Owner

Anyway, IMHO having some methods to query the state of the pushManager and the notifications queue may come in handy, I'll try forking and implementing them later on.

Fair enough. I'll also take a whack at it. It's a little tricky since we're doing asynchronous network IO, though; to check on "done"-ness, we'd need to wait for all network writes to finish, and then make sure we don't have a reply from the APNs gateway. Should be doable, but I suspect it will be non-trivial.

I'll certainly update the README, too. Cheers!

@jchambers
Copy link
Owner

Made the README/site changes in 80ed150 and 00e27c6.

I've been thinking a lot about how to reflect queue state. We can't know even when a message has been sent by the underlying transport, so our options are pretty limited (just checking whether the queue is empty doesn't tell us much because sent messages might get re-enqueued after an exception).

The only way we could really get affirmative confirmation that messages have been sent (and thus have a meaningful sense of queue state) would be to send a deliberately bogus message to the APNs gateway; when the gateway rejects the known-bad message, we'll know that everything prior to that message was handled successfully. I'll explore that in more detail and see if it passes the crazy test.

@jchambers
Copy link
Owner

The only way we could really get affirmative confirmation that messages have been sent (and thus have a meaningful sense of queue state) would be to send a deliberately bogus message to the APNs gateway; when the gateway rejects the known-bad message, we'll know that everything prior to that message was handled successfully.

I've opened #4 to shut down with a known-bad notification.

@jchambers
Copy link
Owner

We've merged #4, which I think closes the book on fuzzy shutdown timing.

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

No branches or pull requests

2 participants