From a49c9a5ea974249cae9eae9fea5e06f804a01402 Mon Sep 17 00:00:00 2001 From: sunnylqm Date: Tue, 7 Jul 2026 19:42:25 +0800 Subject: [PATCH 1/3] 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 --- src/__tests__/client.test.ts | 169 +++++++++++++++++++++++++ src/__tests__/provider.render.test.tsx | 27 +++- src/client.ts | 147 +++++++++++++++++---- src/core.ts | 4 +- src/endpoint.ts | 6 +- src/error.ts | 68 ++++++++++ src/index.ts | 3 + src/locales/en.ts | 6 +- src/locales/zh.ts | 4 +- src/provider.tsx | 74 +++++++---- src/type.ts | 4 + 11 files changed, 452 insertions(+), 60 deletions(-) create mode 100644 src/error.ts diff --git a/src/__tests__/client.test.ts b/src/__tests__/client.test.ts index 81165307..8a179ad1 100644 --- a/src/__tests__/client.test.ts +++ b/src/__tests__/client.test.ts @@ -445,6 +445,151 @@ describe('Pushy server config', () => { }); }); +describe('error pipeline (onError + stable codes, EH-1/EH-2/EH-3)', () => { + test('check failure emits a coded error to onError listeners without throwing (throwError:false)', async () => { + setupClientMocks(); + const logger = mock(() => {}); + (globalThis as any).fetch = mock(async () => { + throw new Error('offline'); + }); + + const { Pushy } = await importFreshClient('pipeline-check-failed'); + const client = new Pushy({ appKey: 'demo-app', logger }); + const seen: any[] = []; + client.onError((e: any, eventType: string) => { + seen.push({ e, eventType }); + }); + + // Default throwError:false — resolves undefined instead of throwing. + expect(await client.checkUpdate()).toBeUndefined(); + + expect(seen).toHaveLength(1); + expect(seen[0].eventType).toBe('errorChecking'); + expect(seen[0].e.code).toBe('CHECK_FAILED'); + expect(seen[0].e.message).toBe('offline'); + expect(logger).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'errorChecking', + data: expect.objectContaining({ code: 'CHECK_FAILED' }), + }), + ); + }); + + test('onError unsubscribe stops delivery', async () => { + setupClientMocks(); + (globalThis as any).fetch = mock(async () => { + throw new Error('offline'); + }); + + const { Pushy } = await importFreshClient('pipeline-unsubscribe'); + const client = new Pushy({ appKey: 'demo-app' }); + const listener = mock(() => {}); + const unsubscribe = client.onError(listener); + unsubscribe(); + + await client.checkUpdate(); + expect(listener).not.toHaveBeenCalled(); + }); + + test('switchVersion failure is reported as errorSwitchVersion and rethrown (EH-3)', async () => { + const reloadUpdate = mock(() => Promise.reject(Error('bundle missing'))); + const logger = mock(() => {}); + setupClientMocks({ reloadUpdate }); + + const { Pushy, sharedState } = await importFreshClient( + 'pipeline-switch-version-failed', + ); + sharedState.downloadedHash = 'next-hash'; + sharedState.applyingUpdate = false; + const client = new Pushy({ appKey: 'demo-app', logger }); + const seen: any[] = []; + client.onError((e: any, eventType: string) => { + seen.push({ e, eventType }); + }); + + await expect(client.switchVersion('next-hash')).rejects.toThrow( + 'bundle missing', + ); + + expect(sharedState.applyingUpdate).toBe(false); + expect(seen).toHaveLength(1); + expect(seen[0].eventType).toBe('errorSwitchVersion'); + expect(seen[0].e.code).toBe('SWITCH_VERSION_FAILED'); + expect(logger).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'errorSwitchVersion', + data: expect.objectContaining({ + code: 'SWITCH_VERSION_FAILED', + newVersion: 'next-hash', + }), + }), + ); + }); + + test('a native-provided rejection code is preserved, not overwritten by the JS fallback (EH-5)', async () => { + // Native modules reject with stable codes from cpp/patch_core/error_codes.h + // (e.g. INVALID_OPTIONS); toUpdateError must keep them instead of stamping + // the JS-layer fallback code on top. + const nativeError: any = Error('empty hash'); + nativeError.code = 'INVALID_OPTIONS'; + const reloadUpdate = mock(() => Promise.reject(nativeError)); + setupClientMocks({ reloadUpdate }); + + const { Pushy, sharedState } = await importFreshClient( + 'pipeline-native-code-preserved', + ); + sharedState.downloadedHash = 'next-hash'; + sharedState.applyingUpdate = false; + const client = new Pushy({ appKey: 'demo-app' }); + const seen: any[] = []; + client.onError((e: any, eventType: string) => { + seen.push({ e, eventType }); + }); + + await expect(client.switchVersion('next-hash')).rejects.toThrow( + 'empty hash', + ); + + expect(seen).toHaveLength(1); + expect(seen[0].e.code).toBe('INVALID_OPTIONS'); + }); + + test('apk download failure keeps the native error and reports its message (EH-4)', async () => { + const downloadAndInstallApk = mock(() => + Promise.reject(Error('disk full')), + ); + const logger = mock(() => {}); + setupAndroidApkMocks(downloadAndInstallApk); + + const { Pushy, sharedState } = await importFreshClient( + 'pipeline-apk-cause', + ); + sharedState.apkStatus = null; + const client = new Pushy({ appKey: 'demo-app', logger }); + const seen: any[] = []; + client.onError((e: any, eventType: string) => { + seen.push({ e, eventType }); + }); + + await client.downloadAndInstallApk('https://example.com/app.apk'); + + expect(seen).toHaveLength(1); + expect(seen[0].eventType).toBe('errorDownloadAndInstallApk'); + expect(seen[0].e.code).toBe('APK_DOWNLOAD_FAILED'); + // The original native error must not be discarded anymore. + expect(seen[0].e.message).toBe('disk full'); + expect(logger).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'errorDownloadAndInstallApk', + data: expect.objectContaining({ + code: 'APK_DOWNLOAD_FAILED', + message: 'disk full', + }), + }), + ); + }); +}); + describe('downloadUpdate fallback chain', () => { const realSetTimeout = globalThis.setTimeout; @@ -680,6 +825,30 @@ describe('downloadUpdate fallback chain', () => { ); }); + test('all-strategies failure carries the DOWNLOAD_FAILED code (EH-1)', async () => { + setupDownloadMocks({ + downloadPatchFromPpk: mock(() => Promise.reject(Error('diff fail'))), + downloadPatchFromPackage: mock(() => Promise.reject(Error('pdiff fail'))), + downloadFullUpdate: mock(() => Promise.reject(Error('full fail'))), + }); + const { Pushy, sharedState } = await importFreshClient('dl-failed-code'); + sharedState.downloadedHash = undefined; + const client = new Pushy({ appKey: 'demo-app', maxRetries: 0 }); + const seen: any[] = []; + client.onError((e: any, eventType: string) => { + seen.push({ e, eventType }); + }); + + const err: any = await client + .downloadUpdate(updateInfo) + .catch((e: any) => e); + + expect(err.code).toBe('DOWNLOAD_FAILED'); + expect(seen).toHaveLength(1); + expect(seen[0].eventType).toBe('errorUpdate'); + expect(seen[0].e).toBe(err); + }); + test('deduplicates concurrent downloads of the same hash (JS-8)', async () => { // A slow native download keeps the first call in-flight while the second // starts, so the dedup must reuse the same promise instead of triggering a diff --git a/src/__tests__/provider.render.test.tsx b/src/__tests__/provider.render.test.tsx index fac9c14f..5c5afab4 100644 --- a/src/__tests__/provider.render.test.tsx +++ b/src/__tests__/provider.render.test.tsx @@ -24,6 +24,7 @@ const updateResult: CheckResult = { const createClient = (options: Record = {}) => { let progressCallback: ((data: ProgressData) => void) | undefined; + const errorListeners = new Set<(e: Error, eventType?: string) => void>(); const client = { options: { updateStrategy: 'alwaysAlert', @@ -32,7 +33,9 @@ const createClient = (options: Record = {}) => { ...options, }, assertDebug: () => true, - checkUpdate: mock(async () => ({ ...updateResult })), + checkUpdate: mock( + async (): Promise => ({ ...updateResult }), + ), notifyAfterCheckUpdate: mock(() => {}), markSuccess: mock(() => {}), switchVersion: mock(async () => {}), @@ -46,6 +49,17 @@ const createClient = (options: Record = {}) => { downloadAndInstallApk: mock(async () => {}), restartApp: mock(async () => {}), t: (key: string) => key, + onError: mock((listener: (e: Error, eventType?: string) => void) => { + errorListeners.add(listener); + return () => { + errorListeners.delete(listener); + }; + }), + // Simulates the real client contract: errors are emitted to onError + // listeners (report + lastError/Alert path) regardless of throwError. + emitError: (e: Error, eventType = 'errorChecking') => { + errorListeners.forEach(listener => listener(e, eventType)); + }, emitProgress: (data: ProgressData) => progressCallback?.(data), }; return client; @@ -127,8 +141,11 @@ describe('UpdateProvider rendering', () => { test('check failure sets lastError and alerts under alwaysAlert', async () => { const client = createClient({ updateStrategy: 'alwaysAlert' }); const checkError = new Error('offline'); + // Real client contract under the default throwError:false — the error is + // emitted to onError listeners and checkUpdate resolves undefined. client.checkUpdate.mockImplementation(async () => { - throw checkError; + client.emitError(checkError, 'errorChecking'); + return undefined; }); const captured: { current?: any } = {}; @@ -149,7 +166,8 @@ describe('UpdateProvider rendering', () => { const client = createClient({ updateStrategy: 'alertUpdateAndIgnoreError' }); const checkError = new Error('offline'); client.checkUpdate.mockImplementation(async () => { - throw checkError; + client.emitError(checkError, 'errorChecking'); + return undefined; }); const captured: { current?: any } = {}; @@ -169,7 +187,8 @@ describe('UpdateProvider rendering', () => { dismissErrorAfter: 20, }); client.checkUpdate.mockImplementation(async () => { - throw new Error('offline'); + client.emitError(new Error('offline'), 'errorChecking'); + return undefined; }); const captured: { current?: any } = {}; diff --git a/src/client.ts b/src/client.ts index 1f842db8..e8727f07 100644 --- a/src/client.ts +++ b/src/client.ts @@ -39,8 +39,19 @@ import { testUrls, } from './utils'; import i18n from './i18n'; +import { toUpdateError, UpdateError, UpdateErrorCode } from './error'; import { dedupeEndpoints, executeEndpointFallback } from './endpoint'; +/** + * Receives every error the client reports, alongside the report event type. + * The UpdateProvider subscribes to surface errors as lastError/Alert; user + * code can subscribe too. Listeners run before any throwError rethrow. + */ +export type UpdateErrorListener = ( + error: UpdateError, + eventType: EventType, +) => void; + const SERVER_PRESETS = { // cn Pushy: { @@ -142,7 +153,10 @@ export class Pushy { if (Platform.OS === 'ios' || Platform.OS === 'android') { if (!options.appKey) { - throw Error(i18n.t('error_appkey_required')); + throw new UpdateError( + i18n.t('error_appkey_required'), + 'APPKEY_REQUIRED', + ); } } @@ -184,13 +198,15 @@ export class Pushy { report = async ({ type, message = '', + code, data = {}, }: { type: EventType; message?: string; + code?: UpdateErrorCode; data?: Record; }) => { - log(`${type} ${message}`); + log(`${type} ${code ? `[${code}] ` : ''}${message}`); if (this.options.logger === noop) { // Wait briefly for a logger to arrive via setOptions (e.g. the rollback // report fires in the constructor before the user configures one), but @@ -214,6 +230,7 @@ export class Pushy { overridePackageVersion, buildTime, message, + code, ...currentVersionInfo, ...data, }, @@ -230,6 +247,39 @@ export class Pushy { throw e; } }; + private errorListeners = new Set(); + /** + * Subscribe to every error the client reports (regardless of throwError). + * Returns an unsubscribe function. + */ + onError = (listener: UpdateErrorListener) => { + this.errorListeners.add(listener); + return () => { + this.errorListeners.delete(listener); + }; + }; + /** + * Single exit point for errors: reports to the logger (with the stable + * code) and notifies onError listeners. Whether to also throw stays with + * the caller (throwIfEnabled or an unconditional rethrow). + */ + private emitError = ( + error: UpdateError, + type: EventType, + { + message = error.message, + data, + }: { message?: string; data?: Record } = {}, + ) => { + this.report({ type, message, code: error.code, data }); + for (const listener of this.errorListeners) { + try { + listener(error, type); + } catch (e: any) { + log('onError listener error:', e?.message || e); + } + } + }; notifyAfterCheckUpdate = (state: UpdateCheckState) => { const { afterCheckUpdate } = this.options; if (!afterCheckUpdate) { @@ -302,11 +352,13 @@ export class Pushy { if (!resp.ok) { const respText = await resp.text(); - throw Error( + throw new UpdateError( this.t('error_http_status', { status: resp.status, statusText: respText, }), + 'HTTP_STATUS', + { extra: { status: resp.status } }, ); } @@ -348,7 +400,13 @@ export class Pushy { if (sharedState.marked || __DEV__ || !isFirstTime) { return; } - await Promise.resolve(PushyModule.markSuccess()); + try { + await Promise.resolve(PushyModule.markSuccess()); + } catch (e) { + const err = toUpdateError(e, 'MARK_SUCCESS_FAILED'); + this.emitError(err, 'errorMarkSuccess'); + throw err; + } sharedState.marked = true; this.report({ type: 'markSuccess' }); }; @@ -366,7 +424,11 @@ export class Pushy { } } catch (e) { sharedState.applyingUpdate = false; - throw e; + const err = toUpdateError(e, 'SWITCH_VERSION_FAILED'); + this.emitError(err, 'errorSwitchVersion', { + data: { newVersion: hash }, + }); + throw err; } try { return await PushyModule.reloadUpdate({ hash }); @@ -374,7 +436,11 @@ export class Pushy { // reloadUpdate can reject (e.g. bundle missing); reset the flag so a // later retry is not permanently blocked by a stuck applyingUpdate. sharedState.applyingUpdate = false; - throw e; + const err = toUpdateError(e, 'SWITCH_VERSION_FAILED'); + this.emitError(err, 'errorSwitchVersion', { + data: { newVersion: hash }, + }); + throw err; } } }; @@ -385,7 +451,15 @@ export class Pushy { } if (assertHash(hash)) { log(`switchVersionLater: ${hash}`); - return PushyModule.setNeedUpdate({ hash }); + try { + return await PushyModule.setNeedUpdate({ hash }); + } catch (e) { + const err = toUpdateError(e, 'SWITCH_VERSION_FAILED'); + this.emitError(err, 'errorSwitchVersion', { + data: { newVersion: hash }, + }); + throw err; + } } }; checkUpdate = async (extra?: Record) => { @@ -456,14 +530,12 @@ export class Pushy { return result; } catch (e: any) { this.lastRespJson = previousRespJson; - const errorMessage = - e?.message || this.t('error_cannot_connect_server'); - this.report({ - type: 'errorChecking', - message: errorMessage, + const err = toUpdateError(e, 'CHECK_FAILED'); + this.emitError(err, 'errorChecking', { + message: err.message || this.t('error_cannot_connect_server'), }); - this.notifyAfterCheckUpdate({ status: 'error', error: e }); - this.throwIfEnabled(e); + this.notifyAfterCheckUpdate({ status: 'error', error: err }); + this.throwIfEnabled(err); // Fall back to the previous successful response if we have one; otherwise // return undefined so callers can distinguish "check failed" from a real // empty result and avoid overwriting the last good updateInfo. @@ -669,14 +741,22 @@ export class Pushy { delete sharedState.progressHandlers[hash]; } if (!succeeded) { + const message = errorMessages.join(';'); + if (lastError) { + const err = toUpdateError(lastError, 'DOWNLOAD_FAILED'); + this.emitError(err, 'errorUpdate', { + message, + data: { newVersion: hash }, + }); + throw err; + } + // No download URL was even attempted (e.g. dev without a full URL): + // report for diagnostics but there is no error object to surface. this.report({ type: 'errorUpdate', data: { newVersion: hash }, - message: errorMessages.join(';'), + message, }); - if (lastError) { - throw lastError; - } return; } else { const duration = Date.now() - patchStartTime; @@ -717,8 +797,12 @@ export class Pushy { return; } if (sharedState.apkStatus === 'downloaded') { - this.report({ type: 'errorInstallApk' }); - this.throwIfEnabled(Error('errorInstallApk')); + const err = new UpdateError( + this.t('error_apk_pending_install'), + 'APK_INSTALL_PENDING', + ); + this.emitError(err, 'errorInstallApk'); + this.throwIfEnabled(err); return; } if (Platform.Version <= 23) { @@ -727,13 +811,18 @@ export class Pushy { PermissionsAndroid.PERMISSIONS.WRITE_EXTERNAL_STORAGE, ); if (granted !== PermissionsAndroid.RESULTS.GRANTED) { - this.report({ type: 'rejectStoragePermission' }); - this.throwIfEnabled(Error('rejectStoragePermission')); + const err = new UpdateError( + this.t('error_storage_permission_rejected'), + 'STORAGE_PERMISSION_REJECTED', + ); + this.emitError(err, 'rejectStoragePermission'); + this.throwIfEnabled(err); return; } } catch (e: any) { - this.report({ type: 'errorStoragePermission' }); - this.throwIfEnabled(e); + const err = toUpdateError(e, 'STORAGE_PERMISSION_ERROR'); + this.emitError(err, 'errorStoragePermission'); + this.throwIfEnabled(err); return; } } @@ -761,10 +850,14 @@ export class Pushy { hash: progressKey, }); sharedState.apkStatus = 'downloaded'; - } catch { + } catch (e) { sharedState.apkStatus = null; - this.report({ type: 'errorDownloadAndInstallApk' }); - this.throwIfEnabled(Error('errorDownloadAndInstallApk')); + // Keep the native error (message/stack) instead of discarding it. + const err = toUpdateError(e, 'APK_DOWNLOAD_FAILED'); + this.emitError(err, 'errorDownloadAndInstallApk', { + message: err.message || this.t('error_apk_download_failed'), + }); + this.throwIfEnabled(err); } finally { if (sharedState.progressHandlers[progressKey]) { sharedState.progressHandlers[progressKey].remove(); diff --git a/src/core.ts b/src/core.ts index 61c41f43..1f4cc5bb 100644 --- a/src/core.ts +++ b/src/core.ts @@ -1,5 +1,6 @@ import { NativeEventEmitter, NativeModules, Platform } from 'react-native'; import i18n from './i18n'; +import { UpdateError } from './error'; import { emptyModule, error, log } from './utils'; /* eslint-disable @react-native/no-deep-imports */ @@ -21,8 +22,9 @@ export const PushyModule = export const UpdateModule = PushyModule; if (!PushyModule) { - throw Error( + throw new UpdateError( 'Failed to load react-native-update native module, please try to recompile', + 'MODULE_NOT_LOADED', ); } diff --git a/src/endpoint.ts b/src/endpoint.ts index 0c4fb86f..0a017cda 100644 --- a/src/endpoint.ts +++ b/src/endpoint.ts @@ -1,3 +1,5 @@ +import { UpdateError } from './error'; + export interface EndpointAttemptSuccess { endpoint: string; value: T; @@ -47,7 +49,7 @@ export const pickRandomEndpoint = ( random: () => number = Math.random, ) => { if (!endpoints.length) { - throw new Error('No endpoints configured'); + throw new UpdateError('No endpoints configured', 'NO_ENDPOINTS'); } return endpoints[Math.floor(random() * endpoints.length)]; }; @@ -120,7 +122,7 @@ export async function executeEndpointFallback({ let candidates = dedupeEndpoints(configuredEndpoints); if (!candidates.length) { - throw new Error('No endpoints configured'); + throw new UpdateError('No endpoints configured', 'NO_ENDPOINTS'); } const firstEndpoint = pickRandomEndpoint(candidates, random); diff --git a/src/error.ts b/src/error.ts new file mode 100644 index 00000000..eb25dc28 --- /dev/null +++ b/src/error.ts @@ -0,0 +1,68 @@ +/** + * Stable, machine-readable error codes. Unlike messages (which are localized + * and vary across platforms), codes are safe to aggregate on in a logger. + * + * The native-originated codes (second group) are defined in + * cpp/patch_core/error_codes.h — the single source of truth shared by the + * Android/iOS/Harmony modules — and flow through promise rejections onto the + * `code` property, which toUpdateError() preserves. + */ +export type UpdateErrorCode = + // JS-layer codes + | 'MODULE_NOT_LOADED' + | 'APPKEY_REQUIRED' + | 'NO_ENDPOINTS' + | 'HTTP_STATUS' + | 'CHECK_FAILED' + | 'DOWNLOAD_FAILED' + | 'SWITCH_VERSION_FAILED' + | 'MARK_SUCCESS_FAILED' + | 'APK_INSTALL_PENDING' + | 'STORAGE_PERMISSION_REJECTED' + | 'STORAGE_PERMISSION_ERROR' + | 'APK_DOWNLOAD_FAILED' + // Native codes (see cpp/patch_core/error_codes.h) + | 'INVALID_OPTIONS' + | 'PATCH_FAILED' + | 'FILE_OPERATION_FAILED' + | 'RESTART_FAILED' + | 'INVALID_HASH_INFO' + | 'UNSUPPORTED_PLATFORM'; + +export class UpdateError extends Error { + code: UpdateErrorCode; + cause?: unknown; + extra?: Record; + + constructor( + message: string, + code: UpdateErrorCode, + options?: { cause?: unknown; extra?: Record }, + ) { + super(message); + this.name = 'UpdateError'; + this.code = code; + this.cause = options?.cause; + this.extra = options?.extra; + } +} + +/** + * Attach a code to an unknown thrown value. An existing Error keeps its + * identity (message, stack, and any code already assigned upstream) so callers + * comparing the caught error to the original still match; non-Error values are + * wrapped. + */ +export const toUpdateError = ( + e: unknown, + code: UpdateErrorCode, +): UpdateError => { + if (e instanceof Error) { + const err = e as UpdateError; + if (!err.code) { + err.code = code; + } + return err; + } + return new UpdateError(String(e ?? code), code); +}; diff --git a/src/index.ts b/src/index.ts index 7e657f95..8a456a50 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,4 +1,7 @@ export { Pushy, Cresc } from './client'; +export type { UpdateErrorListener } from './client'; +export { UpdateError } from './error'; +export type { UpdateErrorCode } from './error'; export { ProgressContext, UpdateContext, diff --git a/src/locales/en.ts b/src/locales/en.ts index c80078be..a3a9d878 100644 --- a/src/locales/en.ts +++ b/src/locales/en.ts @@ -19,7 +19,6 @@ export default { time_remaining: 'Time remaining: {{time}}', // Error messages - error_code: 'Error code: {{code}}', error_message: 'Error message: {{message}}', retry_count: 'Retry attempt: {{count}}/{{max}}', @@ -57,6 +56,11 @@ export default { error_ping_failed: 'Ping failed', error_ping_timeout: 'Ping timeout', error_http_status: '{{status}} {{statusText}}', + error_apk_pending_install: + 'The APK has been downloaded, please complete the installation in the system installer', + error_storage_permission_rejected: + 'Storage permission denied, unable to download the APK', + error_apk_download_failed: 'Failed to download or install the APK', // Development messages dev_debug_disabled: diff --git a/src/locales/zh.ts b/src/locales/zh.ts index 44291aa5..c69aeb50 100644 --- a/src/locales/zh.ts +++ b/src/locales/zh.ts @@ -19,7 +19,6 @@ export default { time_remaining: '剩余时间: {{time}}', // Error messages - error_code: '错误代码: {{code}}', error_message: '错误信息: {{message}}', retry_count: '重试次数: {{count}}/{{max}}', @@ -54,6 +53,9 @@ export default { error_ping_failed: 'Ping 失败', error_ping_timeout: 'Ping 超时', error_http_status: '{{status}} {{statusText}}', + error_apk_pending_install: '安装包已下载完成，请在系统安装界面完成安装', + error_storage_permission_rejected: '存储权限被拒绝，无法下载安装包', + error_apk_download_failed: '安装包下载或安装失败', // Development messages dev_debug_disabled: diff --git a/src/provider.tsx b/src/provider.tsx index 677c4a30..24c6aa86 100644 --- a/src/provider.tsx +++ b/src/provider.tsx @@ -46,15 +46,6 @@ export const UpdateProvider = ({ const [progress, setProgress] = useState(); const [lastError, setLastError] = useState(); - const throwErrorIfEnabled = useCallback( - (e: Error) => { - if (options.throwError) { - throw e; - } - }, - [options.throwError], - ); - const dismissError = useCallback(() => { setLastError(undefined); }, []); @@ -80,6 +71,25 @@ export const UpdateProvider = ({ [options.updateStrategy], ); + // All client errors flow through this single subscription (regardless of + // throwError), so the catches below only handle flow control and never + // duplicate the lastError/Alert surfacing. + useEffect( + () => + client.onError((e, eventType) => { + setLastError(e); + alertError( + client.t( + eventType === 'errorChecking' + ? 'error_update_check_failed' + : 'update_failed', + ), + e.message, + ); + }), + [client, alertError], + ); + const switchVersion = useCallback( async (info: CheckResult | undefined = updateInfoRef.current) => { if (info && info.hash) { @@ -117,10 +127,11 @@ export const UpdateProvider = ({ return false; } if (options.updateStrategy === 'silentAndNow') { - client.switchVersion(hash); + // Failures are surfaced via the onError subscription above. + client.switchVersion(hash).catch(noop); return true; } else if (options.updateStrategy === 'silentAndLater') { - client.switchVersionLater(hash); + client.switchVersionLater(hash).catch(noop); return true; } alertUpdate(client.t('alert_title'), client.t('alert_update_ready'), [ @@ -128,26 +139,33 @@ export const UpdateProvider = ({ text: client.t('alert_next_time'), style: 'cancel', onPress: () => { - client.switchVersionLater(hash); + client.switchVersionLater(hash).catch(noop); }, }, { text: client.t('alert_update_now'), style: 'default', onPress: () => { - client.switchVersion(hash); + client.switchVersion(hash).catch(noop); }, }, ]); return true; } catch (e: any) { - setLastError(e); - alertError(client.t('update_failed'), e.message); - throwErrorIfEnabled(e); + // Client pipeline errors carry a code and were already surfaced via + // the onError subscription; errors thrown by user hooks + // (afterDownloadUpdate) bypass the pipeline and are surfaced here. + if (!e?.code) { + setLastError(e); + alertError(client.t('update_failed'), e.message); + } + if (options.throwError) { + throw e; + } return false; } }, - [client, options, alertUpdate, alertError, throwErrorIfEnabled], + [client, options, alertUpdate, alertError], ); const downloadAndInstallApk = useCallback( @@ -168,9 +186,16 @@ export const UpdateProvider = ({ try { rootInfo = await client.checkUpdate(extra); } catch (e: any) { - setLastError(e); - alertError(client.t('error_update_check_failed'), e.message); - throwErrorIfEnabled(e); + // Client pipeline errors carry a code and were already surfaced via + // the onError subscription; errors thrown by user hooks + // (beforeCheckUpdate) bypass the pipeline and are surfaced here. + if (!e?.code) { + setLastError(e); + alertError(client.t('error_update_check_failed'), e.message); + } + if (options.throwError) { + throw e; + } return; } if (!rootInfo) { @@ -252,10 +277,9 @@ export const UpdateProvider = ({ }, [ client, - alertError, - throwErrorIfEnabled, options, alertUpdate, + alertError, downloadAndInstallApk, downloadUpdate, ], @@ -274,7 +298,8 @@ export const UpdateProvider = ({ let markSuccessTimer: ReturnType | undefined; if (autoMarkSuccess) { markSuccessTimer = setTimeout(() => { - markSuccess(); + // Failures are reported and surfaced via the onError subscription. + Promise.resolve(markSuccess()).catch(noop); }, 1000); } if (checkStrategy === 'both' || checkStrategy === 'onAppResume') { @@ -351,7 +376,8 @@ export const UpdateProvider = ({ try { const payload = typeof code === 'string' ? JSON.parse(code) : code; return parseTestPayload(payload); - } catch { + } catch (e: any) { + log('parseTestQrCode: invalid payload', e?.message || e); return false; } }, diff --git a/src/type.ts b/src/type.ts index 224b34fb..14bc08e1 100644 --- a/src/type.ts +++ b/src/type.ts @@ -52,6 +52,8 @@ export type EventType = | 'downloadSuccess' | 'errorUpdate' | 'markSuccess' + | 'errorMarkSuccess' + | 'errorSwitchVersion' | 'downloadingApk' | 'rejectStoragePermission' | 'errorStoragePermission' @@ -59,6 +61,8 @@ export type EventType = | 'errorInstallApk'; export interface EventData { + /** Stable machine-readable error code (see UpdateErrorCode); present on error events */ + code?: string; currentVersion: string; cInfo: { rnu: string; From 414f02032ab5cce30e8e90182ed1473c0dfbfd0e Mon Sep 17 00:00:00 2001 From: sunnylqm Date: Tue, 7 Jul 2026 19:42:40 +0800 Subject: [PATCH 2/3] fix(native): error visibility, rollback guards, and cross-platform error codes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../modules/update/ErrorCodes.java | 24 +++++ .../modules/update/StateSerialRunner.java | 3 +- .../modules/update/UiThreadRunner.java | 3 +- .../modules/update/UpdateContext.java | 15 ++- .../modules/update/UpdateModuleImpl.java | 30 +++--- cpp/patch_core/error_codes.h | 43 ++++++++ harmony/pushy/src/main/ets/UpdateContext.ts | 22 +++- ios/RCTPushy/RCTPushy.mm | 101 ++++++++++++++---- ios/RCTPushy/RCTPushyDownloader.mm | 23 +++- 9 files changed, 220 insertions(+), 44 deletions(-) create mode 100644 android/src/main/java/cn/reactnative/modules/update/ErrorCodes.java create mode 100644 cpp/patch_core/error_codes.h diff --git a/android/src/main/java/cn/reactnative/modules/update/ErrorCodes.java b/android/src/main/java/cn/reactnative/modules/update/ErrorCodes.java new file mode 100644 index 00000000..a6d3bd85 --- /dev/null +++ b/android/src/main/java/cn/reactnative/modules/update/ErrorCodes.java @@ -0,0 +1,24 @@ +package cn.reactnative.modules.update; + +/** + * Stable, machine-readable error codes used as the promise rejection code so + * the JS layer and user loggers can aggregate errors across platforms. + * + * MUST stay in sync with cpp/patch_core/error_codes.h (the single source of + * truth) and src/error.ts (UpdateErrorCode). Messages are free-form; only the + * codes are part of the contract. + */ +final class ErrorCodes { + static final String INVALID_OPTIONS = "INVALID_OPTIONS"; + static final String DOWNLOAD_FAILED = "DOWNLOAD_FAILED"; + static final String PATCH_FAILED = "PATCH_FAILED"; + static final String FILE_OPERATION_FAILED = "FILE_OPERATION_FAILED"; + static final String SWITCH_VERSION_FAILED = "SWITCH_VERSION_FAILED"; + static final String MARK_SUCCESS_FAILED = "MARK_SUCCESS_FAILED"; + static final String RESTART_FAILED = "RESTART_FAILED"; + static final String INVALID_HASH_INFO = "INVALID_HASH_INFO"; + static final String UNSUPPORTED_PLATFORM = "UNSUPPORTED_PLATFORM"; + + private ErrorCodes() { + } +} diff --git a/android/src/main/java/cn/reactnative/modules/update/StateSerialRunner.java b/android/src/main/java/cn/reactnative/modules/update/StateSerialRunner.java index f0e12c19..b316cf8a 100644 --- a/android/src/main/java/cn/reactnative/modules/update/StateSerialRunner.java +++ b/android/src/main/java/cn/reactnative/modules/update/StateSerialRunner.java @@ -43,6 +43,7 @@ private StateSerialRunner() { static void run( @Nullable final Promise promise, + final String errorCode, final String operationName, final Operation operation ) { @@ -53,7 +54,7 @@ public void run() { operation.run(); } catch (Throwable error) { if (promise != null) { - promise.reject(operationName + " failed", error); + promise.reject(errorCode, operationName + " failed", error); } else { Log.e(UpdateContext.TAG, operationName + " failed", error); } diff --git a/android/src/main/java/cn/reactnative/modules/update/UiThreadRunner.java b/android/src/main/java/cn/reactnative/modules/update/UiThreadRunner.java index 33d494e2..1bafe5b4 100644 --- a/android/src/main/java/cn/reactnative/modules/update/UiThreadRunner.java +++ b/android/src/main/java/cn/reactnative/modules/update/UiThreadRunner.java @@ -15,6 +15,7 @@ private UiThreadRunner() { static void run( @Nullable final Promise promise, + final String errorCode, final String operationName, final Operation operation ) { @@ -25,7 +26,7 @@ public void run() { operation.run(); } catch (Throwable error) { if (promise != null) { - promise.reject(operationName + " failed", error); + promise.reject(errorCode, operationName + " failed", error); } else { Log.e(UpdateContext.TAG, operationName + " failed", error); } diff --git a/android/src/main/java/cn/reactnative/modules/update/UpdateContext.java b/android/src/main/java/cn/reactnative/modules/update/UpdateContext.java index df69fb65..fd9389eb 100644 --- a/android/src/main/java/cn/reactnative/modules/update/UpdateContext.java +++ b/android/src/main/java/cn/reactnative/modules/update/UpdateContext.java @@ -207,8 +207,10 @@ private void applyState(SharedPreferences.Editor editor, StateCoreResult state) } private void persistEditor(SharedPreferences.Editor editor, String reason) { - if (!editor.commit() && DEBUG) { - Log.w(TAG, "Failed to persist update state for " + reason); + // A lost state write can mean a missed rollback or a version switch + // that silently never happens, so this must be visible in release too. + if (!editor.commit()) { + Log.e(TAG, "Failed to persist update state for " + reason); } } @@ -367,6 +369,13 @@ public String getBundleUrl(String defaultAssetsUrl) { ignoreRollback, true ); + if (launchState.didRollback) { + // The crash-protection rollback: the new version never called + // markSuccess. Keep this visible in release logs. + Log.e(TAG, "Version " + currentState.currentVersion + + " was not marked as successful, rolling back to " + + launchState.currentVersion); + } if (launchState.didRollback || launchState.consumedFirstTime) { SharedPreferences.Editor editor = sp.edit(); applyState(editor, launchState); @@ -411,6 +420,8 @@ private String rollBack() { false, false ); + Log.e(TAG, "Rolling back version " + currentState.currentVersion + + " to " + nextState.currentVersion); SharedPreferences.Editor editor = sp.edit(); applyState(editor, nextState); persistEditor(editor, "rollback"); diff --git a/android/src/main/java/cn/reactnative/modules/update/UpdateModuleImpl.java b/android/src/main/java/cn/reactnative/modules/update/UpdateModuleImpl.java index 5a3a9b8a..e21f2e26 100644 --- a/android/src/main/java/cn/reactnative/modules/update/UpdateModuleImpl.java +++ b/android/src/main/java/cn/reactnative/modules/update/UpdateModuleImpl.java @@ -41,7 +41,7 @@ public void onDownloadCompleted(DownloadTaskParams params) { @Override public void onDownloadFailed(Throwable error) { - promise.reject(error); + promise.reject(ErrorCodes.DOWNLOAD_FAILED, error); } }); } @@ -64,7 +64,7 @@ public void onDownloadCompleted(DownloadTaskParams params) { @Override public void onDownloadFailed(Throwable error) { - promise.reject(error); + promise.reject(ErrorCodes.DOWNLOAD_FAILED, error); } }); } @@ -84,7 +84,7 @@ public void onDownloadCompleted(DownloadTaskParams params) { @Override public void onDownloadFailed(Throwable error) { - promise.reject(error); + promise.reject(ErrorCodes.DOWNLOAD_FAILED, error); } }); } @@ -107,11 +107,11 @@ public void onDownloadCompleted(DownloadTaskParams params) { @Override public void onDownloadFailed(Throwable error) { - promise.reject(error); + promise.reject(ErrorCodes.DOWNLOAD_FAILED, error); } }); } catch (Exception e) { - promise.reject("downloadPatchFromPpk failed: " + e.getMessage()); + promise.reject(ErrorCodes.INVALID_OPTIONS, "downloadPatchFromPpk failed: " + e.getMessage(), e); } } @@ -130,7 +130,7 @@ public static void restartApp( @Nullable final String hash, final Promise promise ) { - UiThreadRunner.run(promise, "restartApp", new UiThreadRunner.Operation() { + UiThreadRunner.run(promise, ErrorCodes.RESTART_FAILED, "restartApp", new UiThreadRunner.Operation() { @Override public void run() throws Throwable { ReactReloadManager.restartApp(updateContext, reactContext, hash); @@ -149,7 +149,7 @@ public static void setNeedUpdate( final Promise promise ) { final String hash = options.getString("hash"); - StateSerialRunner.run(promise, "switchVersionLater", new StateSerialRunner.Operation() { + StateSerialRunner.run(promise, ErrorCodes.SWITCH_VERSION_FAILED, "switchVersionLater", new StateSerialRunner.Operation() { @Override public void run() { setNeedUpdateInternal(updateContext, hash); @@ -160,7 +160,7 @@ public void run() { public static void setNeedUpdate(final UpdateContext updateContext, final ReadableMap options) { final String hash = options.getString("hash"); - StateSerialRunner.run(null, "switchVersionLater", new StateSerialRunner.Operation() { + StateSerialRunner.run(null, ErrorCodes.SWITCH_VERSION_FAILED, "switchVersionLater", new StateSerialRunner.Operation() { @Override public void run() { setNeedUpdateInternal(updateContext, hash); @@ -173,7 +173,7 @@ private static void markSuccessInternal(UpdateContext updateContext) { } public static void markSuccess(final UpdateContext updateContext, final Promise promise) { - StateSerialRunner.run(promise, "markSuccess", new StateSerialRunner.Operation() { + StateSerialRunner.run(promise, ErrorCodes.MARK_SUCCESS_FAILED, "markSuccess", new StateSerialRunner.Operation() { @Override public void run() { markSuccessInternal(updateContext); @@ -183,7 +183,7 @@ public void run() { } public static void markSuccess(final UpdateContext updateContext) { - StateSerialRunner.run(null, "markSuccess", new StateSerialRunner.Operation() { + StateSerialRunner.run(null, ErrorCodes.MARK_SUCCESS_FAILED, "markSuccess", new StateSerialRunner.Operation() { @Override public void run() { markSuccessInternal(updateContext); @@ -200,7 +200,7 @@ public static void setUuid( final String uuid, final Promise promise ) { - StateSerialRunner.run(promise, "setUuid", new StateSerialRunner.Operation() { + StateSerialRunner.run(promise, ErrorCodes.FILE_OPERATION_FAILED, "setUuid", new StateSerialRunner.Operation() { @Override public void run() { setUuidInternal(updateContext, uuid); @@ -210,7 +210,7 @@ public void run() { } public static void setUuid(final UpdateContext updateContext, final String uuid) { - StateSerialRunner.run(null, "setUuid", new StateSerialRunner.Operation() { + StateSerialRunner.run(null, ErrorCodes.FILE_OPERATION_FAILED, "setUuid", new StateSerialRunner.Operation() { @Override public void run() { setUuidInternal(updateContext, uuid); @@ -235,7 +235,7 @@ public static void setLocalHashInfo( final String info, final Promise promise ) { - StateSerialRunner.run(promise, "setLocalHashInfo", new StateSerialRunner.Operation() { + StateSerialRunner.run(promise, ErrorCodes.INVALID_HASH_INFO, "setLocalHashInfo", new StateSerialRunner.Operation() { @Override public void run() { setLocalHashInfoInternal(updateContext, hash, info); @@ -249,7 +249,7 @@ public static void setLocalHashInfo( final String hash, final String info ) { - StateSerialRunner.run(null, "setLocalHashInfo", new StateSerialRunner.Operation() { + StateSerialRunner.run(null, ErrorCodes.INVALID_HASH_INFO, "setLocalHashInfo", new StateSerialRunner.Operation() { @Override public void run() { setLocalHashInfoInternal(updateContext, hash, info); @@ -264,7 +264,7 @@ public static void getLocalHashInfo( ) { String value = updateContext.getKv("hash_" + hash); if (!isValidHashInfo(value)) { - promise.reject("getLocalHashInfo failed: invalid json string"); + promise.reject(ErrorCodes.INVALID_HASH_INFO, "getLocalHashInfo failed: invalid json string"); return; } diff --git a/cpp/patch_core/error_codes.h b/cpp/patch_core/error_codes.h new file mode 100644 index 00000000..8e2ac36d --- /dev/null +++ b/cpp/patch_core/error_codes.h @@ -0,0 +1,43 @@ +#ifndef PUSHY_PATCH_CORE_ERROR_CODES_H_ +#define PUSHY_PATCH_CORE_ERROR_CODES_H_ + +// Single source of truth for the stable, machine-readable error codes shared +// by every platform. Native modules reject promises with one of these codes +// so the JS layer (src/error.ts UpdateErrorCode) and user loggers can +// aggregate errors across platforms and locales. +// +// Mirrors that cannot include this header MUST stay in sync by hand: +// - src/error.ts (UpdateErrorCode union, JS layer) +// - android/.../ErrorCodes.java (Java constants) +// iOS (RCTPushy.mm) includes this header directly. +// +// Human-readable messages are NOT part of this contract: they may differ per +// platform and locale. Only the codes are stable. + +namespace pushy { +namespace error_codes { + +// Method options missing or malformed (blank hash/url, wrong types). +constexpr const char* kInvalidOptions = "INVALID_OPTIONS"; +// Native download failed (network error, bad HTTP status, truncated body). +constexpr const char* kDownloadFailed = "DOWNLOAD_FAILED"; +// Unzip or hdiff patch application failed. +constexpr const char* kPatchFailed = "PATCH_FAILED"; +// Local file or state persistence operation failed. +constexpr const char* kFileOperationFailed = "FILE_OPERATION_FAILED"; +// switchVersion / setNeedUpdate state transition failed. +constexpr const char* kSwitchVersionFailed = "SWITCH_VERSION_FAILED"; +// markSuccess state transition failed. +constexpr const char* kMarkSuccessFailed = "MARK_SUCCESS_FAILED"; +// reloadUpdate / restartApp failed. +constexpr const char* kRestartFailed = "RESTART_FAILED"; +// Stored or provided hash info is not a valid JSON object. +constexpr const char* kInvalidHashInfo = "INVALID_HASH_INFO"; +// The method is not supported on this platform (e.g. downloadAndInstallApk +// outside Android). +constexpr const char* kUnsupportedPlatform = "UNSUPPORTED_PLATFORM"; + +} // namespace error_codes +} // namespace pushy + +#endif // PUSHY_PATCH_CORE_ERROR_CODES_H_ diff --git a/harmony/pushy/src/main/ets/UpdateContext.ts b/harmony/pushy/src/main/ets/UpdateContext.ts index f026a4bf..01b0d8cb 100644 --- a/harmony/pushy/src/main/ets/UpdateContext.ts +++ b/harmony/pushy/src/main/ets/UpdateContext.ts @@ -468,13 +468,22 @@ export class UpdateContext { public getBundleUrl() { UpdateContext.isUsingBundleUrl = true; this.trace('getBundleUrl:enter'); + const stateBeforeLaunch = this.getStateSnapshot(); const launchState = NativePatchCore.runStateCore( STATE_OP_RESOLVE_LAUNCH, - this.getStateSnapshot(), + stateBeforeLaunch, '', UpdateContext.ignoreRollback, true, ); + if (launchState.didRollback) { + // The crash-protection rollback: the new version never called + // markSuccess. Keep this visible in release logs. + console.error( + `Version ${stateBeforeLaunch.currentVersion} was not marked as successful,` + + ` rolled back to ${launchState.currentVersion}`, + ); + } if (launchState.didRollback || launchState.consumedFirstTime) { this.persistState(launchState, { markFirstLoadMarker: launchState.consumedFirstTime, @@ -490,7 +499,12 @@ export class UpdateContext { ); let version = launchState.loadVersion || ''; - while (version) { + // Guard the rollback chain against cycles: a corrupted state returning an + // already-visited version would otherwise spin this loop forever during + // startup (Android has the same guard). + const visitedVersions = new Set(); + while (version && !visitedVersions.has(version)) { + visitedVersions.add(version); const bundleFile = this.getBundlePath(version); try { if (!fileIo.accessSync(bundleFile)) { @@ -514,7 +528,11 @@ export class UpdateContext { } private rollBack(): string { + const stateBefore = this.getStateSnapshot(); const nextState = this.runStateOperation(STATE_OP_ROLLBACK); + console.error( + `Rolling back version ${stateBefore.currentVersion} to ${nextState.currentVersion}`, + ); return nextState.currentVersion || ''; } diff --git a/ios/RCTPushy/RCTPushy.mm b/ios/RCTPushy/RCTPushy.mm index c89732ed..de8cef5e 100644 --- a/ios/RCTPushy/RCTPushy.mm +++ b/ios/RCTPushy/RCTPushy.mm @@ -2,6 +2,7 @@ #import "RCTPushyDownloader.h" #import "ZipArchive.h" #include "../../cpp/patch_core/archive_patch_core.h" +#include "../../cpp/patch_core/error_codes.h" #include "../../cpp/patch_core/patch_core.h" #include "../../cpp/patch_core/state_core.h" @@ -37,9 +38,15 @@ static NSString * const SOURCE_PATCH_NAME = @"__diff.json"; static NSString * const BUNDLE_PATCH_NAME = @"index.bundlejs.patch"; -// error def +// error def — messages are human-readable; the stable cross-platform codes +// live in cpp/patch_core/error_codes.h and travel in PushyErrorCodeKey. static NSString * const ERROR_OPTIONS = @"options error"; static NSString * const ERROR_FILE_OPERATION = @"file operation error"; +static NSString * const PushyErrorCodeKey = @"PushyErrorCode"; + +static NSString *PushyCode(const char *code) { + return [NSString stringWithUTF8String:code]; +} // event def static NSString * const EVENT_PROGRESS_DOWNLOAD = @"RCTPushyDownloadProgress"; @@ -80,7 +87,10 @@ static void PushyWithStateLock(void (NS_NOESCAPE ^block)(void)) { static NSError *PushyNSErrorFromStatus(const pushy::patch::Status &status) { return [NSError errorWithDomain:PushyErrorDomain code:-1 - userInfo:@{ NSLocalizedDescriptionKey: [NSString stringWithUTF8String:status.message.c_str()] }]; + userInfo:@{ + NSLocalizedDescriptionKey: [NSString stringWithUTF8String:status.message.c_str()], + PushyErrorCodeKey: PushyCode(pushy::error_codes::kPatchFailed), + }]; } static NSUserDefaults *PushyDefaults(void) { @@ -118,21 +128,24 @@ static BOOL PushyStringIsBlank(NSString *value) { } static void PushyRejectError(RCTPromiseRejectBlock reject, NSError *error) { - reject([NSString stringWithFormat:@"%ld", (long)error.code], error.localizedDescription, error); + // Prefer the stable cross-platform code (error_codes.h); fall back to the + // numeric NSError code for system errors that were not classified. + NSString *code = error.userInfo[PushyErrorCodeKey]; + if (code == nil) { + code = [NSString stringWithFormat:@"%ld", (long)error.code]; + } + reject(code, error.localizedDescription, error); } -static NSError *PushyErrorWithMessage(NSString *message) { +static NSError *PushyErrorWithCode(const char *code, NSString *message) { return [NSError errorWithDomain:PushyErrorDomain code:-1 userInfo:@{ NSLocalizedDescriptionKey: message ?: @"unknown error", + PushyErrorCodeKey: PushyCode(code), }]; } -static void PushyRejectMessage(RCTPromiseRejectBlock reject, NSString *message) { - PushyRejectError(reject, PushyErrorWithMessage(message)); -} - static pushy::patch::PatchManifest PushyPatchManifestFromJson(NSDictionary *json) { pushy::patch::PatchManifest manifest; @@ -257,6 +270,7 @@ + (NSURL *)bundleURL } if (!state.current_version.empty()) { + std::string const versionBeforeLaunch = state.current_version; pushy::state::LaunchDecision decision = pushy::state::ResolveLaunchState( state, ignoreRollback.load(), @@ -264,6 +278,13 @@ + (NSURL *)bundleURL ); state = decision.state; + if (decision.did_rollback) { + // The crash-protection rollback: the new version never called + // markSuccess. Keep this visible in release logs. + RCTLogWarn(@"RCTPushy -- version %@ was not marked as successful, rolled back to %@", + PushyFromStdString(versionBeforeLaunch), + PushyFromStdString(state.current_version)); + } if (decision.did_rollback || decision.consumed_first_time) { PushyApplyStateToDefaults(defaults, state); } @@ -275,13 +296,18 @@ + (NSURL *)bundleURL NSString *loadVersion = PushyFromStdString(decision.load_version); NSString *downloadDir = [RCTPushy downloadDir]; - while (loadVersion.length) { + // Guard the rollback chain against cycles: a corrupted state + // returning an already-visited version would otherwise spin this + // loop forever during startup (Android has the same guard). + NSMutableSet *visitedVersions = [NSMutableSet set]; + while (loadVersion.length && ![visitedVersions containsObject:loadVersion]) { + [visitedVersions addObject:loadVersion]; NSString *bundlePath = [[downloadDir stringByAppendingPathComponent:loadVersion] stringByAppendingPathComponent:BUNDLE_FILE_NAME]; if ([[NSFileManager defaultManager] fileExistsAtPath:bundlePath isDirectory:NULL]) { resolvedURL = [NSURL fileURLWithPath:bundlePath]; return; } else { - RCTLogError(@"RCTPushy -- bundle version %@ not found", loadVersion); + RCTLogError(@"RCTPushy -- bundle version %@ not found, rolling back", loadVersion); state = pushy::state::Rollback(state); PushyApplyStateToDefaults(defaults, state); loadVersion = PushyFromStdString(state.current_version); @@ -359,7 +385,7 @@ - (instancetype)init rejecter:(RCTPromiseRejectBlock)reject) { if (PushyStringIsBlank(uuid)) { - PushyRejectError(reject, PushyErrorWithMessage(ERROR_OPTIONS)); + PushyRejectError(reject, PushyErrorWithCode(pushy::error_codes::kInvalidOptions, ERROR_OPTIONS)); return; } @@ -373,7 +399,7 @@ - (instancetype)init rejecter:(RCTPromiseRejectBlock)reject) { if (PushyStringIsBlank(hash) || PushyStringIsBlank(value)) { - PushyRejectMessage(reject, ERROR_OPTIONS); + PushyRejectError(reject, PushyErrorWithCode(pushy::error_codes::kInvalidOptions, ERROR_OPTIONS)); return; } @@ -386,7 +412,9 @@ - (instancetype)init resolve(@true); } else { - PushyRejectError(reject, error ?: PushyErrorWithMessage(@"json格式校验报错")); + PushyRejectError(reject, PushyErrorWithCode( + pushy::error_codes::kInvalidHashInfo, + error != nil ? error.localizedDescription : @"invalid json string")); } } @@ -421,6 +449,15 @@ - (instancetype)init [self downloadUpdate:PushyTypePatchFromPpk options:options resolver:resolve rejecter:reject]; } +RCT_EXPORT_METHOD(downloadAndInstallApk:(NSDictionary *)options + resolver:(RCTPromiseResolveBlock)resolve + rejecter:(RCTPromiseRejectBlock)reject) +{ + PushyRejectError(reject, PushyErrorWithCode( + pushy::error_codes::kUnsupportedPlatform, + @"downloadAndInstallApk is only supported on Android")); +} + RCT_EXPORT_METHOD(setNeedUpdate:(NSDictionary *)options resolver:(RCTPromiseResolveBlock)resolve rejecter:(RCTPromiseRejectBlock)reject) @@ -502,6 +539,12 @@ - (void)downloadUpdate:(PushyType)type { [self performUpdate:type options:options callback:^(NSError *error) { if (error != nil) { + if (error.userInfo[PushyErrorCodeKey] == nil) { + // Unclassified (system/network) errors from the download + // pipeline; keep the original message. + error = PushyErrorWithCode(pushy::error_codes::kDownloadFailed, + error.localizedDescription); + } PushyRejectError(reject, error); return; } @@ -527,24 +570,38 @@ - (void)performUpdate:(PushyType)type options:(NSDictionary *)options callback:( NSString *hash = PushyOptionString(options, @"hash"); if (PushyStringIsBlank(updateUrl) || PushyStringIsBlank(hash)) { - callback(PushyErrorWithMessage(ERROR_OPTIONS)); + callback(PushyErrorWithCode(pushy::error_codes::kInvalidOptions, ERROR_OPTIONS)); return; } NSString *originHash = PushyOptionString(options, @"originHash"); if (type == PushyTypePatchFromPpk && PushyStringIsBlank(originHash)) { - callback(PushyErrorWithMessage(ERROR_OPTIONS)); + callback(PushyErrorWithCode(pushy::error_codes::kInvalidOptions, ERROR_OPTIONS)); return; } NSString *dir = [RCTPushy downloadDir]; BOOL success = [self ensureDirectoryExistsAtPath:dir]; if (!success) { - callback(PushyErrorWithMessage(ERROR_FILE_OPERATION)); + callback(PushyErrorWithCode(pushy::error_codes::kFileOperationFailed, ERROR_FILE_OPERATION)); return; } NSString *zipFilePath = [dir stringByAppendingPathComponent:[NSString stringWithFormat:@"%@%@",hash, [self zipExtension:type]]]; + // On failure, remove the partial version directory like Android/Harmony + // do: a half-unzipped/half-patched dir leaks disk and could later be + // mistaken for a complete version. hash is validated non-blank above, so + // this can never resolve to the download root itself. + NSString *unzipDir = [dir stringByAppendingPathComponent:hash]; + void (^completion)(NSError *) = ^(NSError *error) { + if (error != nil) { + dispatch_async(self->_fileQueue, ^{ + [[NSFileManager defaultManager] removeItemAtPath:unzipDir error:nil]; + }); + } + callback(error); + }; + RCTLogInfo(@"RCTPushy -- download file %@", updateUrl); [RCTPushyDownloader download:updateUrl savePath:zipFilePath progressHandler:^(long long receivedBytes, long long totalBytes) { if (self->hasListeners) { @@ -556,14 +613,14 @@ - (void)performUpdate:(PushyType)type options:(NSDictionary *)options callback:( } } completionHandler:^(NSString *path, NSError *error) { if (error != nil) { - callback(error); + completion(error); return; } [self unzipDownloadedPackage:zipFilePath hash:hash type:type originHash:originHash - callback:callback]; + callback:completion]; }]; } @@ -629,7 +686,7 @@ - (void)applyPatchForHash:(NSString *)hash NSString *destination = [unzipDir stringByAppendingPathComponent:BUNDLE_FILE_NAME]; NSData *data = [NSData dataWithContentsOfFile:sourcePatch]; if (data == nil) { - callback(PushyErrorWithMessage(@"missing patch manifest")); + callback(PushyErrorWithCode(pushy::error_codes::kPatchFailed, @"missing patch manifest")); return; } @@ -640,7 +697,7 @@ - (void)applyPatchForHash:(NSString *)hash return; } if (![jsonObject isKindOfClass:[NSDictionary class]]) { - callback(PushyErrorWithMessage(@"invalid patch manifest")); + callback(PushyErrorWithCode(pushy::error_codes::kPatchFailed, @"invalid patch manifest")); return; } NSDictionary *json = (NSDictionary *)jsonObject; @@ -695,7 +752,7 @@ - (BOOL)switchVersion:(NSString *)hash error:(NSError **)error { if (PushyStringIsBlank(hash)) { if (error != NULL) { - *error = PushyErrorWithMessage(ERROR_OPTIONS); + *error = PushyErrorWithCode(pushy::error_codes::kInvalidOptions, ERROR_OPTIONS); } return NO; } @@ -758,7 +815,7 @@ - (void)unzipFileAtPath:(NSString *)path NSError *unzipError = error; if (!succeeded && unzipError == nil) { - unzipError = PushyErrorWithMessage(@"unzip failed"); + unzipError = PushyErrorWithCode(pushy::error_codes::kPatchFailed, @"unzip failed"); } completionHandler(unzipError); }]; diff --git a/ios/RCTPushy/RCTPushyDownloader.mm b/ios/RCTPushy/RCTPushyDownloader.mm index 982e5fc7..632450f5 100644 --- a/ios/RCTPushy/RCTPushyDownloader.mm +++ b/ios/RCTPushy/RCTPushyDownloader.mm @@ -113,7 +113,8 @@ - (void)URLSession:(NSURLSession *)session downloadTask:(NSURLSessionDownloadTas // before touching savePath so an existing valid package is not destroyed. NSURLResponse *response = downloadTask.response; if ([response isKindOfClass:[NSHTTPURLResponse class]]) { - NSInteger statusCode = ((NSHTTPURLResponse *)response).statusCode; + NSHTTPURLResponse *httpResponse = (NSHTTPURLResponse *)response; + NSInteger statusCode = httpResponse.statusCode; if (statusCode < 200 || statusCode >= 300) { self.fileError = [NSError errorWithDomain:RCTPushyDownloaderErrorDomain code:statusCode @@ -122,6 +123,26 @@ - (void)URLSession:(NSURLSession *)session downloadTask:(NSURLSessionDownloadTas }]; return; } + + // Reject truncated transfers like Android/Harmony do. Skip the check + // for encoded responses (e.g. gzip): NSURLSession decompresses them + // transparently, so the on-disk size legitimately differs from the + // Content-Length of the encoded body. + NSString *contentEncoding = [httpResponse.allHeaderFields[@"Content-Encoding"] lowercaseString]; + BOOL isEncodedBody = contentEncoding.length > 0 && ![contentEncoding isEqualToString:@"identity"]; + long long expectedLength = httpResponse.expectedContentLength; + if (!isEncodedBody && expectedLength > 0) { + NSNumber *fileSize = [[NSFileManager defaultManager] attributesOfItemAtPath:location.path + error:nil][NSFileSize]; + if (fileSize != nil && fileSize.longLongValue != expectedLength) { + self.fileError = [NSError errorWithDomain:RCTPushyDownloaderErrorDomain + code:-1 + userInfo:@{ + NSLocalizedDescriptionKey: [NSString stringWithFormat:@"download incomplete: expected %lld bytes, got %lld", expectedLength, fileSize.longLongValue], + }]; + return; + } + } } NSFileManager *fileManager = [NSFileManager defaultManager]; From ba0c75972b8523fa1cd7d028c6dfba497383fb17 Mon Sep 17 00:00:00 2001 From: sunnylqm Date: Tue, 7 Jul 2026 21:31:45 +0800 Subject: [PATCH 3/3] 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 --- ios/RCTPushy/RCTPushy.mm | 5 ++++- src/type.ts | 6 ++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/ios/RCTPushy/RCTPushy.mm b/ios/RCTPushy/RCTPushy.mm index de8cef5e..9b16b594 100644 --- a/ios/RCTPushy/RCTPushy.mm +++ b/ios/RCTPushy/RCTPushy.mm @@ -693,7 +693,10 @@ - (void)applyPatchForHash:(NSString *)hash NSError *error = nil; id jsonObject = [NSJSONSerialization JSONObjectWithData:data options:NSJSONReadingAllowFragments error:&error]; if (error != nil) { - callback(error); + // Classify as a patch failure like the sibling manifest branches; + // unclassified errors would otherwise be tagged DOWNLOAD_FAILED by the + // downloadUpdate fallback even though the download itself succeeded. + callback(PushyErrorWithCode(pushy::error_codes::kPatchFailed, error.localizedDescription)); return; } if (![jsonObject isKindOfClass:[NSDictionary class]]) { diff --git a/src/type.ts b/src/type.ts index 14bc08e1..f1943337 100644 --- a/src/type.ts +++ b/src/type.ts @@ -1,3 +1,5 @@ +import type { UpdateErrorCode } from './error'; + export interface VersionInfo { name: string; hash: string; @@ -61,8 +63,8 @@ export type EventType = | 'errorInstallApk'; export interface EventData { - /** Stable machine-readable error code (see UpdateErrorCode); present on error events */ - code?: string; + /** Stable machine-readable error code; present on error events */ + code?: UpdateErrorCode; currentVersion: string; cInfo: { rnu: string;