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

Fix app auto-reconnect on background #1170

Merged
merged 2 commits into from
Jun 10, 2021
Merged

Fix app auto-reconnect on background #1170

merged 2 commits into from
Jun 10, 2021

Conversation

b-onc
Copy link
Contributor

@b-onc b-onc commented Jun 9, 2021

Also moves the handling logic from WSClient to ChatClient.

Basically, when we disconnected, we automatically set a timer for reconnection, and reconnected while in background.

@b-onc b-onc added the 🌐 SDK: StreamChat (LLC) Tasks related to the StreamChat LLC SDK label Jun 9, 2021
@b-onc b-onc requested a review from VojtaStavik June 9, 2021 13:00
@b-onc b-onc marked this pull request as draft June 9, 2021 13:00
@b-onc b-onc force-pushed the fix-reconnection branch 4 times, most recently from e5eeeeb to 5aa1f5e Compare June 9, 2021 13:04
cancelBackgroundTaskIfNeeded()

guard config.shouldConnectAutomatically else { return }
guard connectionStatus != .connected && connectionStatus != .connected else {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we should maybe add one more && connectionStatus != .connected to be 300% sure :trollface:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let confidenceChecks = (1...10).random()
for _ in confidenceChecks {
  guard connectionStatus != .connected else { return }
}

@b-onc b-onc force-pushed the fix-reconnection branch 3 times, most recently from dbc11f8 to 45cc880 Compare June 9, 2021 18:20
@b-onc b-onc marked this pull request as ready for review June 9, 2021 18:20
@Stream-SDK-Bot
Copy link
Collaborator

Stream-SDK-Bot commented Jun 9, 2021

1 Warning
⚠️ Big PR

Generated by 🚫 Danger

Copy link
Contributor

@VojtaStavik VojtaStavik left a comment

Choose a reason for hiding this comment

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

Great refactoring 👍 Please add CHANGELOG

Comment on lines +69 to +70
/// If set to `true`, the `ChatClient` will try to stay connected while app is backgrounded.
/// If set to `false`, websocket disconnects immediately when app is backgrounded.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Would be fine to mention that the client starts starts a background task for this

@b-onc b-onc force-pushed the fix-reconnection branch from 65dd2c2 to 7c56669 Compare June 10, 2021 08:04
b-onc added 2 commits June 10, 2021 11:10
Also moves the handling logic from WSClient to ChatClient.

Basically, when we disconnected, we automatically set a timer for reconnection, and reconnected while in background.
@b-onc b-onc force-pushed the fix-reconnection branch from 7c56669 to 8a14e0e Compare June 10, 2021 08:10
@codecov-commenter
Copy link

codecov-commenter commented Jun 10, 2021

Codecov Report

Merging #1170 (8a14e0e) into main (3462f04) will decrease coverage by 0.07%.
The diff coverage is 96.07%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1170      +/-   ##
==========================================
- Coverage   91.31%   91.24%   -0.08%     
==========================================
  Files         217      217              
  Lines        9248     9261      +13     
==========================================
+ Hits         8445     8450       +5     
- Misses        803      811       +8     
Flag Coverage Δ
llc-tests 91.24% <96.07%> (-0.08%) ⬇️
llc-tests-ios12 ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
Sources/StreamChat/Config/ChatClientConfig.swift 100.00% <ø> (ø)
Sources/StreamChat/ChatClient.swift 96.87% <95.23%> (-0.28%) ⬇️
...ocketClient/Engine/URLSessionWebSocketEngine.swift 58.33% <100.00%> (+4.84%) ⬆️
...s/StreamChat/WebSocketClient/WebSocketClient.swift 89.43% <100.00%> (-0.38%) ⬇️
Sources/StreamChat/Workers/ChatClientUpdater.swift 97.91% <100.00%> (ø)
.../StreamChat/WebSocketClient/ConnectionStatus.swift 77.77% <0.00%> (-22.23%) ⬇️
...eamChat/APIClient/Endpoints/Payloads/RawJSON.swift 73.80% <0.00%> (-11.91%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3462f04...8a14e0e. Read the comment docs.

@b-onc b-onc merged commit 3737c49 into main Jun 10, 2021
@b-onc b-onc deleted the fix-reconnection branch June 10, 2021 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌐 SDK: StreamChat (LLC) Tasks related to the StreamChat LLC SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants