Move notification struct error checking before queueing it in run loop

[bdotdub]

This PR passes back validation and serialization errors synchronously to the caller of Send() instead of...well...ignoring it in the run loop or having to read off of another channel.

Fixes #4

[bdotdub]

@tylrtrmbl @nathany @brunodecarvalho if anyone has some spare 👀, this should help a bit with #4

[nathany]

@bdotdub If it's okay, I might not get to this until next Wednesday.

[taylortrimble]

Actually, I changed my mind back again from #4. I think it makes sense to use the synchronization and information passing mechanism you have now (a channel of Notification structs) and simply use FailedNotifs to report both user (ToBinary) and Apple (conn.Send) errors.

From my perspective, there's no reason I would need to know if the error came from conn.Send or somewhere else.

[taylortrimble]

What's up with Travis CI by the way?

[nathany]

Personally I'd prefer Send return errors when it can.

But then I'm trying to build a synchronous API around Send and FailedNotifs that gives me back Apple's response or a timeout.

[bdotdub]

I've been thinking a lot about this and am feeling like we should have Send() return all of the errors and forego FailedNotifs. If the client is disconnected from Apple, sending a notification shouldn't disappear into the run loop and eventually pop out of the other end through FailedNotifs.

The API feels cleaner to me if I get an error when trying to call Send() while we're disconnected. This will make it clear when we have connection issues with Apple and the caller code can check whether the error is ErrGatewayDisconnected or something and which would make the caller is responsible for calling Connect() in that case.

[taylortrimble]

You're suggesting a synchronous API then?

[bdotdub]

Yeah - it'll require a lot less bookkeeping internally and will push that option to the caller.

[bdotdub]

Especially since the actually sending is basically synchronous on one tcp connection

[biasedbit]

👍 for the synchronous API/pushing that responsibility to the caller.

[taylortrimble]

👍 For me too, actually. I actually prefer that the caller have granular control over stuff like that. Consider my previous comments as "assuming you wanted to keep it async".

Excited to see it now!

[bdotdub]

Going to hold off on merging this for now. I've got some code in the hopper that'll fixes up the internals to be less confusing as mentioned here.

[nathany]

This sounds pretty great and will really help make the API solid. But now I'm waiting for this to magically solve #16 instead of coming up with a solution.

[nathany]

Hoping #14 and #9 can get merged before going too far down this path.

[nathany]

@bdotdub Pushing some error handling back to user of the library sounds good. Say, if a certificate has expired, it shouldn't continue retrying (I've yet to confirm what the current behaviour is).

[bdotdub]

Closing this in favor of #31 and #33