Conversation
… use cases
DomainError extends Throwable, so the catch (e: Exception) in ForgotPinUseCase and
SavePinUseCase never matched: a mapped SDK error escaped the use case instead of
becoming Result.failure, and PinViewModel runs them in viewModelScope.launch, so
"Forgot PIN" against a failing SDK crashed the app rather than showing a message.
Both use cases now run on a shared resultOf { } helper in commonskmm, which catches
Throwable and rethrows CancellationException -- the stdlib runCatching swallows it,
which is why it is not a substitute. SavePinUseCase also implements UseCase<String, Unit>,
as AGENTS.md requires; the signature already matched, so callers are unaffected.
SessionRepositoryImpl moves its four inline catch (d2Error: D2Error) blocks onto
withDomainErrors, closing the same gap fixed for sync in #5071: those catches only saw
a bare D2Error, so any failure the SDK's blocking RxJava operators rewrapped in a
RuntimeException escaped savePin / deletePin / logout completely unmapped.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
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
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.



Fixes ANDROAPP-7759
Problem
DomainErrorissealed class DomainError : Throwable(), so thecatch (e: Exception)inForgotPinUseCaseandSavePinUseCasenever matched. A mapped SDK error propagated out of theuse case instead of becoming
Result.failure(...)as theUseCasecontract promises — and sincePinViewModelcalls them insideviewModelScope.launchand only handlesonSuccess/onFailure,"Forgot PIN" against a failing SDK crashed the app instead of showing a message, leaving the
spinner up.
Independently,
CancellationExceptionis anException, so both use cases were swallowingcancellation and reporting it as a failed
Result.Changes
resultOf { }helper incommonskmm/commonMain(org.dhis2.mobile.commons.domain),mirroring the existing
withDomainErrors: catchesThrowableso aDomainErroris reported asa failure, and rethrows
CancellationExceptionso cancellation is never a failure. The stdlibrunCatchingis not a substitute — it swallows cancellation, which is why the idiom was alreadyhand-rolled in
FetchOrgUnits.ktandLoginRepositoryImpl.kt.SavePinUseCasenow implementsUseCase<String, Unit>as AGENTS.md requires; the signature already matched, so callers are unaffected.
SessionRepositoryImplmoves its four inlinecatch (d2Error: D2Error)blocks ontowithDomainErrors. This closes a live gap in the same file: those inline catches only saw a bareD2Error, so any failure the SDK's blocking RxJava operators rewrapped in aRuntimeExceptionescaped
savePin/deletePin/logoutcompletely unmapped — the same bug class fixed forsyncin refactor: [ANDROAPP-7733] Map SDK errors in sync repositories with withDomainErrors #5071. The no-opcatch (e: Exception) { throw e }insetSessionLockedgoes with it.Tests
ResultOfTest(commonskmmcommonTest), extendedForgotPinUseCaseTest/SavePinUseCaseTest(login
commonTest) and a newSessionRepositoryImplTest(loginandroidHostTest) prove that:DomainErrorthrown by the repository comes back asResult.failurecarrying the same instance;SessionRenewalRequiredErrorfromlogout()survives as itself, so the session renewal dialog can still fire from this path;CancellationExceptionpropagates instead of becoming a failedResult;RuntimeException(d2Error)maps as well as a bareD2Error, and an unrelated failure travels on untouched../gradlew :commonskmm:testAndroidHostTest :login:testAndroidHostTest→ 201 passed, 0 failed../gradlew ktlintCheck→ clean.:app:compileDhis2DebugKotlin→ clean.Deliberately out of scope
Two follow-ups the ticket calls out, each wanting its own ticket:
DomainErrorextendExceptionwould repair every site at once, but has a measured blastradius of 42
catch (…: Exception)sites in the KMP modules;LoginRepositoryImpl.kt:136and
:154would silently downgrade a precise error such asSessionRenewalRequiredErrorto ageneric
ConfigurationError.PinViewModelusesviewModelScope.launchinstead oflaunchUseCase, so its coroutines are nottracked by the Espresso idling resource.
🤖 Generated with Claude Code