Socket connection drops next notification if invalid notification is sent

[kyledrake]

So, this is the problem I ran into on Lead Zeppelin (https://github.com/geoloqi/lead_zeppelin). It's kindof a big problem right now, and I haven't figured out a graceful way to fix it yet.

If you send an illegit message (such as a message with a bad device token), the connection will drop without throwing an exception for the next message, and that next message will effectively be dropped.

Example:

pusher = Grocer.pusher(certificate: "/path/to/validcert.pem")
notification = Grocer::Notification.new(device_token: "valid device token", alert: 'this will work')
pusher.push(notification)
notification = Grocer::Notification.new(device_token: "bad device token", alert: 'this will fail, as expected')
pusher.push(notification)
notification = Grocer::Notification.new(device_token: "valid device token", alert: 'THIS WILL UNEXPECTEDLY FAIL WITH NO EXCEPTIONS OR WARNINGS')
pusher.push(notification)
notification = Grocer::Notification.new(device_token: "valid device token", alert: 'this will work because an exception will be thrown, reconnecting the socket')
pusher.push(notification)

I don't know what the source of this problem is.. whether it's something in Ruby, something on Apple's APNS servers, or some sort of buffering issue, or what.

I worked around this problem by putting in an IO.select to wait for a specific period of time, to see if the socket becomes available for reading. With the enhanced protocol, being readable indicates that there was an error. But the only way to do this is to wait for a response before using it again, due to the silent failure. But this solution sucks, because it slows everything down.

You can see the implementation of that IO.select here: https://github.com/geoloqi/lead_zeppelin/blob/master/lib/lead_zeppelin/apns/gateway.rb#L96

The solution I was pondering to deal with this problem was to switch back to the version 0 protocol, but then of course you don't get the enhanced error response, which also kindof sucks.

The real solution would be for APNS to send a confirmation bit like they should, or drop the connection correctly... but that's not going to happen anytime soon. Really at a loss for how to solve this.

[kenkeiter]

My guess is that this issue is being caused by the fact that you're using typical non-blocking sockets, which are implemented a bit weirdly in Ruby by default. I ran into a similar experience, and switching to kgio (http://bogomips.org/kgio/) resolved it. Voodoo, I know -- but what works, works.

[kyledrake]

I was thinking of trying kgio to see if it helped, I will try this. Thank
you!

On Friday, June 22, 2012, Kenneth Keiter wrote:

My guess is that this issue is being caused by the fact that you're using
typical non-blocking sockets, which are implemented a bit weirdly in Ruby
by default. I ran into a similar experience, and switching to kgio (
http://bogomips.org/kgio/) resolved it. Voodoo, I know -- but what works,
works.

---

Reply to this email directly or view it on GitHub:
https://github.com/highgroove/grocer/issues/14#issuecomment-6522121

[kyledrake]

KGIO does not appear to resolve this situation (on Lead Zeppelin at least). I have tried this on JRuby and got the same results. After testing 3 different SSL socket implementations, I'm starting to arrive at the conclusion that there is no way to fix this except to introduce an IO.select and wait an arbitrary amount of time for an error before using the connection again. Again, if people more apt at socket programming see an obvious problem here, feel free to chime in, it's entirely possible that we're just not doing some cryptic thing that we're supposed to do.

Implementing the 0 version of the protocol is another option on the table that might resolve this problem (at the expense of better error reporting).

[netbe]

I had this kind of error using apn_on_rails , I didn't find out a good solution too. Used to rescue the error with the device id of the bad token and reopen a connection and start with the next device.

I'm curious about what you'll find out.

[kyledrake]

The only implementation that does this correctly AFAICT is Lead Zeppelin, which uses IO.select to wait an arbitrary time to see if an error comes back. This makes it able to send less messages though.

My one and only idea for solving this would be to try to implement the first version of the protocol (0) and see if the problem goes away.

I'm really surprised that I'm the only person that has brought this up before.. I found this problem immediately with very basic testing.

[kevboh]

I'm running into this as well. Any solutions in the past three months?

[kyledrake]

Nothing has been attempted yet. My only idea so far is to try to inplement
the original version of the protocol and see if it fixes it. I have not had
the time to work on this unfortunately.

I implemented an apns solution around the same time called lead_zeppelin
that uses an arbitrary io.select wait time, which you can find in my
repository. Until a better solution is found, that small wait solves the
problem, and you can probably implement something like that with a sleep of
the same length with grocer. Not ideal of course, but probably manageable
if you're not sending a ton of messages.

FWIW, every apns implementation i've tried has had the exact same problem.
It's not a problem specific to Grocer. I would more describe it as an
unfortunate consequence of the implementation on Apple's end.

I want to try to implement the original protocol with grocer to see if it
fixes it. I think that's the next action here.

On Friday, October 12, 2012, kevboh wrote:

I'm running into this as well. Any solutions in the past three months?

—
Reply to this email directly or view it on GitHubhttps://github.com/highgroove/grocer/issues/14#issuecomment-9374344.

[kevboh]

Sounds good. I'll check out lead_zeppelin.

[kbrock]

for #43 I introduced 2 different IO.selects

1. select on read and write after every call. This will allow you to not block, but find errors as quickly as they are found
2. select on read only once you are done. This will ensure any errors raised will be found.
   (you will still need to go back and send any notifications after the error was found)

I tried sleep(1) before reading, but that sometimes waited too long, so the apple connection was closed and I couldn't get the error information.

[stevenharman]

@vanstee @kbrock @kyledrake @Aleph7 I want to try to clear some things up, get us all on the same page, and then propose a possible, not-waiting-after-every-write, solution.

TL;DR: avoid waiting after every write and move to a producer/consumer setup.

The problem(s)

1. Simple notifications: Apple sends no errors back when/if a notification fails. They just silently close the connection. So that stinks.
2. Enhanced notifications: Apple sends errors back only for notifications that fail. They send nothing for successful notifications.
3. This applies to both of the above: The connection to APNS is asynchronous, so we can't assume that the notification sent right before the connection closes is the one that was bad. For Simple notifications, we're out of luck. For Enhanced, we could leverage the optional :identifier bytes to know which notification failed.

Current approaches

The approaches so far all use an IO.select to wait an amount of time after every write, just in case it failed. This is a huge hit to performance and also, b/c the connection is async, doesn't actually solve the problem of knowing which notification failed as we could have gone over our wait timeout and sent another message, but we'd not yet received the error from the previous one.

An alternative approach

What if we used a couple of thread-safe queues and the producer/consumer pattern? So there is a queue of to_be_sent notifications and a queue of sent notifications. We'd mark each notification with an :identifier before sending it, then immediately push it into the sent queue. Then, use an async read to listen for any errors.

When an error is found :identifier in the error message lets us know which notification was bad. So anything ahead of it in the sent queue can be assumed to be "good." Everything behind it needs to be re-enqueued in the to_be_sent queue, and the bad notification can be discarded (or collected so the client code can act accordingly).

For more background with better descriptions of the problems, current approaches, and even this kind of queue-based alternative, check out this post: http://redth.info/the-problem-with-apples-push-notification-ser/

[kbrock]

Hi @stevenharman, @vanstee, @kyledrake, and @Aleph7

We are all on the same page: It is not a viable option to block after every push.

But please keep in mind that IO.select does not necessarily block.

One implementation

You can find the actual implementation in our copy of pusher.rb.

notifications.each do | notification |
  push notification
  if check_for_issues_non_blocking
    rewind_to notification_after_error
  end
end
if check_for_issues_blocking(1.second)
    rewind_to notification_after_error
end

Having the 1 second block at the very end allows us to get the response code from Apple. We can confidently clear out all state once this method is finished.

Also, if we were not blocking at the end and Apple did send a response code. We may not come into the method for another few seconds. Apple will have already closed the socket and the Apple response code will have been lost.

We can use IO.select for blocking and non blocking requests:

IO.select takes 4 parameters: read, write, error, and timeout.

For our purposes, there are 2 ways we can call it:

1. IO.select [@ssl], [], [@ssl], 1
2. IO.select [@ssl], [@ssl], [@ssl], 1

First example

You have 1 second to see if we can read from the socket (or if there are socket errors).
This is the blocking case: check_for_issues_blocking(1.second)

Second example

You have 1 seconds to see if we can read OR write from the socket (or if there are socket errors).
Since we can basically always write to the socket, it will not block. Or if it did, it would have blocked when we wrote to the socket anyways. So no loss.
When there is an issue from apple, IO.select will tell a socket has an error packet on it, and we can read from the socket. As a side, IO.select always seems to say we can write to the socket, even after Apple has written to the socket and is about to close it.
This is the non-blocking case: check_for_errors_non_blocking

You can see both implementation in our copy of ssl_connection.rb

What did I miss something?

Most of the implementations I've seen use the first example. And since this blocks, most developers have assumed that the code has to block.

Anyone know if there is a technical reason why developers have only been using the first example instead of leveraging both?

[vanstee]

@stevenharman Totally agree with your "alternative approach". Seems like our best option. Although, we'll need a separate queue for the simple notifications since we wouldn't be able to figure out where in the queue that we failed.

Any thoughts on also providing a blocking API as well?

@kbrock I think blocking code is probably less complex which might be a good argument for it. For that "second example" are you saying, that we can call select on the connection for both read and write, knowing that the write connection will immediately available, and then just check the errors for the response from Apple? That's pretty clever, although I'm still leaning towards doing a blocking read in another thread so far.

[kbrock]

1. @vanstee - Second example does not block. And that is what I am currently using after every write.
   That is the code I am now using in production.
2. The "alternative approach" could be nice and simple.

I think the main issue is the secondary thread needs to communicate with the primary thread to say: you need to close the connection and you need to rewind back and send those alerts again.

Also, after leaving the producer, the secondary thread will not have a primary thread to tell that we need to rewind/ send the alerts again.

Maybe it is a 3 threaded model
A secondary apple error thread
A third push notification to apple thread
The primary thread just adds notifications to the third apple thread.

I guess I have tended to avoid the threaded producer/consumer code when possible. It just seems to have so many synchronization and possible race conditions.

[alejandro-isaza]

I think introducing threads overcomplicates things. Something simple like this should suffice

notifications.each do |notification|
  write_blocking notification
  reply = read_nonblocking
  if reply
    handle_reply(reply) # use identifier to know where to resume
    return
  end
end
reply = read_blocking(timeout: 1)
handle_reply(reply) if reply

We read without blocking after each write. And at the end of the loop we do a blocking read so that we don't miss any late replies. If there is a reply we go back to the identifier that failed.

In the case of "simple notifications" we would just resume where we left, which will potentially miss some notifications. Nothing to do about that.

Am I missing something?

[vanstee]

In the case of "simple notifications" we would just resume where we left, which will potentially miss some notifications.

Wasn't exactly sure what this meant.

The non-threaded example still has the potential for missing messages, so we would have to do the same history queue thing for that example as well.

[alejandro-isaza]

I don't see why it has potential for missing messages. With messages you mean notifications or replies? And are you referring to the "simple" or the "enhanced" case?

[vanstee]

Both would still have the potential for sending notifications to Apple after they have decided to no longer receive data on the socket due to an invalid notification. Since we're using Nagles algorithm to combine packets sent over the wire (as recommended by Apple), there's the potential for Apple to receive a few notifications from us before they have checked the validity of the first notification.

Here's a quick example:

Let's say we send 3 notifications, but the first one is invalid. We'll send all three down the pipe. Apple will start inspecting them and forwarding them to the correct devices. However the first notification has an invalid token so Apple sends us an error response, closes the connection, and throws away the other messages we've already sent. So we would still have to check the identifier in the error response and rewind to that position to make sure we didn't lose any messages.

I'm totally not an expert on networking stuff, so I might be misunderstanding how this actually works. Let me know if I missed anything!

[alejandro-isaza]

That is why you use the identifier to know which notification failed and restart from there. See the actual implementation in my comment on #21 (comment)

[kbrock]

I'm running an algorithm similar to the pull request 21 algorithm 10-20 times. Playing with some different variables.
We introduce an error (by sending a bad token) and end up sending 2 to 4 messages after the bad request until apple responds.

We have yet to loose a packet or send a duplicate.

Do note: when I introduced a sleep instead of a blocking read at the very end, I did get errors without a response code. So I feel strongly about that.

[stevenharman]

One thing to keep in mind - the producer/consumer model has a very nice, and desirable, side benefit of also being thread safe. The read-after-each-send-and-once-more-at-the-end is not, in and of itself, thread safe.

I'm not exactly sure what the implementation would look like, but I know that it really helps to keep threaded code simple and concise - so that would be a guiding principle.

[kbrock]

@stevenharman

There is a notification sender, and a notification error receiver. Both of which need to act upon a single socket.

In the read after write case - both read and write and close are happening on the primary thread by the pusher.
In the threaded case, read is happening by a secondary thread, and write/close is happening by the primary thread.

In both cases, the pusher can only be used by a single thread.

Personally, I'm running this in event machine and as expected, the blocking on write to apple is not working for me.

[stevenharman]

I should have been more clear - I meant that from the consumer side, this would make Grocer::Pusher thread-safe.

I am making the assumption that the send queue is thread-safe (which Ruby's Queue is), so you could get a Grocer.pusher instance and share that across threads. Then each client thread can safely push new Grocer::Notification objects via the Grocer::Pusher instance, which will just push them onto the queue.

As you've said, there is a single sender thread which only writes and closes the socket. So yes, we are only sending via one thread, but those writes are small and fast, and by using the single connection for all writes we save a huge amount on the overhead of opening/closing the sockets.

Does that make sense, or am I misunderstanding, confused, or don't-know-what-I'm-talking-about? (I would not be offended if you said the later. 😄 )

[jmoses]

Regardless of how it comes out, a non-threaded as an option would be appreciated. Our use-case for Grocer is via Sidekiq, so using threads with my threads feels like a fragile house full of cards. We instantiate a separate Pusher for each thread, so the overall thread-safety of the class isn't a concern.

[stevenharman]

We instantiate a separate Pusher for each thread, so the overall thread-safety of the class isn't a concern.

Depending on how you're sending messages, you need to be careful of this as Apple will can consider many short-lived connections to be a DDoS and shut you down.

[jmoses]

Yeah, I saw that in some of the documentation, forums. The threads are long lived, and the sockets don't look like they'll timeout on their own (the pusher never closes them as a surprise, from what I can see). We also limit our concurrency to that particular Sidekiq instance to 15.

--edit
Not to highjack the issue.

[AlexRiedler]

Let's go digging! since this was never resolved...

APNS has some oddities in regards to what they expect:

Note: I am not sure how this applies to the HTTP/2 API

Dropping Connection on Failure

Any notifications that you sent after the malformed notification using the same connection are discarded, and must be resent.
https://developer.apple.com/library/ios/documentation/NetworkingInternet/Conceptual/RemoteNotificationsPG/Chapters/CommunicatingWIthAPS.html

It's possible to send over 500 notifications before a write fails because of the connection being dropped. Around 1,700 notifications writes can fail just because the pipe is full, so just retry in that case once the stream is ready for writing again.
https://developer.apple.com/library/ios/technotes/tn2265/_index.html#//apple_ref/doc/uid/DTS40010376-CH1-TNTAG3

The reason why people do the synchronous blocking:
Now, here's where the tradeoffs get interesting. You can check for an error response after every write, and you'll catch the error right away. But this causes a huge increase in the time it takes to send a batch of notifications.

Async Error Responses

• you could send N messages, and none of them actually send due to one having an error
  • you need to keep track of which messages were 'sent', 'delivered', 'rejected'
  • this means you (in most situations) will want to re-send all these messages

Possible Solutions

Multi-Threaded Strategy

• consumer thread
• producer thread
• Allows for sending batches of notifications in blocks (more TCP efficient)
• Broken Pipe Issues (if you send while there is an error response, you get hosed; and potentially lose messages).

Implementation Details

• On shutdown/closing of the connection what is the intended behaviour?
• Thread synchronization
• Ring-buffer of previously sent messages during this session (this has to be reasonably big)
• How do you know its safe to shutdown the current connection to APNS (is there some expected timeout)

Apple guidelines indicate:
If your development tool of choice supports multiple threads or interprocess communication, you could have a thread or process waiting for an error response all the time and let the main sending thread or process know when it should give up and retry.

Single-Threaded Strategy

• single thread that is producer/consumer
• buffer up N messages before send, read after them; handle the failures.
• Mechanism to get list of identifiers messages
  • This could be a ring buffer or something similar with upto 2k messages (upwards of ~4MB + overhead)

User Strategy

• user controls the identifiers
• some callback mechanism for getting a list of identifiers
• some callback mechanism for marking a notification as failed (and reason)

@stevenharman do you have any suggestions as to how we should implement these? Should we implement them all ?

I was thinking we use some sort of pluggable strategy approach.

[kbrock]

Hi, I'm no longer using grocer, so things may have changed, but I did use grocer and have much success with it. In my situation we could not loose any messages, so we did solve this issue.

The actual problem is in the catch / retry logic on a connection close. It retries the send and forgot to ask the connection, "wait, did you want to leave a message?"

The tricky part is the messages have to be resent. A typical API shouldn't keep a history of messages around. Because that causes a memory leak. But that is what is required here. Whether keeping it in the connection object, or in the queue / postgres.

Also, going over ssl is a little tricky, because you call a select on read which tells you there is no data, so you write, but since it is going over ssl, a write sometimes requires a read, and it blocks. Or maybe I swapped read and write in this case. I found it confusing and had issues with linux/java/jruby. But as with tcp/ip, you only detect a connection is closed when you try and write on it. THEN you can read from the socket to figure out the message that Apple had sent you.

Your solution is to either a) cache messages along with the connection and be able to resend them when a connection retry is necessary, or b) move it out into a service.

I removed the retry logic, and kept a "memory leak" / message cache along with each connection.
While it felt wrong, it worked well. Also, I removed the retry logic and just exposed it to the caller. This allowed me to store metrics in statsd about the number of failures and the number of resends. So we could accurately depict the throughput.

If I had unlimited time, I would have created a service rather than treating apns/google as an api. Just put messages into postgres to be sent out. May have lost a little in throughput since that can't keep up with rabbit mq, but I would have been able to capture metrics on the number of retries per telephone number and come up with meaningful statistics.

Best of luck.

update:
I'm no longer using grocer because I am in a different company and not because I changed technology stacks.

[tipycalFlow]

The HTTP/2 API will make life a lot easier, but when!

[idyll]

With the changes to push tokens in iOS 9.0 this bug has made grocer completely unreliable.

iOS 9.0 results in a new push token being generated when an app is deleted then re-installed. The old token is invalidated. Often it will take the feedback services hours to report it, but Apple rejects it pretty much immediately.

I think that if there's no clear solution to this the README should be updated with a warning.

The project simply isn't usable without this being fixed.

[stevenharman]

Hello @idyll,

I don't use Grocer in my current product, and so am unable to dedicate much time to it other than smaller maintenance things.

In the event you (or anyone else reading this) would like to help out, I think a reasonable way forward is to implement the recently released HTTP/2 push provider.. Building that will "fix" this issue (and a handful of others) by way of allowing us to ignore the awkward and indeterminate push APIs that have caused this issue. I would be happy to help with such an effort, but I can't be the one driving it at this time.

Thank you.

[idyll]

There's an implementation here: https://github.com/alloy/lowdown but you'd need some sort of connection pooling and reuse to really make it work.

But that's not really the point. This situation breaks existing apps once the users upgrade to iOS 9.

I'm just saying this merits an update to the readme to warn people. And probably a statement asking for help on a PR to move to the HTTP/2 interface.

[kyledrake]

How's this:

A WARNING TO APPLE SHARE CROPPERS

Using an Apple SDK to develop software for your livelihood is dangerous. They will give you a broken implementation of a broken protocol written by somebody that's clearly never written an API before and has zero concept of error handling, then randomly add breaking changes to it, making your barely working broken implementation now 100% absolutely unusably broken. Expect everything to break, and expect Apple to not care or provide any way to communicate with the authors.

If you want to write reliable software, or software that doesn't make you feel like you're cooking soup for prisoners in a Siberian gulag, you should not under any circumstances develop software with an Apple SDK or API.

[idyll]

Important Note

iOS 9.0 (and subsequent versions) introduced a subtle change to the way push tokens provided.

If you delete and then re-install an application the push token is invalidated and a new push token is generated. This is important because the feedback service does not deliver the list of invalidated tokens quickly enough to prevent you from using the now invalidated token.

There is currently a bug in grocer (#14) that will cause the APNS socket connect to hang up and fail to send subsequent notifications when one of these invalid tokens is used.

This bug combined with the change to push tokens in iOS results in varied reliability of push notification delivery. This may or may not affect you, but if you are seeing a large amount of undelivered notifications - specifically when sending multiple messages in quick succession - it is likely that you are coming up against this.

Right now we are looking for help moving over to Apple's HTTP/2 notification endpoint which should address this situation, but the current maintainer doesn't have time to do this work.

[stevenharman]

@idyll I've added a notice, using the bulk of your working. Thanks! (See: 2af5adb)

[mohamedhafez]

So just to clarify, if we send an invalid notification, we'll lose the notifications that we send quickly after it, but the pusher does eventually get the error message and re-open the connection, so that at some point it will resume being able to send notifications successfully again, right?

[kbrock]

@mohamedhafez The messages you send right after sending a bad token will be lost unless you are tracking messages sent.

[mohamedhafez]

@kbrock just the messages immediately after, or all messages afterwards, forever?

[AlexRiedler]

@mohamedhafez it is documented as such:

let G be good messages.
let B be the bad message.
let R be when the SSL socket receives an error from APNS

GGGGGBGGGGGGR
time --->

All messages right of the bad message will be dropped.

[mohamedhafez]

@AlexRiedler my question is what happens after the R? Does the pusher object recover by opening a new connection, or do we lose all messages after the R as well?

[AlexRiedler]

@mohamedhafez the connection is closed immediately by APNS; so it has to re-establish a connection.

[stevenharman]

@mohamedhafez Once Grocer realizes the connection has been closed, it will open a new connection and resume sending messages. So any messages sent between the B message and R will be lost. Unless you're tracking them and then try to re-send them.

[mohamedhafez]

1. So messages sent after the R is received will be delivered successfully by the pusher object on a new connection, correct?

2. How reliable is the receipt of the R after sending B? Are there any edge cases where we won't get the R and we'll be losing subsequent messages indefinitely without knowing it?

[stevenharman]

1. So messages sent after the R is received will be delivered successfully by the pusher object on a new connection, correct?

Assuming they're good notifications, yes.

2. How reliable is the receipt of the R after sending B? Are there any edge cases where we won't get the R and we'll be losing subsequent messages indefinitely without knowing it?

The R isn't even an error message sent back by APNS, it's that they close their end of the socket, and so suddenly Grocer can't push any more messages. Internally, Grocer checks before sending each notification to make sure the connection is open. When it finds it's closed, it will open a new one and use that.

[mohamedhafez]

Perfect, thanks for your patience guys:)