No way to close the client

[willfaught]

If I have something like:

client := [...]
go func() {
    for f := range client.FailedNotifs {
        [...]
    }
}()
for [...] {
    [...]
    [...] client.Send(n) [...]
    [...]
}
// done, now what?

How do I clean up? I don't see any code that closes client.FailedNotifs, so that goroutine won't stop unless I signal it myself (if I even know it exists). It's also strange to be expected to close client.Conn yourself; this requires intimate familiarity with the client implementation instead of just its interface.

It would be great if Client had a Close() error method, or something comparable.

[taylortrimble]

I support this. Are you inclined to submit the pull request yourself?

[willfaught]

Sure.

Just to make sure I understand: the only things that need to be cleaned up are Client.FailedNotifs, which can be closed with close(c.FailedNotifs), and Client.Conn, which can be closed with if err := c.Conn.Close(); err != nil [...]. Is that everything?

[willfaught]

Should it sleep for 1 second before closing FailedNotifs to capture any remaining errors?

Looks like I'll have to use a done channel to kill the main loop?

[taylortrimble]

I would recommend against waiting for an arbitrary interval to receive data. The errors on the Gateway connection are only related for binary formatting and shut down, neither of which we're interested in for a Close scenario.

Looks like you'll need a done channel, and your clean up looks about right. Don't forget to close the errs channel in runLoop as well.

[willfaught]

Wouldn't you want to know about pending errors when you close for the same reason that you want to know before you close? For example, if any of the tokens I have are invalid, I'd like to note that so I don't use them again. If we don't wait, then couldn't I miss one of those errors?

[taylortrimble]

There's no positive acknowledgement for successful notifications, nor any reason to believe 1s is a long enough to get errors back (and it's not :wink:). Also since invalid tokens come over the Feedback service, only misformatted tokens would be reported on the return channel, which is... not going to happen.

If you really wanted to wait until the gateway had finished returning error responses, you would accomplish this with a unidirectional connection shutdown on the client write side instead of a "cease communication, timeout waiting for responses, then close both sides" pattern. That's the canonical way to support waiting on pending server responses. However, I doubt that APNS will adopt that pattern (I bet they'll assume you just close it) and the interesting responses come from the Feedback service anyway, so it's probably not worth the effort.

[willfaught]

Doesn't APNS drop any subsequent push notifications after it reports an error? My understanding was that you have to know what notifications you've sent since the one identified by the error so you can resend them (because APNS didn't act on them after the error). Does this client not handle that?

[nathany]

There is code in this client to resend notifications once an error comes back.