fix: prevent splash screen from hanging on slow DB#4061
Merged
Conversation
MainViewModel's init gated the splash-dismiss signal on vaultRepository.hasVaults(), which on a cold DB open also runs the full Room migration chain. On slow storage or after multiple beta updates some users report the splash hanging indefinitely, since there is no timeout on that call and any thrown exception leaves _isLoading = true forever. Wrap the query in withTimeoutOrNull(5s), fall back to Route.AddVault on timeout or exception, and route the init block through safeLaunch so non-CancellationException throws no longer crash the process. Unit tests cover the four reachable paths (true, false, timeout, throw).
Contributor
📝 WalkthroughWalkthroughMainViewModel now includes timeout protection for vault existence checks, replacing unsafe coroutine launches with safer alternatives using Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/test/java/com/vultisig/wallet/app/activity/MainViewModelTest.kt (1)
20-22: Match the app test assertion stack.The new file uses
kotlin.testassertions. If Kotest is already available in the app module, switching these imports would keep the new tests aligned with the rest of the app test suite.As per coding guidelines, "
app/src/test/**/*.kt: Use JUnit 5 (Jupiter) test platform for unit tests with Mockk for mocking and Kotest for assertions".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/test/java/com/vultisig/wallet/app/activity/MainViewModelTest.kt` around lines 20 - 22, Tests in MainViewModelTest import kotlin.test assertions (assertEquals/assertFalse/assertTrue); switch to Kotest matchers to match the project convention: replace the kotlin.test imports with Kotest imports (e.g., io.kotest.matchers.shouldBe and io.kotest.matchers.booleans.shouldBeTrue/shouldBeFalse) and change assertion calls — replace assertEquals(expected, actual) with actual shouldBe expected, assertTrue(cond) with cond.shouldBeTrue(), and assertFalse(cond) with cond.shouldBeFalse(); update usages in the test methods accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/vultisig/wallet/app/activity/MainViewModel.kt`:
- Around line 211-216: In openUri(), replace the plain viewModelScope.launch
usage with safeLaunch and replace the direct vaultRepository.hasVaults() call
with the hasAnyVault() helper so the deeplink path gets the same
SPLASH_VAULT_QUERY_TIMEOUT protection as resolveStartDestination();
specifically, change the coroutine builder at the openUri() entry (currently
calling viewModelScope.launch) to use safeLaunch and use hasAnyVault() where
vaultRepository.hasVaults() is invoked so Room initialization is guarded by the
existing timeout logic.
---
Nitpick comments:
In `@app/src/test/java/com/vultisig/wallet/app/activity/MainViewModelTest.kt`:
- Around line 20-22: Tests in MainViewModelTest import kotlin.test assertions
(assertEquals/assertFalse/assertTrue); switch to Kotest matchers to match the
project convention: replace the kotlin.test imports with Kotest imports (e.g.,
io.kotest.matchers.shouldBe and
io.kotest.matchers.booleans.shouldBeTrue/shouldBeFalse) and change assertion
calls — replace assertEquals(expected, actual) with actual shouldBe expected,
assertTrue(cond) with cond.shouldBeTrue(), and assertFalse(cond) with
cond.shouldBeFalse(); update usages in the test methods accordingly.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 778ef082-084d-44d6-a7e6-05754191f1f6
📒 Files selected for processing (2)
app/src/main/java/com/vultisig/wallet/app/activity/MainViewModel.ktapp/src/test/java/com/vultisig/wallet/app/activity/MainViewModelTest.kt
openUri runs after splash dismiss, but the underlying hasVaults() call hits the same code path. If DataStore or Room is still warming up when a deep link opens the app, the deeplink also hangs. Route through hasAnyVault() and safeLaunch so it inherits the same 3-second timeout and crash-safety.
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.
Devs are reporting the splash screen hanging on cold-start for 20+ seconds, sometimes resolving after force-closing and reopening a few times. Release builds strip Timber so there's no app-side signal, but from ADB logs the process is alive and the main thread is responsive — the only thing holding the splash is
MainViewModel's init coroutine waiting onvaultRepository.hasVaults(), which transitively opens the Room DB and can stall for any number of reasons (contended storage, keystore daemon, etc.).Wrap that call in
withTimeout(5.seconds)and treat timeout/exception as "no vaults" so the splash always dismisses intoRoute.AddVaultinstead of hanging. Route the init block and theopenUrideeplink entry throughsafeLaunchso a thrown exception can no longer leave_isLoading = trueforever.