Skip to content

e2e: Disconnect the previous ChatClient when the app re-initializes the SDK#6585

Merged
andremion merged 5 commits into
developfrom
andrerego/and-1321-disconnect-the-previous-chatclient-when-the-e2e-app-re
Jul 23, 2026
Merged

e2e: Disconnect the previous ChatClient when the app re-initializes the SDK#6585
andremion merged 5 commits into
developfrom
andrerego/and-1321-disconnect-the-previous-chatclient-when-the-e2e-app-re

Conversation

@andremion

@andremion andremion commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Goal

In the e2e flavor, every harness launch of StartupActivity calls ChatHelper.initializeSdk, which builds a new ChatClient without disconnecting the previous one. The app process survives across tests and retry attempts, so each launch added one more live client. In the 2026-07-21 nightly failure of test_offlineMessageInTheMessageList, attempt 3 had three clients reconnecting and calling /sync at the same time, including against the previous attempts' mock servers. The extra clients contend on reconnect, the active client's /sync round trip stalls, and SyncManager restores channels only after the /sync response, so recovery lands outside the assertion window and the test fails all retries.

Resolves AND-1321

Implementation

In the e2e StartupActivity, when the launch carries a BASE_URL extra (every harness launch), disconnect the current client with ChatClient.instance().disconnect(flushPersistence = true).await() before calling ChatHelper.initializeSdk. Since the disconnect is a suspend call, the onCreate body now runs inside lifecycleScope.launch.

Notes on the paths that stay unchanged:

  • The notification-tap launch (no BASE_URL) has no suspension point, and lifecycleScope uses Dispatchers.Main.immediate, so that branch still runs synchronously inside onCreate.
  • On the first launch in a fresh process no user is connected, so the disconnect returns a logged failure with no side effects.
  • I did not reuse ChatHelper.disconnectUser() on purpose: it stops SharedLocationService, and SharedLocationService.stop() cancels its coroutine scope permanently, which would break later attempts in the same process.

Testing

This changes only the e2e flavor startup path, so the e2e suite is the verification:

  1. Run any compose sample e2e test locally (bundle exec fastlane build_and_run_e2e_test) and confirm it still passes.
  2. To verify the fix itself, run a test twice in the same process (for example, force a failure so RetryRule retries) and check logcat: after the second launch there should be a [disconnect] log from the previous client, and only one client should reconnect and call /sync against the current mock server.
  3. The real signal is the nightly e2e run: test_offlineMessageInTheMessageList should stop failing on retries with multiple clients alive.

Summary by CodeRabbit

  • Bug Fixes
    • Improved startup initialization and navigation reliability.
    • Ensured existing chat connections are fully disconnected before reinitializing.
    • Preserved test-specific routing and standard navigation behavior.
    • Improved activity completion handling during startup.

@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

PR checklist ✅

All required conditions are satisfied:

  • Title length is OK (or ignored by label).
  • At least one pr: label exists.
  • Sections ### Goal, ### Implementation, and ### Testing are filled, or the PR is bot-authored.
  • An issue is linked (Linear ticket or GitHub issue), or the PR is bot-authored.

🎉 Great job! This PR is ready for review.

@andremion andremion added the pr:test Test-only changes label Jul 22, 2026
@andremion

Copy link
Copy Markdown
Contributor Author

@CodeRabbit review

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

StartupActivity now runs initialization and navigation asynchronously, disconnects the existing ChatClient before reinitializing when configured, preserves existing routing paths, and finishes after coroutine work completes.

Changes

Startup flow

Layer / File(s) Summary
Coroutine-based startup and navigation
stream-chat-android-compose-sample/src/e2e/.../StartupActivity.kt
Lifecycle-scoped startup now awaits ChatClient disconnection before SDK initialization, retains intent or task-stack navigation, and calls finish() after the flow completes.

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

I’m a bunny watching startup hop,
Awaiting disconnect before the stop.
SDK blooms, links preview bright,
Channels stack into place just right.
Then off I bounce—goodnight!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change: disconnecting the previous ChatClient before SDK re-init in e2e startup.
Description check ✅ Passed The core Goal, Implementation, and Testing sections are present and detailed; only optional UI/checklist/GIF sections are missing.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch andrerego/and-1321-disconnect-the-previous-chatclient-when-the-e2e-app-re

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@stream-chat-android-compose-sample/src/e2e/java/io/getstream/chat/android/compose/sample/ui/StartupActivity.kt`:
- Around line 37-67: Add regression tests for StartupActivity covering a fresh
BASE_URL launch, a retry that awaits ChatClient.instance().disconnect before
ChatHelper.initializeSdk, and a notification launch without BASE_URL that
preserves the existing client. Verify each branch’s initialization and
navigation behavior using the existing test conventions and relevant
StartupActivity symbols.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 196def97-6ffd-443a-a76a-154c9a2c715f

📥 Commits

Reviewing files that changed from the base of the PR and between bbc01ad and 974e6e5.

📒 Files selected for processing (1)
  • stream-chat-android-compose-sample/src/e2e/java/io/getstream/chat/android/compose/sample/ui/StartupActivity.kt

@github-actions

Copy link
Copy Markdown
Contributor

SDK Size Comparison 📏

SDK Before After Difference Status
stream-chat-android-client 5.95 MB 5.95 MB 0.00 MB 🟢
stream-chat-android-ui-components 11.21 MB 11.21 MB 0.00 MB 🟢
stream-chat-android-compose 12.69 MB 12.69 MB 0.00 MB 🟢

@andremion
andremion marked this pull request as ready for review July 22, 2026 11:05
@andremion
andremion requested a review from a team as a code owner July 22, 2026 11:05
@andremion
andremion enabled auto-merge (squash) July 23, 2026 09:56
@sonarqubecloud

Copy link
Copy Markdown

@andremion
andremion merged commit 7b95d3f into develop Jul 23, 2026
23 of 24 checks passed
@andremion
andremion deleted the andrerego/and-1321-disconnect-the-previous-chatclient-when-the-e2e-app-re branch July 23, 2026 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:test Test-only changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants