Add stream-android-core 5.0.0 dependency to video SDK foundation#1725
Open
aleksandar-apostolov wants to merge 2 commits into
Open
Add stream-android-core 5.0.0 dependency to video SDK foundation#1725aleksandar-apostolov wants to merge 2 commits into
aleksandar-apostolov wants to merge 2 commits into
Conversation
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
aleksandar-apostolov
force-pushed
the
gsd/phase-1-02-agp
branch
from
b97e659 to
ac352a9
Compare
aleksandar-apostolov
force-pushed
the
gsd/phase-1-03-core-dep
branch
from
29a9e4f to
cb50d8e
Compare
Contributor
WalkthroughAdds Changesstream-android-core v5 Foundation Wiring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
SDK Size Comparison 📏
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt`:
- Line 299: In the onFailure lambda where the cleanup failure is logged, replace
the raw exception interpolation with a sanitized identifier to prevent potential
exposure of sensitive information like tokens or call IDs. Instead of
interpolating the full exception object `$e` in the logger.w call, use the
exception class name via `e::class.simpleName` or similar stable identifier to
maintain security compliance while still providing useful debugging context.
- Around line 297-300: The cleanup() method in StreamVideoClient contains a
runBlocking call that executes streamClient.disconnect() without any timeout
protection, which can cause the main thread to hang indefinitely. Wrap the
streamClient.disconnect() call with withTimeoutOrNull(2000L) to enforce a
2-second timeout on the disconnect operation. Add logging to indicate when the
timeout occurs so that if disconnect takes longer than the timeout period, it
will fail gracefully and log the timeout condition instead of blocking the main
thread.
🪄 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: 41922916-d105-4e2d-8fc0-8be22b8d558e
📒 Files selected for processing (5)
build-logic/convention/src/main/kotlin/io/getstream/video/KotlinAndroid.ktgradle/libs.versions.tomlstream-video-android-core/build.gradle.ktsstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/ActiveStateGate.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt
Comment on lines
+297
to
+300
| runBlocking { | ||
| runCatching { streamClient.disconnect() } | ||
| .onFailure { e -> logger.w { "[cleanup] StreamClient disconnect failed: $e" } } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify blocking cleanup path and timeout presence around disconnect.
rg -nC3 --type=kt 'runBlocking|disconnect\(' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt
rg -n --type=kt 'withTimeout|withTimeoutOrNull' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt
rg -nC2 --type=kt '\bcleanup\(\)' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideo.ktRepository: GetStream/stream-video-android
Length of output: 155
🏁 Script executed:
#!/bin/bash
# Read the file at lines 297-300
echo "=== StreamVideoClient.kt lines 290-310 ==="
sed -n '290,310p' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt
# Search for runBlocking and disconnect in the file
echo -e "\n=== Search for runBlocking and disconnect ==="
grep -nC3 'runBlocking\|disconnect(' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt
# Search for timeout usage
echo -e "\n=== Search for timeout handling ==="
grep -n 'withTimeout\|withTimeoutOrNull' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt
# Find cleanup method definition
echo -e "\n=== Search for cleanup() method definition ==="
grep -nC5 'fun cleanup()' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt
# Check imports
echo -e "\n=== Check imports ==="
head -30 stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.ktRepository: GetStream/stream-video-android
Length of output: 3970
🏁 Script executed:
#!/bin/bash
# Check full imports for Dispatchers
echo "=== Check for Dispatchers import ==="
grep -n 'Dispatchers' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt
# Find where cleanup() is called
echo -e "\n=== Where cleanup() is called ==="
grep -rn '\.cleanup()' stream-video-android-core/src/main/kotlin/ | head -20
# Check interface definition (likely implementing something)
echo -e "\n=== StreamVideoClient class declaration ==="
grep -A2 'class StreamVideoClient' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt | head -5
# Check if cleanup is from interface
echo -e "\n=== Check for interface with cleanup ==="
grep -rn 'interface.*{' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/ | xargs -I {} sh -c 'grep -l "cleanup()" {} 2>/dev/null && grep -A30 "interface" {} 2>/dev/null | head -40'
# List interfaces/classes in same package
echo -e "\n=== Other kotlin files in core package ==="
find stream-video-android-core/src/main/kotlin/io/getstream/video/android/core -maxdepth 1 -name '*.kt' -type f | head -10Repository: GetStream/stream-video-android
Length of output: 2933
🏁 Script executed:
#!/bin/bash
# Check StreamVideo.kt cleanup call context
echo "=== StreamVideo.kt around cleanup call (line 234) ==="
sed -n '225,245p' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideo.kt
# Check class declaration for StreamVideoClient to see what it implements
echo -e "\n=== Full StreamVideoClient class declaration ==="
sed -n '1,80p' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt | grep -A10 'class StreamVideoClient'
# Look for interface definition in same directory
echo -e "\n=== Check StreamVideoImpl or similar ==="
find stream-video-android-core/src/main/kotlin/io/getstream/video/android/core -name '*StreamVideo*' -type f
# Check disconnect signature in StreamClient
echo -e "\n=== Search for disconnect definition/usage ==="
grep -rn 'fun disconnect' stream-video-android-core/src/main/ | head -5Repository: GetStream/stream-video-android
Length of output: 2615
🏁 Script executed:
#!/bin/bash
# Check streamClient type and disconnect method signature
echo "=== StreamVideoClient - streamClient definition ==="
grep -n 'streamClient' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt | head -10
# Get full import section
echo -e "\n=== All imports in StreamVideoClient ==="
sed -n '1,170p' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt | tail -50
# Check the StreamClient from stream-android-core
echo -e "\n=== Check if StreamClient.disconnect is suspend ==="
grep -rn 'class StreamClient' stream-video-android-core/src/main/kotlin/ | head -3
# Verify actual interface that cleanup() overrides
echo -e "\n=== StreamVideoClient - look at wider context of cleanup ==="
sed -n '280,310p' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.ktRepository: GetStream/stream-video-android
Length of output: 4240
🏁 Script executed:
#!/bin/bash
# Check StreamClient source to understand disconnect signature
echo "=== Find StreamClient class definition ==="
find . -name '*.kt' -type f | xargs grep -l 'class StreamClient' | head -5
# Check if streamClient.disconnect is suspend or not
echo -e "\n=== Check StreamClient.disconnect in stream-android-core ==="
find . -name '*.java' -o -name '*.kt' | xargs grep -B2 -A2 'suspend fun disconnect\|fun disconnect()' | head -40
# Check what type streamClient is
echo -e "\n=== StreamVideoClient - streamClient declaration ==="
sed -n '270,280p' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt
# Get surrounding context
echo -e "\n=== Check how streamClient is created/obtained ==="
grep -n 'streamClient =' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.ktRepository: GetStream/stream-video-android
Length of output: 3985
🏁 Script executed:
#!/bin/bash
# Check coreClientHolder to understand StreamClient source
echo "=== StreamVideoClient - coreClientHolder and StreamClient import ==="
sed -n '1,30p' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt | grep -E 'import.*StreamClient|import.*stream-android'
# Get full coreClientHolder definition
echo -e "\n=== coreClientHolder definition ==="
grep -B5 -A5 'coreClientHolder' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt | head -30
# Check if StreamClient.disconnect blocks or is actually suspend
echo -e "\n=== Check stream-android-core gradle dependency ==="
find . -name 'build.gradle.kts' -o -name 'build.gradle' | xargs grep -i 'stream.*android.*core' | head -5
# Verify cleanup() is public/override
echo -e "\n=== Verify cleanup() method ==="
sed -n '289,302p' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.ktRepository: GetStream/stream-video-android
Length of output: 3063
🏁 Script executed:
#!/bin/bash
# Check what interface cleanup() overrides
echo "=== Search for interface with cleanup() method ==="
grep -rn 'interface.*{' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/ --include='*.kt' | while read -r line; do
file=$(echo "$line" | cut -d: -f1)
grep -q 'fun cleanup()' "$file" 2>/dev/null && echo "$file"
done
# Check CallState.kt and other files that call cleanup
echo -e "\n=== Check CallState or interface for cleanup ==="
find stream-video-android-core/src/main/kotlin -name '*.kt' | xargs grep -l 'fun cleanup()' | head -5
# Check if cleanup is called from main thread context
echo -e "\n=== Check removeClient context in StreamVideo ==="
sed -n '225,240p' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideo.kt
# Check if there's any annotation on cleanup
echo -e "\n=== Full cleanup method with annotations ==="
sed -n '285,310p' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.ktRepository: GetStream/stream-video-android
Length of output: 2800
🏁 Script executed:
#!/bin/bash
# Critical: Check if StreamClient (from stream-android-core) has suspend disconnect or regular disconnect
echo "=== Find StreamClient in stream-android-core dependencies ==="
find . -type f -name '*.jar' | head -5
# Check pom.xml or gradle for dependency version
echo -e "\n=== Check gradle dependencies for stream-android-core ==="
grep -rn 'stream-android-core' . --include='*.gradle.kts' --include='*.gradle' --include='pom.xml' | head -10
# Since it's external, check if it's documented in the code
echo -e "\n=== Check how streamClient.disconnect() is called elsewhere ==="
grep -rn 'streamClient\.disconnect' stream-video-android-core/src/main/kotlin/
# Check documentation or comments about StreamClient
echo -e "\n=== Search for StreamClient documentation/comments ==="
grep -B3 -A3 'StreamClient' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt | head -40Repository: GetStream/stream-video-android
Length of output: 2255
Add timeout to disconnect() in cleanup() to prevent indefinite main thread blocking.
Line 297 executes streamClient.disconnect() inside runBlocking without a timeout. Since cleanup() is called from the public StreamVideo.removeClient() method (likely on the main thread), an indefinite stall in disconnect will hang the thread and risk an ANR.
Per coding guidelines, cleanup/teardown paths must handle cancellation and failure, and critical RTC operations must not block the main thread indefinitely. Wrap the disconnect call in withTimeoutOrNull(2000L) and log if the timeout occurs.
Suggested fix
+import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.suspendCancellableCoroutine
import kotlinx.coroutines.withContext
import kotlinx.coroutines.withTimeoutOrNull
@@
- runBlocking {
- runCatching { streamClient.disconnect() }
- .onFailure { e -> logger.w { "[cleanup] StreamClient disconnect failed: $e" } }
+ runBlocking(Dispatchers.IO) {
+ val disconnected = withTimeoutOrNull(2_000L) {
+ runCatching { streamClient.disconnect() }
+ .onFailure { e ->
+ logger.w { "[cleanup] StreamClient disconnect failed (${e::class.simpleName})" }
+ }
+ true
+ } ?: false
+ if (!disconnected) {
+ logger.w { "[cleanup] StreamClient disconnect timed out" }
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| runBlocking { | |
| runCatching { streamClient.disconnect() } | |
| .onFailure { e -> logger.w { "[cleanup] StreamClient disconnect failed: $e" } } | |
| } | |
| runBlocking(Dispatchers.IO) { | |
| val disconnected = withTimeoutOrNull(2_000L) { | |
| runCatching { streamClient.disconnect() } | |
| .onFailure { e -> logger.w { "[cleanup] StreamClient disconnect failed (${e::class.simpleName})" } } | |
| true | |
| } ?: false | |
| if (!disconnected) { | |
| logger.w { "[cleanup] StreamClient disconnect timed out" } | |
| } | |
| } |
🤖 Prompt for 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.
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt`
around lines 297 - 300, The cleanup() method in StreamVideoClient contains a
runBlocking call that executes streamClient.disconnect() without any timeout
protection, which can cause the main thread to hang indefinitely. Wrap the
streamClient.disconnect() call with withTimeoutOrNull(2000L) to enforce a
2-second timeout on the disconnect operation. Add logging to indicate when the
timeout occurs so that if disconnect takes longer than the timeout period, it
will fail gracefully and log the timeout condition instead of blocking the main
thread.
Source: Coding guidelines
| // rationale: best-effort dispose at SDK teardown — exceptions must not propagate | ||
| runBlocking { | ||
| runCatching { streamClient.disconnect() } | ||
| .onFailure { e -> logger.w { "[cleanup] StreamClient disconnect failed: $e" } } |
Contributor
There was a problem hiding this comment.
Avoid logging raw exception text in cleanup warnings.
Line 299 interpolates $e directly. Exception strings from network/auth layers can contain sensitive values; log a stable class/code instead.
As per coding guidelines, “Sanitize logs to avoid dumping JWTs, ICE tokens, or call IDs in verbose logs.”
💡 Proposed fix
- .onFailure { e -> logger.w { "[cleanup] StreamClient disconnect failed: $e" } }
+ .onFailure { e ->
+ logger.w { "[cleanup] StreamClient disconnect failed (${e::class.simpleName})" }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| .onFailure { e -> logger.w { "[cleanup] StreamClient disconnect failed: $e" } } | |
| .onFailure { e -> | |
| logger.w { "[cleanup] StreamClient disconnect failed (${e::class.simpleName})" } | |
| } |
🤖 Prompt for 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.
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt`
at line 299, In the onFailure lambda where the cleanup failure is logged,
replace the raw exception interpolation with a sanitized identifier to prevent
potential exposure of sensitive information like tokens or call IDs. Instead of
interpolating the full exception object `$e` in the logger.w call, use the
exception class name via `e::class.simpleName` or similar stable identifier to
maintain security compliance while still providing useful debugging context.
Source: Coding guidelines
aleksandar-apostolov
force-pushed
the
gsd/phase-1-02-agp
branch
from
ac352a9 to
7cb0bb1
Compare
aleksandar-apostolov
force-pushed
the
gsd/phase-1-03-core-dep
branch
from
ea604e2 to
30bd8b0
Compare
The existing 'androidx.lifecycle.AtomicReference' import resolved incorrectly under the prior classpath: the existing compiled class file uses java.util.concurrent.atomic.AtomicReference<Job> for both backing fields. With androidx.lifecycle bumping to 2.9.x, the import would resolve to a lifecycle-internal KMP helper that only exposes get() and compareAndSet() (no zero-arg constructor, no set()), causing 'Unresolved reference set' compile errors. Switch to the JDK type explicitly so behaviour matches the existing bytecode and is robust against future lifecycle bumps.
- Add streamCore = "5.0.0" and stream-android-core library to the version catalog (alphabetic slot beside the other Stream entries). - Declare api(libs.stream.android.core) on stream-video-android-core so downstream modules see core v5 types transitively (Phase 2 needs this scope; PR3 itself is wiring-only and produces no traffic).
aleksandar-apostolov
force-pushed
the
gsd/phase-1-03-core-dep
branch
from
30bd8b0 to
8f3bd4a
Compare
aleksandar-apostolov
changed the title
|
PratimMallick
approved these changes
Jun 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Goal
Closes AND-1242 — third and final step of the V2 migration's build toolchain modernization. Adds
io.getstream:stream-android-core:5.0.0as anapidependency onstream-video-android-core, making the core v5 toolkit available on the classpath. No usage in this PR — Phase 2 wires the real coordinator socket swap andStreamClientlifecycle on top.Stacked on #1723 (AGP 8.10.1). Merge after parent.
Sequential stack on
develop-v2: Kotlin 2.2.0 (#1722) → AGP 8.10.1 (#1723) → this PR.Implementation
gradle/libs.versions.tomlstreamCore = "5.0.0"+stream-android-corelibrary entrystream-video-android-core/build.gradle.ktsapi(libs.stream.android.core)—apiscope so Phase 2+ sees core v5 types transitively through the SDKstream-video-android-core/src/main/kotlin/.../ActiveStateGate.ktandroidx.lifecycle.AtomicReferencetojava.util.concurrent.atomic.AtomicReference. core v5 transitively upgradesandroidx.lifecycleto 2.9.x where the old lifecycle-internal type no longer exists. Bytecode-equivalent.StreamVideoClient.ktis untouched in this PR.StreamClientinstantiation, scope wiring, and lifecycle disposal land in Phase 2 alongside real coordinator traffic.Testing
./gradlew :stream-video-android-core:dependencies --configuration releaseRuntimeClasspath | grep stream-android-core:5.0.0: resolves../gradlew compileDebugKotlinacross all modules: green../gradlew testDebugUnitTest: zero failures.@Ignorebaseline holds at 21../gradlew spotlessCheck: green../gradlew apiCheck: green..apidiff EMPTY — no public surface change (no consumer references core v5 types yet).