From 6f5f8fa928268fb310c02bb61d0437b654648344 Mon Sep 17 00:00:00 2001 From: Spencer Date: Mon, 20 May 2024 00:02:13 -0400 Subject: [PATCH] Fix #1433 Logging Exceptions to Both Console and Exception Loggers (#5396) ## Explanation Fix #1433 There are 3 TODO marks the place where we want to have both have log in both Console and Exception Loggers By checking 3 related TODOs, only TODO in ExceptionController needs to add the exception logger for logging and I added it as the issue required and removed the TODO comment. The other 2 TODOs already meet the requirement of the issue, so I simply deleted TODO comments. ## Essential Checklist - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only If your PR includes UI-related changes, then: - Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes - For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see [RTL guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines)) - Add a video showing the full UX flow with a screen reader enabled (see [accessibility guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide)) - For PRs introducing new UI elements or color changes, both light and dark mode screenshots must be included - Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing --------- Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com> --- .../android/domain/oppialogger/analytics/AnalyticsController.kt | 1 - .../oppialogger/analytics/PerformanceMetricsController.kt | 1 - .../domain/oppialogger/exceptions/ExceptionsController.kt | 2 +- 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/AnalyticsController.kt b/domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/AnalyticsController.kt index 686ad0c864a..6de1f35fa92 100644 --- a/domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/AnalyticsController.kt +++ b/domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/AnalyticsController.kt @@ -199,7 +199,6 @@ class AnalyticsController @Inject constructor( .addEventLogsToUpload(eventLog) .build() } else { - // TODO(#1433): Refactoring for logging exceptions to both console and exception loggers. val exception = IllegalStateException("Least Recent Event index absent -- EventLogCacheStoreSize is 0") consoleLogger.e("AnalyticsController", "Failure while caching event.", exception) diff --git a/domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/PerformanceMetricsController.kt b/domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/PerformanceMetricsController.kt index 710679e58f3..f518ff6121e 100644 --- a/domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/PerformanceMetricsController.kt +++ b/domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/PerformanceMetricsController.kt @@ -81,7 +81,6 @@ class PerformanceMetricsController @Inject constructor( .addOppiaMetricLog(oppiaMetricLog) .build() } else { - // TODO(#1433): Refactoring for logging exceptions to both console and exception loggers. val exception = IllegalStateException( "Least Recent Event index absent -- MetricLogStorageCacheSize is 0" diff --git a/domain/src/main/java/org/oppia/android/domain/oppialogger/exceptions/ExceptionsController.kt b/domain/src/main/java/org/oppia/android/domain/oppialogger/exceptions/ExceptionsController.kt index 2e1e059f809..026156cf0cd 100644 --- a/domain/src/main/java/org/oppia/android/domain/oppialogger/exceptions/ExceptionsController.kt +++ b/domain/src/main/java/org/oppia/android/domain/oppialogger/exceptions/ExceptionsController.kt @@ -131,12 +131,12 @@ class ExceptionsController @Inject constructor( .addExceptionLog(exceptionLog) .build() } else { - // TODO(#1433): Refactoring for logging exceptions to both console and exception loggers. val exception = NullPointerException( "Least Recent Exception index absent -- ExceptionLogCacheStoreSize is 0" ) consoleLogger.e(EXCEPTIONS_CONTROLLER, exception.toString()) + exceptionLogger.logException(exception) } } return@storeDataAsync oppiaExceptionLogs.toBuilder().addExceptionLog(exceptionLog).build()