Skip to content

[MS-1251] Make Simber.w throwable argument non-optional to avoid lumping all warnings together in Crashlytics#1467

Merged
BurningAXE merged 1 commit into
mainfrom
feature/MS-1251-Unlump-warnings
Nov 24, 2025
Merged

[MS-1251] Make Simber.w throwable argument non-optional to avoid lumping all warnings together in Crashlytics#1467
BurningAXE merged 1 commit into
mainfrom
feature/MS-1251-Unlump-warnings

Conversation

@BurningAXE
Copy link
Copy Markdown
Contributor

@BurningAXE BurningAXE commented Nov 13, 2025

JIRA ticket
Will be released in: 2026.1.0

Root cause analysis (for bugfixes only)

First known affected version: forever

  • Currently all Simber.w logs are wrapped in a generic Exception which in Crashlytics they are all lumped together into this single issue with 10s of variants. This is very impractical for monitoring and risks hiding real issues among benign ones.

Notable changes

  • Makes the Throwable parameter in Simber.w() non-optional in order to force callers to pass a Throwable argument. This will group different call sites together
  • All current Simber.w() calls were provided an appropriate exception

Testing guidance

  • I guess we'll have to either wait this to reach prod or create a staging build and force some warning scenarios to happen

Additional work checklist

  • Effect on other features and security has been considered
  • Design document marked as "In development" (if applicable)
  • External (Gitbook) and internal (Confluence) Documentation is up to date (or ticket created)
  • Test cases in Testiny are up to date (or ticket created)
  • Other teams notified about the changes (if applicable)

@cla-bot cla-bot Bot added the ... label Nov 13, 2025
@BurningAXE BurningAXE force-pushed the feature/MS-1251-Unlump-warnings branch from d468f76 to cc50c9a Compare November 13, 2025 17:40
@BurningAXE BurningAXE marked this pull request as ready for review November 13, 2025 17:59
@BurningAXE BurningAXE requested review from a team, TristramN, alex-vt, alexandr-simprints, Copilot, luhmirin-s, meladRaouf and ybourgery and removed request for a team November 14, 2025 16:43
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses a long-standing issue where all warning logs in Crashlytics were grouped together into a single generic issue, making it difficult to monitor and triage different warning scenarios. The solution makes the Throwable parameter in Simber.w() non-optional, forcing callers to provide specific exceptions that will enable proper grouping in Crashlytics.

Key changes:

  • Modified Simber.w() to require a non-null Throwable parameter
  • Updated all existing call sites to provide appropriate exception types with descriptive messages
  • Simplified the internal logging logic by removing the null-handling branch

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
infra/logging/src/main/java/com/simprints/infra/logging/Simber.kt Made Throwable parameter non-optional and simplified internal logic to handle only non-null cases
infra/logging/src/test/java/com/simprints/infra/logging/writers/CrashlyticsLogWriterTest.kt Removed test for deprecated null-Throwable behavior
infra/ui-base/src/main/java/com/simprints/infra/uibase/navigation/NavigationResultExt.kt Added IllegalStateException for navigation failure warning
infra/matching/src/main/java/com/simprints/infra/matching/usecase/FingerprintMatcherUseCase.kt Added IllegalArgumentException for missing SDK warning
infra/matching/src/main/java/com/simprints/infra/matching/usecase/FaceMatcherUseCase.kt Added IllegalArgumentException for missing SDK warning
infra/event-sync/src/main/java/com/simprints/infra/eventsync/event/commcare/CommCareEventDataSource.kt Added IllegalArgumentException for date parsing failure warning
feature/orchestrator/src/main/java/com/simprints/feature/orchestrator/usecases/steps/FallbackToCommCareDataSourceIfNeededUseCase.kt Added Exception for CommCare fallback scenario warning
infra/event-sync/src/test/java/com/simprints/infra/eventsync/event/commcare/CommCareEventDataSourceTest.kt Updated test assertions to verify Throwable parameter is provided

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread infra/logging/src/main/java/com/simprints/infra/logging/Simber.kt
@BurningAXE BurningAXE force-pushed the feature/MS-1251-Unlump-warnings branch from cc50c9a to 76d5544 Compare November 24, 2025 11:52
@BurningAXE BurningAXE force-pushed the feature/MS-1251-Unlump-warnings branch from 76d5544 to 3cf5521 Compare November 24, 2025 13:55
@sonarqubecloud
Copy link
Copy Markdown

@BurningAXE BurningAXE merged commit 8bd07e6 into main Nov 24, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants