Skip to content

Commit 616e206

Browse files
sunnylqmclaude
sunnylqm
and
claude
authored
refactor: unified error handling pipeline with stable cross-platform error codes (#606)
* refactor(js): unified error pipeline with stable error codes 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> * fix(native): error visibility, rollback guards, and cross-platform error 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> * fix: address CodeRabbit review on PR #606 - 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> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent ff55fd3 commit 616e206

20 files changed

Lines changed: 678 additions & 105 deletions

File tree

‎android/src/main/java/cn/reactnative/modules/update/ErrorCodes.java‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package cn.reactnative.modules.update;
2+
3+
/**
4+
* Stable, machine-readable error codes used as the promise rejection code so
5+
* the JS layer and user loggers can aggregate errors across platforms.
6+
*
7+
* MUST stay in sync with cpp/patch_core/error_codes.h (the single source of
8+
* truth) and src/error.ts (UpdateErrorCode). Messages are free-form; only the
9+
* codes are part of the contract.
10+
*/
11+
final class ErrorCodes {
12+
static final String INVALID_OPTIONS = "INVALID_OPTIONS";
13+
static final String DOWNLOAD_FAILED = "DOWNLOAD_FAILED";
14+
static final String PATCH_FAILED = "PATCH_FAILED";
15+
static final String FILE_OPERATION_FAILED = "FILE_OPERATION_FAILED";
16+
static final String SWITCH_VERSION_FAILED = "SWITCH_VERSION_FAILED";
17+
static final String MARK_SUCCESS_FAILED = "MARK_SUCCESS_FAILED";
18+
static final String RESTART_FAILED = "RESTART_FAILED";
19+
static final String INVALID_HASH_INFO = "INVALID_HASH_INFO";
20+
static final String UNSUPPORTED_PLATFORM = "UNSUPPORTED_PLATFORM";
21+
22+
private ErrorCodes() {
23+
}
24+
}

‎android/src/main/java/cn/reactnative/modules/update/StateSerialRunner.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ private StateSerialRunner() {
4343

4444
static void run(
4545
@Nullable final Promise promise,
46+
final String errorCode,
4647
final String operationName,
4748
final Operation operation
4849
) {
@@ -53,7 +54,7 @@ public void run() {
5354
operation.run();
5455
} catch (Throwable error) {
5556
if (promise != null) {
56-
promise.reject(operationName + " failed", error);
57+
promise.reject(errorCode, operationName + " failed", error);
5758
} else {
5859
Log.e(UpdateContext.TAG, operationName + " failed", error);
5960
}

‎android/src/main/java/cn/reactnative/modules/update/UiThreadRunner.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ private UiThreadRunner() {
1515

1616
static void run(
1717
@Nullable final Promise promise,
18+
final String errorCode,
1819
final String operationName,
1920
final Operation operation
2021
) {
@@ -25,7 +26,7 @@ public void run() {
2526
operation.run();
2627
} catch (Throwable error) {
2728
if (promise != null) {
28-
promise.reject(operationName + " failed", error);
29+
promise.reject(errorCode, operationName + " failed", error);
2930
} else {
3031
Log.e(UpdateContext.TAG, operationName + " failed", error);
3132
}

‎android/src/main/java/cn/reactnative/modules/update/UpdateContext.java‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,10 @@ private void applyState(SharedPreferences.Editor editor, StateCoreResult state)
207207
}
208208

209209
private void persistEditor(SharedPreferences.Editor editor, String reason) {
210-
if (!editor.commit() && DEBUG) {
211-
Log.w(TAG, "Failed to persist update state for " + reason);
210+
// A lost state write can mean a missed rollback or a version switch
211+
// that silently never happens, so this must be visible in release too.
212+
if (!editor.commit()) {
213+
Log.e(TAG, "Failed to persist update state for " + reason);
212214
}
213215
}
214216

@@ -367,6 +369,13 @@ public String getBundleUrl(String defaultAssetsUrl) {
367369
ignoreRollback,
368370
true
369371
);
372+
if (launchState.didRollback) {
373+
// The crash-protection rollback: the new version never called
374+
// markSuccess. Keep this visible in release logs.
375+
Log.e(TAG, "Version " + currentState.currentVersion
376+
+ " was not marked as successful, rolling back to "
377+
+ launchState.currentVersion);
378+
}
370379
if (launchState.didRollback || launchState.consumedFirstTime) {
371380
SharedPreferences.Editor editor = sp.edit();
372381
applyState(editor, launchState);
@@ -411,6 +420,8 @@ private String rollBack() {
411420
false,
412421
false
413422
);
423+
Log.e(TAG, "Rolling back version " + currentState.currentVersion
424+
+ " to " + nextState.currentVersion);
414425
SharedPreferences.Editor editor = sp.edit();
415426
applyState(editor, nextState);
416427
persistEditor(editor, "rollback");

‎android/src/main/java/cn/reactnative/modules/update/UpdateModuleImpl.java‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public void onDownloadCompleted(DownloadTaskParams params) {
4141

4242
@Override
4343
public void onDownloadFailed(Throwable error) {
44-
promise.reject(error);
44+
promise.reject(ErrorCodes.DOWNLOAD_FAILED, error);
4545
}
4646
});
4747
}
@@ -64,7 +64,7 @@ public void onDownloadCompleted(DownloadTaskParams params) {
6464

6565
@Override
6666
public void onDownloadFailed(Throwable error) {
67-
promise.reject(error);
67+
promise.reject(ErrorCodes.DOWNLOAD_FAILED, error);
6868
}
6969
});
7070
}
@@ -84,7 +84,7 @@ public void onDownloadCompleted(DownloadTaskParams params) {
8484

8585
@Override
8686
public void onDownloadFailed(Throwable error) {
87-
promise.reject(error);
87+
promise.reject(ErrorCodes.DOWNLOAD_FAILED, error);
8888
}
8989
});
9090
}
@@ -107,11 +107,11 @@ public void onDownloadCompleted(DownloadTaskParams params) {
107107

108108
@Override
109109
public void onDownloadFailed(Throwable error) {
110-
promise.reject(error);
110+
promise.reject(ErrorCodes.DOWNLOAD_FAILED, error);
111111
}
112112
});
113113
} catch (Exception e) {
114-
promise.reject("downloadPatchFromPpk failed: " + e.getMessage());
114+
promise.reject(ErrorCodes.INVALID_OPTIONS, "downloadPatchFromPpk failed: " + e.getMessage(), e);
115115
}
116116
}
117117

@@ -130,7 +130,7 @@ public static void restartApp(
130130
@Nullable final String hash,
131131
final Promise promise
132132
) {
133-
UiThreadRunner.run(promise, "restartApp", new UiThreadRunner.Operation() {
133+
UiThreadRunner.run(promise, ErrorCodes.RESTART_FAILED, "restartApp", new UiThreadRunner.Operation() {
134134
@Override
135135
public void run() throws Throwable {
136136
ReactReloadManager.restartApp(updateContext, reactContext, hash);
@@ -149,7 +149,7 @@ public static void setNeedUpdate(
149149
final Promise promise
150150
) {
151151
final String hash = options.getString("hash");
152-
StateSerialRunner.run(promise, "switchVersionLater", new StateSerialRunner.Operation() {
152+
StateSerialRunner.run(promise, ErrorCodes.SWITCH_VERSION_FAILED, "switchVersionLater", new StateSerialRunner.Operation() {
153153
@Override
154154
public void run() {
155155
setNeedUpdateInternal(updateContext, hash);
@@ -160,7 +160,7 @@ public void run() {
160160

161161
public static void setNeedUpdate(final UpdateContext updateContext, final ReadableMap options) {
162162
final String hash = options.getString("hash");
163-
StateSerialRunner.run(null, "switchVersionLater", new StateSerialRunner.Operation() {
163+
StateSerialRunner.run(null, ErrorCodes.SWITCH_VERSION_FAILED, "switchVersionLater", new StateSerialRunner.Operation() {
164164
@Override
165165
public void run() {
166166
setNeedUpdateInternal(updateContext, hash);
@@ -173,7 +173,7 @@ private static void markSuccessInternal(UpdateContext updateContext) {
173173
}
174174

175175
public static void markSuccess(final UpdateContext updateContext, final Promise promise) {
176-
StateSerialRunner.run(promise, "markSuccess", new StateSerialRunner.Operation() {
176+
StateSerialRunner.run(promise, ErrorCodes.MARK_SUCCESS_FAILED, "markSuccess", new StateSerialRunner.Operation() {
177177
@Override
178178
public void run() {
179179
markSuccessInternal(updateContext);
@@ -183,7 +183,7 @@ public void run() {
183183
}
184184

185185
public static void markSuccess(final UpdateContext updateContext) {
186-
StateSerialRunner.run(null, "markSuccess", new StateSerialRunner.Operation() {
186+
StateSerialRunner.run(null, ErrorCodes.MARK_SUCCESS_FAILED, "markSuccess", new StateSerialRunner.Operation() {
187187
@Override
188188
public void run() {
189189
markSuccessInternal(updateContext);
@@ -200,7 +200,7 @@ public static void setUuid(
200200
final String uuid,
201201
final Promise promise
202202
) {
203-
StateSerialRunner.run(promise, "setUuid", new StateSerialRunner.Operation() {
203+
StateSerialRunner.run(promise, ErrorCodes.FILE_OPERATION_FAILED, "setUuid", new StateSerialRunner.Operation() {
204204
@Override
205205
public void run() {
206206
setUuidInternal(updateContext, uuid);
@@ -210,7 +210,7 @@ public void run() {
210210
}
211211

212212
public static void setUuid(final UpdateContext updateContext, final String uuid) {
213-
StateSerialRunner.run(null, "setUuid", new StateSerialRunner.Operation() {
213+
StateSerialRunner.run(null, ErrorCodes.FILE_OPERATION_FAILED, "setUuid", new StateSerialRunner.Operation() {
214214
@Override
215215
public void run() {
216216
setUuidInternal(updateContext, uuid);
@@ -235,7 +235,7 @@ public static void setLocalHashInfo(
235235
final String info,
236236
final Promise promise
237237
) {
238-
StateSerialRunner.run(promise, "setLocalHashInfo", new StateSerialRunner.Operation() {
238+
StateSerialRunner.run(promise, ErrorCodes.INVALID_HASH_INFO, "setLocalHashInfo", new StateSerialRunner.Operation() {
239239
@Override
240240
public void run() {
241241
setLocalHashInfoInternal(updateContext, hash, info);
@@ -249,7 +249,7 @@ public static void setLocalHashInfo(
249249
final String hash,
250250
final String info
251251
) {
252-
StateSerialRunner.run(null, "setLocalHashInfo", new StateSerialRunner.Operation() {
252+
StateSerialRunner.run(null, ErrorCodes.INVALID_HASH_INFO, "setLocalHashInfo", new StateSerialRunner.Operation() {
253253
@Override
254254
public void run() {
255255
setLocalHashInfoInternal(updateContext, hash, info);
@@ -264,7 +264,7 @@ public static void getLocalHashInfo(
264264
) {
265265
String value = updateContext.getKv("hash_" + hash);
266266
if (!isValidHashInfo(value)) {
267-
promise.reject("getLocalHashInfo failed: invalid json string");
267+
promise.reject(ErrorCodes.INVALID_HASH_INFO, "getLocalHashInfo failed: invalid json string");
268268
return;
269269
}
270270

‎cpp/patch_core/error_codes.h‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
#ifndef PUSHY_PATCH_CORE_ERROR_CODES_H_
2+
#define PUSHY_PATCH_CORE_ERROR_CODES_H_
3+
4+
// Single source of truth for the stable, machine-readable error codes shared
5+
// by every platform. Native modules reject promises with one of these codes
6+
// so the JS layer (src/error.ts UpdateErrorCode) and user loggers can
7+
// aggregate errors across platforms and locales.
8+
//
9+
// Mirrors that cannot include this header MUST stay in sync by hand:
10+
// - src/error.ts (UpdateErrorCode union, JS layer)
11+
// - android/.../ErrorCodes.java (Java constants)
12+
// iOS (RCTPushy.mm) includes this header directly.
13+
//
14+
// Human-readable messages are NOT part of this contract: they may differ per
15+
// platform and locale. Only the codes are stable.
16+
17+
namespace pushy {
18+
namespace error_codes {
19+
20+
// Method options missing or malformed (blank hash/url, wrong types).
21+
constexpr const char* kInvalidOptions = "INVALID_OPTIONS";
22+
// Native download failed (network error, bad HTTP status, truncated body).
23+
constexpr const char* kDownloadFailed = "DOWNLOAD_FAILED";
24+
// Unzip or hdiff patch application failed.
25+
constexpr const char* kPatchFailed = "PATCH_FAILED";
26+
// Local file or state persistence operation failed.
27+
constexpr const char* kFileOperationFailed = "FILE_OPERATION_FAILED";
28+
// switchVersion / setNeedUpdate state transition failed.
29+
constexpr const char* kSwitchVersionFailed = "SWITCH_VERSION_FAILED";
30+
// markSuccess state transition failed.
31+
constexpr const char* kMarkSuccessFailed = "MARK_SUCCESS_FAILED";
32+
// reloadUpdate / restartApp failed.
33+
constexpr const char* kRestartFailed = "RESTART_FAILED";
34+
// Stored or provided hash info is not a valid JSON object.
35+
constexpr const char* kInvalidHashInfo = "INVALID_HASH_INFO";
36+
// The method is not supported on this platform (e.g. downloadAndInstallApk
37+
// outside Android).
38+
constexpr const char* kUnsupportedPlatform = "UNSUPPORTED_PLATFORM";
39+
40+
} // namespace error_codes
41+
} // namespace pushy
42+
43+
#endif // PUSHY_PATCH_CORE_ERROR_CODES_H_

‎harmony/pushy/src/main/ets/UpdateContext.ts‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -468,13 +468,22 @@ export class UpdateContext {
468468
public getBundleUrl() {
469469
UpdateContext.isUsingBundleUrl = true;
470470
this.trace('getBundleUrl:enter');
471+
const stateBeforeLaunch = this.getStateSnapshot();
471472
const launchState = NativePatchCore.runStateCore(
472473
STATE_OP_RESOLVE_LAUNCH,
473-
this.getStateSnapshot(),
474+
stateBeforeLaunch,
474475
'',
475476
UpdateContext.ignoreRollback,
476477
true,
477478
);
479+
if (launchState.didRollback) {
480+
// The crash-protection rollback: the new version never called
481+
// markSuccess. Keep this visible in release logs.
482+
console.error(
483+
`Version ${stateBeforeLaunch.currentVersion} was not marked as successful,` +
484+
` rolled back to ${launchState.currentVersion}`,
485+
);
486+
}
478487
if (launchState.didRollback || launchState.consumedFirstTime) {
479488
this.persistState(launchState, {
480489
markFirstLoadMarker: launchState.consumedFirstTime,
@@ -490,7 +499,12 @@ export class UpdateContext {
490499
);
491500

492501
let version = launchState.loadVersion || '';
493-
while (version) {
502+
// Guard the rollback chain against cycles: a corrupted state returning an
503+
// already-visited version would otherwise spin this loop forever during
504+
// startup (Android has the same guard).
505+
const visitedVersions = new Set<string>();
506+
while (version && !visitedVersions.has(version)) {
507+
visitedVersions.add(version);
494508
const bundleFile = this.getBundlePath(version);
495509
try {
496510
if (!fileIo.accessSync(bundleFile)) {
@@ -514,7 +528,11 @@ export class UpdateContext {
514528
}
515529

516530
private rollBack(): string {
531+
const stateBefore = this.getStateSnapshot();
517532
const nextState = this.runStateOperation(STATE_OP_ROLLBACK);
533+
console.error(
534+
`Rolling back version ${stateBefore.currentVersion} to ${nextState.currentVersion}`,
535+
);
518536
return nextState.currentVersion || '';
519537
}
520538

0 commit comments

Comments
 (0)