refactor: unified error handling pipeline with stable cross-platform error codes#606
Merged
Merged
Conversation
Introduce UpdateError { code, cause, extra } and a single client-side
error exit (emitError: report with code + notify onError listeners).
The provider now surfaces lastError/Alert by subscribing to
client.onError, which fixes check/download errors being invisible to
the Provider UI under the default throwError:false, and removes the
duplicated throwError handling between client and provider.
- report() payload gains a stable machine-readable `code` field
(purely additive for user loggers)
- previously unreported failures now report: switchVersion /
switchVersionLater (errorSwitchVersion), markSuccess
(errorMarkSuccess)
- apk errors keep the original native error (message/stack) and use
i18n messages instead of bare event-name strings
- export UpdateError / UpdateErrorCode / onError for user code
- remove the dead error_code locale key; add apk error messages
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ror codes Error visibility / robustness (all platforms): - log the crash-protection rollback (version not marked successful) on all three platforms; it was previously fully silent - Android: persistEditor commit failures now log in release too - iOS/Harmony: guard the startup rollback loop with a visited set (Android already had one) to prevent an infinite spin on corrupted state - iOS: remove the partial version directory when an update fails (parity with Android/Harmony) and verify Content-Length on downloads, skipping encoded (gzip) responses which NSURLSession decompresses transparently Cross-platform error codes (spec: cpp/patch_core/error_codes.h): - Android: unify all 12 promise.reject sites to reject(code, message, throwable); runners take an errorCode param (the operation name was previously passed as the code) - iOS: carry codes via PushyErrorCodeKey in NSError userInfo; classify patch/option/file/download errors; replace the Chinese "json格式校验报错" message with "invalid json string"; implement the spec'd downloadAndInstallApk (UNSUPPORTED_PLATFORM) which previously crashed new-arch callers - Harmony left unchanged: code propagation through RNOH rejections is unverified without a device, and the JS layer stamps fallback codes on every path Verified: javac against RN 0.85.3 AAR, clang -fsyntax-only against Example Pods, harmony strict tsc, 97 JS unit tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR standardizes update error codes across native and JS layers, adds JS error subscription and typed error helpers, and updates rollback handling plus download validation on Android, iOS, and HarmonyOS. ChangesNative error codes and rollback guards
JS error pipeline and provider integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant App
participant PushyClient as Pushy Client
participant UpdateProvider
participant NativeModule as Native Module
App->>UpdateProvider: downloadUpdate()
UpdateProvider->>PushyClient: switchVersion(hash)
PushyClient->>NativeModule: switchVersion(hash)
NativeModule-->>PushyClient: reject(errorCode, message)
PushyClient->>PushyClient: toUpdateError(err, code)
PushyClient->>PushyClient: emitError(eventType, err)
PushyClient-->>UpdateProvider: onError(err, eventType)
UpdateProvider->>UpdateProvider: setLastError and alertError
PushyClient-->>App: rethrow if throwError enabled
sequenceDiagram
participant NativeUpdate as Native getBundleUrl
participant StateStore as Persisted State
NativeUpdate->>StateStore: read stateBeforeLaunch
NativeUpdate->>StateStore: runStateCore(RESOLVE_LAUNCH)
StateStore-->>NativeUpdate: launchState with didRollback
NativeUpdate->>NativeUpdate: log rollback from and to versions
NativeUpdate->>NativeUpdate: add visited version
NativeUpdate->>StateStore: check next version
NativeUpdate->>NativeUpdate: stop on repeated version
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/type.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: .eslintrc.js » 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ios/RCTPushy/RCTPushy.mm (1)
693-701: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPatch-manifest JSON parse error gets mislabeled as
DOWNLOAD_FAILED.This unclassified
NSError(fromNSJSONSerialization) has noPushyErrorCodeKey, so it falls through the newdownloadUpdatefallback (Line 542-547) and gets taggedkDownloadFailed. But this failure happens after a successful download+unzip, during patch manifest parsing — it's a patch failure, not a download failure. The sibling branches at Line 689 and Line 700 already usekPatchFailedfor adjacent manifest issues; this one was missed.🐛 Proposed fix
NSError *error = nil; id jsonObject = [NSJSONSerialization JSONObjectWithData:data options:NSJSONReadingAllowFragments error:&error]; if (error != nil) { - callback(error); + callback(PushyErrorWithCode(pushy::error_codes::kPatchFailed, error.localizedDescription)); return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ios/RCTPushy/RCTPushy.mm` around lines 693 - 701, The JSON parsing failure in `RCTPushy.mm` inside the patch-manifest handling path is being reported as a download error because the `NSError` from `NSJSONSerialization` is passed through unclassified. Update the `JSONObjectWithData` error branch in the patch-manifest parsing logic to wrap or translate that error with `PushyErrorWithCode` using `pushy::error_codes::kPatchFailed`, matching the neighboring manifest validation branches in the same method, so `downloadUpdate` will not fall back to `kDownloadFailed` for this post-download parse failure.
🧹 Nitpick comments (1)
src/type.ts (1)
63-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType
codewithUpdateErrorCodeinstead of plainstring.
error.tsdefinesUpdateErrorCodespecifically to keep error codes stable and safe to aggregate on in a logger, butEventData.codeis typed as a loosestring, so any directreport()/EventDataconstruction outsideemitErrorwon't get compile-time protection against typos in the code value.♻️ Proposed fix
+import type { UpdateErrorCode } from './error'; + export interface EventData { /** Stable machine-readable error code (see UpdateErrorCode); present on error events */ - code?: string; + code?: UpdateErrorCode;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/type.ts` around lines 63 - 65, `EventData.code` is too loosely typed, so direct `report()` or `EventData` construction can use invalid error codes. Update the `EventData` interface in `type.ts` to use `UpdateErrorCode` for `code` instead of `string`, and make sure the type is imported from `error.ts` or re-exported appropriately. Keep `emitError` and any other `EventData` producers aligned with this stricter type so all error code values are compile-time checked.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@ios/RCTPushy/RCTPushy.mm`:
- Around line 693-701: The JSON parsing failure in `RCTPushy.mm` inside the
patch-manifest handling path is being reported as a download error because the
`NSError` from `NSJSONSerialization` is passed through unclassified. Update the
`JSONObjectWithData` error branch in the patch-manifest parsing logic to wrap or
translate that error with `PushyErrorWithCode` using
`pushy::error_codes::kPatchFailed`, matching the neighboring manifest validation
branches in the same method, so `downloadUpdate` will not fall back to
`kDownloadFailed` for this post-download parse failure.
---
Nitpick comments:
In `@src/type.ts`:
- Around line 63-65: `EventData.code` is too loosely typed, so direct `report()`
or `EventData` construction can use invalid error codes. Update the `EventData`
interface in `type.ts` to use `UpdateErrorCode` for `code` instead of `string`,
and make sure the type is imported from `error.ts` or re-exported appropriately.
Keep `emitError` and any other `EventData` producers aligned with this stricter
type so all error code values are compile-time checked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0a5929ae-cef4-4100-ba25-3da0e7b1d73b
📒 Files selected for processing (20)
android/src/main/java/cn/reactnative/modules/update/ErrorCodes.javaandroid/src/main/java/cn/reactnative/modules/update/StateSerialRunner.javaandroid/src/main/java/cn/reactnative/modules/update/UiThreadRunner.javaandroid/src/main/java/cn/reactnative/modules/update/UpdateContext.javaandroid/src/main/java/cn/reactnative/modules/update/UpdateModuleImpl.javacpp/patch_core/error_codes.hharmony/pushy/src/main/ets/UpdateContext.tsios/RCTPushy/RCTPushy.mmios/RCTPushy/RCTPushyDownloader.mmsrc/__tests__/client.test.tssrc/__tests__/provider.render.test.tsxsrc/client.tssrc/core.tssrc/endpoint.tssrc/error.tssrc/index.tssrc/locales/en.tssrc/locales/zh.tssrc/provider.tsxsrc/type.ts
- iOS: classify the patch-manifest JSON parse error as PATCH_FAILED; it previously fell through unclassified and the downloadUpdate fallback mislabeled it DOWNLOAD_FAILED even though the download itself had succeeded - type EventData.code as UpdateErrorCode instead of a loose string for compile-time protection against typo'd codes Co-Authored-By: Claude Fable 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
1 participant
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.
背景
错误处理与上报专项重构（三步全部包含在本 PR）。此前的问题：错误无机器可读的分类（logger 只能按随 locale/平台漂移的 message 字符串聚合）、错误出口四路并行且互相重叠、关键失败路径静默（switchVersion/markSuccess 失败不上报、native 回滚无日志）、三端 reject 形态与文案不一致。
第 1 步：JS 层单一错误管道（commit 1）
UpdateError extends Error { code, cause, extra }+ 稳定错误码集合（src/error.ts）；toUpdateError()对已有 Error 就地附加 code、保持对象身份（afterCheckUpdate等处的错误同一性不变）onError(listener)（返回退订函数）与内部单一出口emitError()（report 带 code + 通知监听器）；report()payload 新增code字段（对用户 logger 纯增量）client.onError：修复默认throwError:false下 check/download 错误既不弹窗也不进lastError的问题；删除 client/provider 两层重复的 throwError 判定switchVersion/switchVersionLater失败 → 新事件errorSwitchVersion；markSuccess失败 →errorMarkSuccessError('errorInstallApk')等三处）UpdateError/UpdateErrorCode/UpdateErrorListener；删除死翻译 keyerror_code第 2 步：native 可见性与健壮性小改（commit 2，不动接口）
Log.e/ iOSRCTLogWarn/ Harmonyconsole.error均记录"从 X 回滚到 Y"persistEditorcommit 失败 release 下也打Log.e（丢状态写=丢回滚/丢切版本）第 3 步：三端错误码统一（commit 2）
cpp/patch_core/error_codes.h（9 个 native 码），iOS 直接 include，Java 镜像ErrorCodes.java，JSUpdateErrorCodeunion 同步扩展；已确认随 npm 包发布reject(code, message, throwable)（此前混用三种形态，Runner 把"switchVersion failed"消息串当 code 传，下载失败 code 是EUNSPECIFIED）PushyErrorCodeKeyuserInfo 机制承载稳定码；hdiff/解包错误归PATCH_FAILED；中文文案"json格式校验报错"改为invalid json string；补实现downloadAndInstallApk（rejectUNSUPPORTED_PLATFORM，修复新架构下 spec 声明但无实现导致误调即崩）e.code，JS 层保留不覆盖（有回归测试）兼容性
code字段为纯增量；新事件类型errorSwitchVersion/errorMarkSuccess对只处理已知类型的 logger 无影响alwaysAlert下 check/apk 错误现在默认可见（不再要求throwError:true）EUNSPECIFIED/消息串变为稳定码——此前的值本就无人可依赖验证
clang++ -fsyntax-only通过（仅存量 deprecation warning）🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
onError) with stable, machine-readable error codes.UpdateError,UpdateErrorCode) and exposed them from the SDK.Bug Fixes
Tests