Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add timeout support when connecting user #3303

Merged
merged 29 commits into from
Jul 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
ed22ea6
Fix broken fake refresh logic in the Demo App
nuno-vieira Jul 12, 2024
e70a2e5
Fix Demo App connection banner not showing
nuno-vieira Jul 12, 2024
c5895e1
Fix Channel List not hiding error view when data is available
nuno-vieira Jul 12, 2024
e7959d0
WIP Add reconnection time out handler
nuno-vieira Jul 12, 2024
4237323
Fix incorrectly considering invalid token as reconnectable
nuno-vieira Jul 15, 2024
aa38ee9
Fix trying to connect when network is available
nuno-vieira Jul 15, 2024
c93bd81
Fix constantly restarting the reconnection timeout handler
nuno-vieira Jul 15, 2024
b86baf4
Add previous state from timeout
nuno-vieira Jul 15, 2024
a74035b
Do not observe network status if no user is logged in
nuno-vieira Jul 12, 2024
ce576d1
Fix channel list error view animation on the Demo App
nuno-vieira Jul 16, 2024
12e8f96
Revert "Do not observe network status if no user is logged in"
nuno-vieira Jul 16, 2024
5a13120
When coming from offline / inactive state, try to connect in case it …
nuno-vieira Jul 16, 2024
b9f264a
Make the reconnection timeout configurable
nuno-vieira Jul 17, 2024
36ad7a2
Fix StreamChatUI tests
nuno-vieira Jul 17, 2024
63424f9
Fix StreamChat Tests compilation
nuno-vieira Jul 17, 2024
0b615e2
Fix web socket connection state tests
nuno-vieira Jul 17, 2024
e8714dd
Fix reconnection recovery tests
nuno-vieira Jul 17, 2024
f0b21d1
Add timeout testing coverage
nuno-vieira Jul 17, 2024
f635d07
Fix expired token being treated as invalid token
nuno-vieira Jul 17, 2024
ffeb619
Update CHANGELOG.md
nuno-vieira Jul 18, 2024
362fb39
Update CHANGELOG.md
nuno-vieira Jul 18, 2024
95005cb
Merge branch 'develop' into fix/add-timeout-when-connecting-user-alte…
nuno-vieira Jul 18, 2024
9fde3b8
Revert networking changes logic so that E2E Tests pass and to minimiz…
nuno-vieira Jul 18, 2024
d77edba
Fix not able to change chat client config on the demo app once initia…
nuno-vieira Jul 18, 2024
8f3623d
Fix E2E Test not actually waiting for connection and disconnection
nuno-vieira Jul 18, 2024
a263a37
Only observe network status when the user is logged in
nuno-vieira Jul 18, 2024
5b0a68d
Fix LLC Tests
nuno-vieira Jul 18, 2024
eb86ff1
Make test more robust
nuno-vieira Jul 18, 2024
57d8308
Fix connection recovery handler LLC Tsts
nuno-vieira Jul 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,19 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
# Upcoming

## StreamChat
### ✅ Added
- Add an option to configure a reconnection timeout [#3303](https://github.com/GetStream/stream-chat-swift/pull/3303)
### 🐞 Fixed
- Improve the stability of the reconnection process [#3303](https://github.com/GetStream/stream-chat-swift/pull/3303)
- Fix invalid token errors considered as recoverable errors [#3303](https://github.com/GetStream/stream-chat-swift/pull/3303)
### 🔄 Changed
- Dropped iOS 12 support [#3285](https://github.com/GetStream/stream-chat-swift/pull/3285)
- Increase QoS for `Throttler` and `Debouncer` to `utility` [#3295](https://github.com/GetStream/stream-chat-swift/issues/3295)
- Improve reliability of accessing database models in controller provided completion handlers [#3304](https://github.com/GetStream/stream-chat-swift/issues/3304)
- Improve reliability of accessing data in controllers' completion handlers [#3304](https://github.com/GetStream/stream-chat-swift/issues/3304)

## StreamChatUI
### 🐞 Fixed
- Fix Channel List not hiding error state view when data is available [#3303](https://github.com/GetStream/stream-chat-swift/pull/3303)

## StreamChatUI
### ✅ Added
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ class AppConfigViewController: UITableViewController {
enum ChatClientConfigOption: String, CaseIterable {
case isLocalStorageEnabled
case staysConnectedInBackground
case reconnectionTimeout
case shouldShowShadowedMessages
case deletedMessagesVisibility
case isChannelAutomaticFilteringEnabled
Expand Down Expand Up @@ -347,6 +348,9 @@ class AppConfigViewController: UITableViewController {
cell.accessoryView = makeSwitchButton(chatClientConfig.staysConnectedInBackground) { [weak self] newValue in
self?.chatClientConfig.staysConnectedInBackground = newValue
}
case .reconnectionTimeout:
cell.detailTextLabel?.text = chatClientConfig.reconnectionTimeout.map { "\($0)" } ?? "None"
cell.accessoryType = .disclosureIndicator
case .shouldShowShadowedMessages:
cell.accessoryView = makeSwitchButton(chatClientConfig.shouldShowShadowedMessages) { [weak self] newValue in
self?.chatClientConfig.shouldShowShadowedMessages = newValue
Expand All @@ -371,6 +375,8 @@ class AppConfigViewController: UITableViewController {
switch option {
case .deletedMessagesVisibility:
pushDeletedMessagesVisibilitySelectorVC()
case .reconnectionTimeout:
pushReconnectionTimeoutSelectorVC()
default:
break
}
Expand Down Expand Up @@ -581,6 +587,24 @@ class AppConfigViewController: UITableViewController {
navigationController?.pushViewController(selectorViewController, animated: true)
}

private func pushReconnectionTimeoutSelectorVC() {
let selectorViewController = OptionsSelectorViewController<TimeInterval?>(
options: [nil, 15.0, 30.0, 45.0, 60.0],
initialSelectedOptions: [chatClientConfig.reconnectionTimeout],
allowsMultipleSelection: false,
optionFormatter: { option in
option.map { "\($0)" } ?? "None"
}
)
selectorViewController.didChangeSelectedOptions = { [weak self] options in
guard let selectedOption = options.first else { return }
self?.chatClientConfig.reconnectionTimeout = selectedOption
self?.tableView.reloadData()
}

navigationController?.pushViewController(selectorViewController, animated: true)
}

private func showTokenDetailsAlert() {
let alert = UIAlertController(
title: "Token Refreshing",
Expand Down
14 changes: 8 additions & 6 deletions DemoApp/Shared/StreamChatWrapper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,17 @@ final class StreamChatWrapper {
var client: ChatClient?

// ChatClient config
var config: ChatClientConfig = {
var config = ChatClientConfig(apiKeyString: apiKeyString)
var config: ChatClientConfig {
didSet {
client = ChatClient(config: config)
}
}

private init() {
config = ChatClientConfig(apiKeyString: apiKeyString)
config.shouldShowShadowedMessages = true
config.applicationGroupIdentifier = applicationGroupIdentifier
config.urlSessionConfiguration.httpAdditionalHeaders = ["Custom": "Example"]
return config
}()

private init() {
configureUI()
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
//
// Copyright © 2024 Stream.io Inc. All rights reserved.
//

import StreamChatUI
import UIKit

class DemoChatChannelListErrorView: ChatChannelListErrorView {
override func show() {
UIView.animate(withDuration: 0.5) {
self.isHidden = false
}
}

override func hide() {
UIView.animate(withDuration: 0.5) {
self.isHidden = true
}
}
}
1 change: 1 addition & 0 deletions DemoApp/StreamChat/StreamChatWrapper+DemoApp.swift
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ extension StreamChatWrapper {
Components.default.messageActionsVC = DemoChatMessageActionsVC.self
Components.default.messageLayoutOptionsResolver = DemoChatMessageLayoutOptionsResolver()
Components.default.reactionsSorting = ReactionSorting.byFirstReactionAt
Components.default.channelListErrorView = DemoChatChannelListErrorView.self

// Customize MarkdownFormatter
let defaultFormatter = DefaultMarkdownFormatter()
Expand Down
4 changes: 2 additions & 2 deletions Sources/StreamChat/APIClient/RequestDecoder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ struct DefaultRequestDecoder: RequestDecoder {
throw ClientError.Unknown("Unknown error. Server response: \(httpResponse).")
}

if serverError.isInvalidTokenError {
log.info("Request failed because of an experied token.", subsystems: .httpRequests)
if serverError.isExpiredTokenError {
log.info("Request failed because of an expired token.", subsystems: .httpRequests)
throw ClientError.ExpiredToken()
}

Expand Down
6 changes: 4 additions & 2 deletions Sources/StreamChat/ChatClient+Environment.swift
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ extension ChatClient {
_ extensionLifecycle: NotificationExtensionLifecycle,
_ backgroundTaskScheduler: BackgroundTaskScheduler?,
_ internetConnection: InternetConnection,
_ keepConnectionAliveInBackground: Bool
_ keepConnectionAliveInBackground: Bool,
_ reconnectionTimeoutHandler: StreamTimer?
) -> ConnectionRecoveryHandler = {
DefaultConnectionRecoveryHandler(
webSocketClient: $0,
Expand All @@ -114,7 +115,8 @@ extension ChatClient {
internetConnection: $5,
reconnectionStrategy: DefaultRetryStrategy(),
reconnectionTimerType: DefaultTimer.self,
keepConnectionAliveInBackground: $6
keepConnectionAliveInBackground: $6,
reconnectionTimeoutHandler: $7
)
}

Expand Down
8 changes: 7 additions & 1 deletion Sources/StreamChat/ChatClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,8 @@ public class ChatClient {
extensionLifecycle,
environment.backgroundTaskSchedulerBuilder(),
environment.internetConnection(eventNotificationCenter, environment.internetMonitor),
config.staysConnectedInBackground
config.staysConnectedInBackground,
config.reconnectionTimeout.map { ScheduledStreamTimer(interval: $0, fireOnStart: false, repeats: false) }
)
}

Expand Down Expand Up @@ -288,6 +289,8 @@ public class ChatClient {
tokenProvider: @escaping TokenProvider,
completion: ((Error?) -> Void)? = nil
) {
connectionRecoveryHandler?.start()

authenticationRepository.connectUser(
userInfo: userInfo,
tokenProvider: tokenProvider,
Expand Down Expand Up @@ -379,6 +382,7 @@ public class ChatClient {
userInfo: UserInfo,
completion: ((Error?) -> Void)? = nil
) {
connectionRecoveryHandler?.start()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nuno-vieira @martinmitrevski Hello, I found that this code is causing an issue, so I'm leaving a comment.

connectGuestUser calls logOut first. As a result, ConnectionRecoveryHandler.stop is invoked, causing the monitoring of appDidBecomeActive and appDidEnterBackground to stop. This leads to the issue where "the WebSocket does not reconnect when the app resumes from the background."

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I posted issue #3338

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @masarusanjp, thanks for spotting this issue!

The actual issue is that when reconnecting, logOut should not be happening, so the logic should be changed here. Stopping the recovery handler on logout is something that we want, but it caused this unwanted side effect due to the fact that logOutFirst in guest users is not properly implemented, it seems.

authenticationRepository.connectGuestUser(userInfo: userInfo, completion: { completion?($0) })
}

Expand All @@ -402,6 +406,7 @@ public class ChatClient {
/// Connects an anonymous user
/// - Parameter completion: The completion that will be called once the **first** user session for the given token is setup.
public func connectAnonymousUser(completion: ((Error?) -> Void)? = nil) {
connectionRecoveryHandler?.start()
authenticationRepository.connectAnonymousUser(
completion: { completion?($0) }
)
Expand Down Expand Up @@ -437,6 +442,7 @@ public class ChatClient {
/// Disconnects the chat client from the chat servers. No further updates from the servers
/// are received.
public func disconnect(completion: @escaping () -> Void) {
connectionRecoveryHandler?.stop()
connectionRepository.disconnect(source: .userInitiated) {
log.info("The `ChatClient` has been disconnected.", subsystems: .webSocket)
completion()
Expand Down
4 changes: 4 additions & 0 deletions Sources/StreamChat/Config/ChatClientConfig.swift
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,10 @@ public struct ChatClientConfig {
/// It controls how long (in seconds) a network task should wait for additional data to arrive before giving up
public var timeoutIntervalForRequest: TimeInterval = 30

/// The maximum time in seconds the SDK will wait until it reconnects successfully, in case it was disconnected by a recoverable error.
/// By default there is no timeout, so the SDK will keep trying to connect for an undetermined time.
public var reconnectionTimeout: TimeInterval?

/// Enable/Disable local filtering for Channel lists. When enabled,
/// whenever a new channel is created,/updated the SDK will try to
/// match the channel list filter automatically.
Expand Down
2 changes: 1 addition & 1 deletion Sources/StreamChat/Errors/ErrorPayload.swift
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ extension ErrorPayload {

extension ClosedRange where Bound == Int {
/// The error codes for token-related errors. Typically, a refreshed token is required to recover.
static let tokenInvalidErrorCodes: Self = StreamErrorCode.expiredToken...StreamErrorCode.invalidTokenSignature
static let tokenInvalidErrorCodes: Self = StreamErrorCode.notYetValidToken...StreamErrorCode.invalidTokenSignature
nuno-vieira marked this conversation as resolved.
Show resolved Hide resolved

/// The range of HTTP request status codes for client errors.
static let clientErrorCodes: Self = 400...499
Expand Down
6 changes: 2 additions & 4 deletions Sources/StreamChat/Repositories/ConnectionRepository.swift
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,8 @@ class ConnectionRepository {
case let .connected(connectionId: id):
shouldNotifyConnectionIdWaiters = true
connectionId = id
case let .disconnected(source) where source.serverError?.isInvalidTokenError == true:
if source.serverError?.isExpiredTokenError == true {
onExpiredToken()
}
case let .disconnected(source) where source.serverError?.isExpiredTokenError == true:
nuno-vieira marked this conversation as resolved.
Show resolved Hide resolved
onExpiredToken()
shouldNotifyConnectionIdWaiters = false
connectionId = nil
case .disconnected:
Expand Down
21 changes: 14 additions & 7 deletions Sources/StreamChat/Utils/StreamTimer/ScheduledStreamTimer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,30 +5,37 @@
import Foundation

public class ScheduledStreamTimer: StreamTimer {
let interval: TimeInterval
var runLoop = RunLoop.current
var timer: Foundation.Timer?
public var onChange: (() -> Void)?

public var isRunning: Bool {
timer?.isValid ?? false
}
let interval: TimeInterval
let fireOnStart: Bool
let repeats: Bool

public init(interval: TimeInterval) {
public init(interval: TimeInterval, fireOnStart: Bool = true, repeats: Bool = true) {
self.interval = interval
self.fireOnStart = fireOnStart
self.repeats = repeats
}

public var isRunning: Bool {
timer?.isValid ?? false
}

public func start() {
stop()

timer = Foundation.Timer.scheduledTimer(
withTimeInterval: interval,
repeats: true
repeats: repeats
) { _ in
self.onChange?()
}
runLoop.add(timer!, forMode: .common)
timer?.fire()
if fireOnStart {
timer?.fire()
}
}

public func stop() {
Expand Down
16 changes: 10 additions & 6 deletions Sources/StreamChat/WebSocketClient/ConnectionStatus.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,7 @@ extension ConnectionStatus {
self = .disconnecting

case let .disconnected(source):
let isWaitingForReconnect = webSocketConnectionState.isAutomaticReconnectionEnabled || source.serverError?
.isInvalidTokenError == true
nuno-vieira marked this conversation as resolved.
Show resolved Hide resolved

let isWaitingForReconnect = webSocketConnectionState.isAutomaticReconnectionEnabled
self = isWaitingForReconnect ? .connecting : .disconnected(error: source.serverError)
}
}
Expand All @@ -55,10 +53,13 @@ typealias ConnectionId = String
/// A web socket connection state.
enum WebSocketConnectionState: Equatable {
/// Provides additional information about the source of disconnecting.
enum DisconnectionSource: Equatable {
indirect enum DisconnectionSource: Equatable {
/// A user initiated web socket disconnecting.
case userInitiated

/// The connection timed out while trying to connect.
case timeout(from: WebSocketConnectionState)

/// A server initiated web socket disconnecting, an optional error object is provided.
case serverInitiated(error: ClientError? = nil)

Expand Down Expand Up @@ -128,8 +129,9 @@ enum WebSocketConnectionState: Equatable {
return false
}

if serverInitiatedError.isClientError {
// Don't reconnect on client side errors
if serverInitiatedError.isClientError && !serverInitiatedError.isExpiredTokenError {
// Don't reconnect on client side errors unless it is an expired token
// Expired tokens return 401, so it is considered client error.
return false
}
}
Expand All @@ -141,6 +143,8 @@ enum WebSocketConnectionState: Equatable {
return true
case .userInitiated:
return false
case .timeout:
return false
}
}
}
11 changes: 11 additions & 0 deletions Sources/StreamChat/WebSocketClient/WebSocketClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,17 @@ class WebSocketClient {
eventsBatcher.processImmediately(completion: completion)
}
}

func timeout() {
let previousState = connectionState
connectionState = .disconnected(source: .timeout(from: previousState))
engineQueue.async { [engine, eventsBatcher] in
engine?.disconnect()

eventsBatcher.processImmediately {}
}
log.error("Connection timed out. `\(connectionState)", subsystems: .webSocket)
}
}

protocol ConnectionStateDelegate: AnyObject {
Expand Down
Loading
Loading