Enhancements: It would be good to expose the detailed connection states to the user

[fans3210]

It would be good to expose the detailed connection states to public so that user can choose to manually fetch the missing messages between the disconnect-to-connected time window. Eg: Connecting, Connected, Not Observable(might be 'connecting' too although might not be observable), Disconnected etc.

[rohandubal]

Hello @fans3210

Thanks for the feedback. Can you elaborate a little more?

For e.g. Do you want these states at a client level or at a subscription level? Can you elaborate the use-cases which you are targeting?

Thanks
Rohan

[fans3210]

@rohandubal . I think the connection status for each topic should be on the client side like other xmpp service providers eg: Quickblox. Currently I applied this in my private fork of this repo. Currently what I did is some simple logics like below:

public typealias NonDisconnectionMQTTStateHandler = (MQTTStatus) -> Void

public func subscribeToCreateNewConversation(otherMQTTConnectionStateHandler: @escaping NonDisconnectionMQTTStateHandler, simpleResultHandler handler: @escaping SimpleSubResultHandler<Bool>) -> AWSAppSyncSubscriptionWatcher<NewConversationSubSubscription>? {
xxxxxx
}

I expose the connection status in the subscribe function temporarily. But I think a better place is in a delegation method like:
just for example:
func connectionStatusChanged(for watcher: toplic:)

UseCase:
For a chat app, normally I will unsubscribe all topics when the app entered background to reduce the battery and data cost. By exposing the detailed connection status to the developer, when app enters foreground again, I can manually handle retrieval of the chat messages I missed during the disconnection period.

[MarioBajr]

Hi @fans3210, I have a branch fixing this waiting for #28 to be merged.
Feel free to test and check if it meets your needs.
https://github.com/EFEducationFirstMobile/aws-mobile-appsync-sdk-ios/tree/feature/observableSubscriptionState
or apply this diff on top of #28 https://gist.github.com/MarioBajr/d297cc0144a6e8fca4f6bd559946484b

[fans3210]

@MarioBajr . Amazing. Tks

[frankmuellr]

@rohandubal Looks like this PR was merged a while back. Can this issue be closed?

[MarioBajr]

Hi @muellerfr,
I believe that you're referring to this PR #75 , and it hasn't been merged yet. In order to create the observable subscriptions status PR on top of the master branch I need the #28 to be accepted and merged.

[palpatim]

@fans3210

Regarding your use case description:

UseCase:
For a chat app, normally I will unsubscribe all topics when the app entered background to reduce the battery and data cost. By exposing the detailed connection status to the developer, when app enters foreground again, I can manually handle retrieval of the chat messages I missed during the disconnection period.

AppSync currently suspends network activity when it enters the background, and resumes when the app resumes foreground. In addition, the recently-added Delta Sync feature basically does just what you specify: when creating a new sync subscription, the system will query for only the changes since the last successful sync.

Does that meet your needs for this use case?

If so, I'd like to scope down this feature request to providing better notification of an initial connection. Currently, there's no way to know that a subscription has been established and will start receiving updates, only that you've either a) received a new subscription message; or b) disconnected.

I propose a simplified state like:

public enum SubscriptionWatcherStatus {
    case connecting
    case connected
    case disconnected
    case error(AWSAppSyncSubscriptionError)
 }

and an appropriate refactoring of AWSAppSyncSubscriptionError to be enum, rather than struct, based, similar to what you show in your gist

Thoughts?

[fans3210]

@palpatim Sorry I don't know this issue is still opened. U guys did amazing jobs. The refactor is totally fine. Will reopen this issue when someone else requires such feature again. Thanks!

[shannon-hager-skookum]

@palpatim I know this issue is closed but are you still planning to add the SubscriptionWatcherStatus enum?

[palpatim]

@shannon-hager-skookum

Yes, I think it makes sense to reopen this, scoped to the simplified state I mentioned in the comment above. The biggest question in my mind is how we actually accomplish the "connected" state, since right now there's no really great hook from the underlying subscription connection that the service has begun fulfilling it. In testing, I'm working around this by inspecting the list of topics available on a subscription--once it moves to a non-empty list, then I'm interpreting that to mean the service is delivering subscriptions. But I'm not sure that's reliable, and I am sure I'd rather have a service-blessed mechanism for detecting that state. :) So part of this feature request will be to investigate the "right" way of fulfilling the "connected" state.

[achager]

@palpatim Would you be able to elaborate on what is considered the "connected" state in this scenario? Is it more than the web socket itself being open?

[palpatim]

There is a delay between the time the socket is opened/connected to the server and the time the service begins delivering subscription updates to that socket. In our integration tests, we rely on either a hardcoded timeout or the aforementioned topic count to accommodate that.

[palpatim]

Clarification on the proposed statuses:

public enum SubscriptionWatcherStatus {
    /// The subscription is in process of connecting
    case connecting

    /// The subscription has connected and is receiving events from the service
    case connected

    /// The subscription has been disconnected because of a lifecycle event or manual disconnect request
    case disconnected

    /// The subscription is in an error state. The enum's associated value will provide more details, including recovery options if available.
    case error(AWSAppSyncSubscriptionError)
 }

[achager]

@palpatim Those proposed status look good and should cover our use cases. Thanks!

[palpatim]

Working on an implementation of this now.

[palpatim]

This should be coming later today, but in the meantime, the API changes are as follows:

// New typealias:
public typealias SubscriptionStatusChangeHandler = (AWSAppSyncSubscriptionWatcherStatus) -> Void

// New status enum

/// The status of a SubscriptionWatcher
public enum AWSAppSyncSubscriptionWatcherStatus {
    /// The subscription is in process of connecting
    case connecting

    /// The subscription has connected and is receiving events from the service
    case connected

    /// The subscription has been disconnected because of a lifecycle event or manual disconnect request
    case disconnected

    /// The subscription is in an error state. The enum's associated value will provide more details, including recovery options if available.
    case error(AWSAppSyncSubscriptionError)
}


// New `statusChangeHandler` option to the AppSyncClient `subscribe()` method:

public func subscribe<Subscription: GraphQLSubscription>(subscription: Subscription,
                                                         queue: DispatchQueue = DispatchQueue.main,
                                                         statusChangeHandler: SubscriptionStatusChangeHandler? = nil,
                                                         resultHandler: @escaping SubscriptionResultHandler<Subscription>) throws -> AWSAppSyncSubscriptionWatcher<Subscription>?

The statusChangeHandler will be invoked upon status changes to the watcher's underlying MQTT client. This is specifically useful for monitoring when the watcher moves to .connected status, as it indicates that the watcher client has received a SUBACK from the MQTT broker and is receiving subscription messages from the service. At that point, it would be safe to update the UI with appropriate messaging, or issue a "catch up" query (in the case of a reconnect operation, as will be done automatically by Delta Sync in the sync method)

At this point, I expect the changes to be wholly additive, so they shouldn't break any existing implementations.

[palpatim]

Sorry for missing the update on this issue, but we believe this was fixed by #196, and refined by @rohandubal on subsequent commits. I'm closing this issue but please do open a new one if you see use cases that aren't covered by this status reporting.