Fix RTN7c: notify publish callbacks of broken connection.

[tcard]

[ricardopereira]

And if the current connection state is FAILED?

[tcard]

What then? The spec says "If a connection enters the CLOSED or FAILED state [...] the client should consider the delivery of those messages as failed", and that's what's happening here. (Maybe this is being done at the wrong level, though, because RTN7 is about ACK and NACK...)

[ricardopereira]

LGTM

[mattheworiordan]

I am not sure this is quite right. It's not clear that the state being lost has triggered the failure as opposed to just a timeout. What we need to check is that as soon as the new connection is made, and the connection state is lost because the connection ID has changed, then immediately all messages should be treated as failed to be published.

[tcard]

Done at 7debb07.

[tcard]

Review status: 0 of 5 files reviewed at latest revision, 2 unresolved discussions.

---

ablySpec/RealtimeClientConnection.swift, line 781 [r1] (raw file):
Done at 7debb07.

---

Comments from the review on Reviewable.io

[tcard]

@mattheworiordan PTAL

[mattheworiordan]

LGTM

[ricardopereira]

I think you need to remove the listener as well, right?

[ricardopereira]

Maybe not. Line 168 stops the listener.

[tcard]

I've made it a once instead of an on. Fixed at fb5ccdd.

[tcard]

I had to revert this at 2ed75ea. The listener only has to be unsubscribed if there's a failure or the message is sent. The connection can change several times before the message callback arrives, e. g. when reconnecting.

RealtimeClientConnection and RealtimeClientChannel tests are passing now on my machine, let's see if the whole suite does... (I'm not running all of them in my machine because that basically blocks me from working for half an hour.)

[ricardopereira]

Can we merge this after a green test?

[tcard]

I guess, I'm still trying to get this to pass...

[mattheworiordan]

This looks like it's ready to be merged no?

[ricardopereira]

Test suite isn't passing:

[08:24:32]: ▸ ablySpec.RealtimeClientConnection
[08:24:32]: ▸ Connection__ACK_and_NACK__should_trigger_the_failure_callback_for_the_remaining_pending_messages_if__lost_connection_state, failed - expected to be true, got <false>
[08:24:32]: ▸ /Users/travis/build/ably/ably-ios/ablySpec/RealtimeClientConnection.swift:788
[08:24:32]: ▸ ```
[08:24:32]: ▸                         expect(client.connection.id).toNot(equal(oldConnectionId))
[08:24:32]: ▸                         expect(gotPublishedCallback).to(beTrue())
[08:24:32]: ▸                     }

[ricardopereira]

💚

[ricardopereira]

With the excitement... I forgot to autosquash 😖 Shame on me.