Repository navigation
Logging: introduce log levels on JS and Android - #70
Merged
Merged
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Recurring LogBox warnings and missing exception stacks still affect update diagnostics.
Review effort: Balanced
Findings: 3
Open (3)
What changed in this PR
This PR gives CodePush’s JavaScript and Android logs severity levels while keeping native and JavaScript output recognizable across the update flow.
Changes:
- Add info, warning, and error logging for JavaScript and Android call sites.
- Improve failure messages and stack traces, and remove a leftover debug log.
- Standardize iOS log formatting and update related test comments.
| File | Description |
|---|---|
| test/test.mts | Updates an iOS log-parsing comment. |
| logging.js | Adds JavaScript log levels and error stacks. |
| ios/CodePush/CodePushUtils.m | Removes the leading log newline. |
| ios/CodePush/CodePushUpdateUtils.m | Uses the prefixed iOS logger. |
| CodePush.js | Applies log levels to update and reporting paths. |
| android/app/src/test/java/com/microsoft/codepush/react/CodePushUpdateManagerTest.kt | Updates a logger test comment. |
| android/app/src/main/java/com/microsoft/codepush/react/SettingsManager.java | Logs metadata warnings. |
| android/app/src/main/java/com/microsoft/codepush/react/FileUtils.java | Logs file-operation warnings. |
| android/app/src/main/java/com/microsoft/codepush/react/CodePushUtils.java | Removes the old Android logger. |
| android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateUtils.java | Logs verification status and failures. |
| android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java | Logs update application and signature warnings. |
| android/app/src/main/java/com/microsoft/codepush/react/CodePushTelemetryManager.java | Logs report-parsing failures. |
| android/app/src/main/java/com/microsoft/codepush/react/CodePushNativeModule.java | Logs bridge and reload events; removes a debug line. |
| android/app/src/main/java/com/microsoft/codepush/react/CodePushLog.kt | Introduces the Android logger. |
| android/app/src/main/java/com/microsoft/codepush/react/CodePushDialog.java | Logs dialog-handler failures. |
| android/app/src/main/java/com/microsoft/codepush/react/CodePush.java | Logs bundle selection and rollback events. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| await withTimeout(reportStatusDownload({ ...remotePackage, downloadDurationMs, status }), REPORT_STATUS_DOWNLOAD_TIMEOUT_MS); | ||
| } catch (err) { | ||
| log(`Report download status failed: ${err}`); | ||
| log.warn(`Failed to report download status. ${err}`); |
Collaborator
Author
There was a problem hiding this comment.
Fixed, added a second err param to log.warn()
| if (!remotePackage || updateShouldBeIgnored) { | ||
| if (updateShouldBeIgnored) { | ||
| log("An update is available, but it is being ignored due to having been previously rolled back."); | ||
| log.warn("An update is available, but it is being ignored due to having been previously rolled back."); |
Collaborator
Author
There was a problem hiding this comment.
Disagree, it's useful to keep logging the reason an available update is not installed for each launch. Makes debugging easier.
This was referenced Sep 29, 2026
- logging.js exports info/warn/error instead of a single log(). warn and error map to console.warn/console.error. error() includes the stack. - Android: new CodePushLog.kt (Log.i/Log.w/Log.e) replaces CodePushUtils.log(). Exception-only call sites now have a context message instead of "[CodePush] Exception". printStackTrace() calls now go through the logger. - iOS: CPLog no longer prepends a newline. The signature verification line uses CPLog, so it has the [CodePush] prefix. - Remove the leftover [MyDebug] line and the untyped CodePush.log export. - "Report status failed" and sync failures now log the error and stack. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ofalvai
force-pushed
the
push-rtrwzqsknzvr
branch
from
September 30, 2026 08:15
afc2546 to
4de04ac
Compare
ofalvai
marked this pull request as ready for review
September 30, 2026 08:20
miklosboros
reviewed
Sep 30, 2026
| retryOnAppResume && retryOnAppResume.remove(); | ||
| } catch (e) { | ||
| log(`Report status failed: ${JSON.stringify(statusReport)}`); | ||
| log.error(`Report status failed: ${JSON.stringify(statusReport)}`, e); |
Contributor
There was a problem hiding this comment.
Routine offline failures now surface as red LogBox errors on every sync and every app resume. Same in line 596.
checkForUpdate → sdk.queryUpdateWithCurrentPackage rejects whenever the device is offline or the CodePush server is unreachable.
Fix: use log.warn for the retried/recoverable network paths (sync's catch-all, tryReportStatus), matching the download-report call; keep error for the non-recoverable ones (callback threw, module not installed).
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.
Why
Part of a series of logging-related PRs, see PR stack. OTA updates are hard to test (by users of this library, not us), so the CodePush logs are often the only proof of what an update did on a device.
Today, every JS line is a
console.logand every Android line is aLog.d, so a failure looks the same as a normal step. Errors lose their stack. Some Android failures print only[CodePush] Exception, or go toprintStackTrace()without setting the right log tag.This PR adds log levels and fixe formatting. It keeps the message text almost the same. The PRs above in the stack change what gets logged.
Changes
info(),warn()anderror()functions to our logging util. The barelog()is gone, so every call site needs to explicitly choose a logging level.CodePushLog.ktreplacesCodePushUtils.log()and uses Android'sLog.i,Log.wandLog.e.CPLogno longer prepends a newline, so there is no blank line before every message. The signature verification line usesCPLognow, so it has the[CodePush]prefix and shows up in the E2E log.Decisions
CPLogwithout levels. The logging util usesNSLog, which doesn't support log levels. The modern replacement is the Swift-only os.Logger, so once we enable non-test Swift usage in this codebase, we can easily implement the same log levels as on Android. I added a TODO comment to the relevant code so that it's going to be a reminder for whoever touches that part of the codebase again.CodePush.logis removed. It was exported but not in the typings. In theory, this is a breaking change, but because it was never exposed in the typings, I think this is acceptable. Also, I don't expect any app to use this exportedCodePush.log()symbol for anything useful, it's absolutely an internal utility.ClassCastExceptionis an info line. It's expected on Expo, so a warning would only be noise.