From 73ef2e63bed466c4f4e39189f159cc64c5a0d8d5 Mon Sep 17 00:00:00 2001 From: Floyd Kim Date: Wed, 26 Aug 2026 19:02:46 +0900 Subject: [PATCH 01/10] fix(ios): report the error code a failed download really carries `%lu` formats an unsigned long, and the code handed to it is a signed one that is negative for every URL loading error: `NSURLErrorNetworkConnectionLost` reached JS as 18446744073709550611 rather than -1005. The value was deterministic, so it could be mapped back, but nothing about it read as an error code - and the codes are the only stable way JS tells a connection that dropped apart from one that timed out, since the message alongside them is localized. --- ios/CodePush/CodePush.mm | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ios/CodePush/CodePush.mm b/ios/CodePush/CodePush.mm index ea74b58e..aa347bf7 100644 --- a/ios/CodePush/CodePush.mm +++ b/ios/CodePush/CodePush.mm @@ -772,7 +772,7 @@ -(void)loadBundleOnTick:(NSTimer *)timer { NSDictionary *newPackage = [CodePushPackage getPackage:mutableUpdatePackage[PackageHashKey] error:&err]; if (err) { - return reject([NSString stringWithFormat: @"%lu", (long)err.code], err.localizedDescription, err); + return reject([NSString stringWithFormat: @"%ld", (long)err.code], err.localizedDescription, err); } if (updateArchiveResult) { @@ -796,7 +796,7 @@ -(void)loadBundleOnTick:(NSTimer *)timer { // Stop observing frame updates if the download fails. _didUpdateProgress = NO; self.paused = YES; - reject([NSString stringWithFormat: @"%lu", (long)err.code], err.localizedDescription, err); + reject([NSString stringWithFormat: @"%ld", (long)err.code], err.localizedDescription, err); }]; } @@ -876,7 +876,7 @@ - (void)restartAppInternal:(BOOL)onlyIfUpdateIsPending NSMutableDictionary *package = [[CodePushPackage getCurrentPackage:&error] mutableCopy]; if (error) { - return reject([NSString stringWithFormat: @"%lu", (long)error.code], error.localizedDescription, error); + return reject([NSString stringWithFormat: @"%ld", (long)error.code], error.localizedDescription, error); } else if (package == nil) { // The app hasn't downloaded any CodePush updates yet, // so we simply return nil regardless if the user @@ -930,7 +930,7 @@ - (void)restartAppInternal:(BOOL)onlyIfUpdateIsPending error:&error]; if (error) { - reject([NSString stringWithFormat: @"%lu", (long)error.code], error.localizedDescription, error); + reject([NSString stringWithFormat: @"%ld", (long)error.code], error.localizedDescription, error); } else { [self savePendingUpdate:updatePackage[PackageHashKey] isLoading:NO]; From 981fa2f2a4f8f8b9a10c850bfcfb06d39b538eba Mon Sep 17 00:00:00 2001 From: Floyd Kim Date: Wed, 26 Aug 2026 19:03:27 +0900 Subject: [PATCH 02/10] fix(android): bound a download that stops making progress `URLConnection` reads a timeout of 0 as no timeout at all, which is what both downloads were opened with: `getInputStream()` and every `read()` after it could wait for as long as the operating system kept the socket alive. iOS has always bounded its download - 60 seconds of silence and the request fails - so a stalled download was a failure there and an indefinite wait here. A caller that gives up on its own clock does not close the connection underneath this, so the download went on running long after anyone was waiting for it, and reported whatever it eventually hit as if it had just happened. The read timeout bounds silence rather than the download, so a slow connection that keeps delivering is still never cut off. --- .../codepush/react/CodePushConstants.java | 18 ++++++++++++++++++ .../codepush/react/CodePushUpdateManager.java | 4 ++++ 2 files changed, 22 insertions(+) diff --git a/android/app/src/main/java/com/microsoft/codepush/react/CodePushConstants.java b/android/app/src/main/java/com/microsoft/codepush/react/CodePushConstants.java index e70fdfcf..127bdab2 100644 --- a/android/app/src/main/java/com/microsoft/codepush/react/CodePushConstants.java +++ b/android/app/src/main/java/com/microsoft/codepush/react/CodePushConstants.java @@ -36,6 +36,24 @@ public class CodePushConstants { public static final String DEFAULT_JS_BUNDLE_NAME = "index.android.bundle"; public static final String DIFF_MANIFEST_FILE_NAME = "hotcodepush.json"; public static final int DOWNLOAD_BUFFER_SIZE = 1024 * 256; + + /** + * How long a download waits for the server to answer at all. + * + * Kept well above any real handshake, because the only thing this has to catch is a + * host that will never answer - a captive portal, a dead route, a network that went + * away between the update check and the download. + */ + public static final int DOWNLOAD_CONNECT_TIMEOUT_IN_MS = 10 * 1000; + + /** + * How long a download waits for the next bytes of a response already flowing. + * + * This bounds silence, not the download: a slow connection that keeps delivering is + * never cut off, however long the whole archive takes. A connection that delivers + * nothing for this long is one the operating system has stopped reporting as dead. + */ + public static final int DOWNLOAD_READ_TIMEOUT_IN_MS = 30 * 1000; public static final String DOWNLOAD_FILE_NAME = "download.zip"; public static final String DOWNLOAD_PROGRESS_EVENT_NAME = "CodePushDownloadProgress"; public static final String DOWNLOAD_URL_KEY = "downloadUrl"; diff --git a/android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java b/android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java index f8e0d887..d49a41a8 100644 --- a/android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java +++ b/android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java @@ -296,6 +296,8 @@ ArchiveRestoreResult downloadAndInstallPackage(JSONObject updatePackage, String try { URL downloadUrl = new URL(downloadUrlString); connection = (HttpURLConnection) (downloadUrl.openConnection()); + connection.setConnectTimeout(CodePushConstants.DOWNLOAD_CONNECT_TIMEOUT_IN_MS); + connection.setReadTimeout(CodePushConstants.DOWNLOAD_READ_TIMEOUT_IN_MS); if (android.os.Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP && downloadUrl.toString().startsWith("https")) { @@ -507,6 +509,8 @@ public void downloadAndReplaceCurrentBundle(String remoteBundleUrl, String bundl try { downloadUrl = new URL(remoteBundleUrl); connection = (HttpURLConnection) (downloadUrl.openConnection()); + connection.setConnectTimeout(CodePushConstants.DOWNLOAD_CONNECT_TIMEOUT_IN_MS); + connection.setReadTimeout(CodePushConstants.DOWNLOAD_READ_TIMEOUT_IN_MS); bin = new BufferedInputStream(connection.getInputStream()); File downloadFile = new File(getCurrentPackageBundlePath(bundleFileName)); downloadFile.delete(); From f9ad794885ea75adb407929d79a6810f12152281 Mon Sep 17 00:00:00 2001 From: Floyd Kim Date: Wed, 26 Aug 2026 19:06:34 +0900 Subject: [PATCH 03/10] fix(android): fail a download the server answered with an error status Nothing read the response status, so a 4xx or a 5xx reached the caller as whatever `getInputStream()` happened to throw for it - an IOException among all the other IOExceptions a download can end in, with no way to tell a release served from a CDN that answered 503 apart from a connection that dropped. iOS has always refused a status of 400 or above, and names it the same way, so one release answered with one status now reads the same in both platforms' reports. The status is read before the body, because asking `getInputStream()` first turns some statuses into a stream over the error page and others into an exception that no longer knows which status it was. --- .../codepush/react/CodePushHttpException.java | 29 +++++++++++++++++++ .../codepush/react/CodePushUpdateManager.java | 15 ++++++++++ .../CodePushUpdateManagerDownloadTest.java | 19 ++++++++++++ 3 files changed, 63 insertions(+) create mode 100644 android/app/src/main/java/com/microsoft/codepush/react/CodePushHttpException.java diff --git a/android/app/src/main/java/com/microsoft/codepush/react/CodePushHttpException.java b/android/app/src/main/java/com/microsoft/codepush/react/CodePushHttpException.java new file mode 100644 index 00000000..fd419b9c --- /dev/null +++ b/android/app/src/main/java/com/microsoft/codepush/react/CodePushHttpException.java @@ -0,0 +1,29 @@ +package com.microsoft.codepush.react; + +import java.io.IOException; + +/** + * The server answered a download with a status that is not a body to install. + * + * An I/O exception rather than one of this package's unchecked ones, because every caller + * of a download already handles `IOException` and this is one more way a download does not + * arrive - and because the alternative, an unchecked exception, would escape the download + * path uncaught. + * + * The message reads the way the other platform's does, so one release answered with the + * same status reads the same in both platforms' reports. + */ +public class CodePushHttpException extends IOException { + + private final int mStatusCode; + + public CodePushHttpException(String url, int statusCode) { + super("Received " + statusCode + " response from " + url); + mStatusCode = statusCode; + } + + /** The status the server answered with. */ + public int getStatusCode() { + return mStatusCode; + } +} diff --git a/android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java b/android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java index d49a41a8..62b24429 100644 --- a/android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java +++ b/android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java @@ -309,6 +309,15 @@ ArchiveRestoreResult downloadAndInstallPackage(JSONObject updatePackage, String } connection.setRequestProperty("Accept-Encoding", "identity"); + + // Read before the body is: an error status carries a body of its own, and + // asking `getInputStream()` for it first turns some statuses into a stream and + // others into an exception that says nothing about which status it was. + int responseCode = connection.getResponseCode(); + if (responseCode >= 400) { + throw new CodePushHttpException(downloadUrlString, responseCode); + } + bin = new BufferedInputStream(connection.getInputStream()); // Announced only once the response is flowing, so a connection that fails to @@ -511,6 +520,12 @@ public void downloadAndReplaceCurrentBundle(String remoteBundleUrl, String bundl connection = (HttpURLConnection) (downloadUrl.openConnection()); connection.setConnectTimeout(CodePushConstants.DOWNLOAD_CONNECT_TIMEOUT_IN_MS); connection.setReadTimeout(CodePushConstants.DOWNLOAD_READ_TIMEOUT_IN_MS); + + int responseCode = connection.getResponseCode(); + if (responseCode >= 400) { + throw new CodePushHttpException(remoteBundleUrl, responseCode); + } + bin = new BufferedInputStream(connection.getInputStream()); File downloadFile = new File(getCurrentPackageBundlePath(bundleFileName)); downloadFile.delete(); diff --git a/android/app/src/test/java/com/microsoft/codepush/react/CodePushUpdateManagerDownloadTest.java b/android/app/src/test/java/com/microsoft/codepush/react/CodePushUpdateManagerDownloadTest.java index 8bc23645..7f12fe48 100644 --- a/android/app/src/test/java/com/microsoft/codepush/react/CodePushUpdateManagerDownloadTest.java +++ b/android/app/src/test/java/com/microsoft/codepush/react/CodePushUpdateManagerDownloadTest.java @@ -2,6 +2,7 @@ import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -138,6 +139,24 @@ public void reportsAnInvalidManifestWhenThePatchUrlDoesNotServeAnArchive() throw assertFalse("bytes that are not an update must not reach the package folder", mPackageFolder.exists()); } + @Test + public void failsTheDownloadWhenTheServerAnswersTheArchiveWithAnErrorStatus() throws IOException { + // Nothing is served at this path, so the server answers it with a 404. + String fullUrl = mServer.urlOf("/missing-full.zip"); + + try { + updateManager(applierWriting(TARGET_BUNDLE)).downloadPackage( + fullUpdatePackage(mPackageHash, fullUrl), BUNDLE_FILE_NAME, ignoreProgress()); + fail("a download the server refused must not be reported as installed"); + } catch (CodePushHttpException e) { + assertEquals(404, e.getStatusCode()); + assertTrue("the message names the status the server answered with", + e.getMessage().contains("404")); + } + + assertFalse("nothing the server refused reaches the package folder", mPackageFolder.exists()); + } + @Test public void fallsBackToTheFullArchiveWhenApplyingThePatchRunsOutOfMemory() throws IOException { String patchUrl = serve("/patch.zip", zipOf(patchArchiveContents())); From e95f358fd15323454d4916dc75eac4d0ad3bda43 Mon Sep 17 00:00:00 2001 From: Floyd Kim Date: Wed, 26 Aug 2026 20:08:21 +0900 Subject: [PATCH 04/10] feat(android): name what kind of failure a download ended in A promise rejected with a bare throwable is given the code `EUNSPECIFIED` by React Native, so every way an Android download can fail arrived in JS under one code with only the message telling them apart - and the message is an exception's own words, which is not a value a report can group by. iOS has always carried the `NSURLError` code, so the same failure was classifiable on one platform and not on the other. The four categories are the ones that ask for different things to happen next. A connection that dropped is worth waiting out; a server that answered 404 is not the network's doing; an update that failed its integrity check will fail it again however many times it is downloaded. A category no caller would act differently on would only make the reports wider, which is why there is no category per way a download can fail. --- .../codepush/react/CodePushErrorCode.java | 82 +++++++++++++++++++ .../codepush/react/CodePushNativeModule.java | 4 +- .../codepush/react/CodePushErrorCodeTest.java | 70 ++++++++++++++++ docs/telemetry-callbacks.ko.md | 20 +++++ docs/telemetry-callbacks.md | 22 +++++ 5 files changed, 196 insertions(+), 2 deletions(-) create mode 100644 android/app/src/main/java/com/microsoft/codepush/react/CodePushErrorCode.java create mode 100644 android/app/src/test/java/com/microsoft/codepush/react/CodePushErrorCodeTest.java diff --git a/android/app/src/main/java/com/microsoft/codepush/react/CodePushErrorCode.java b/android/app/src/main/java/com/microsoft/codepush/react/CodePushErrorCode.java new file mode 100644 index 00000000..1dd5022c --- /dev/null +++ b/android/app/src/main/java/com/microsoft/codepush/react/CodePushErrorCode.java @@ -0,0 +1,82 @@ +package com.microsoft.codepush.react; + +import java.net.SocketException; +import java.net.SocketTimeoutException; +import java.net.UnknownHostException; + +import javax.net.ssl.SSLException; + +/** + * What kind of failure a download ended in, in the words JS reads it by. + * + * React Native fills the code of a promise rejected with a bare throwable with + * `EUNSPECIFIED`, which left the message the only thing telling one failure apart from + * another - and a message is written for a person, not for a report to group by. + * + * The categories are the ones that ask for different things to happen next, which is why + * there are four of them rather than one per way a download can fail. A category no + * caller would act differently on is a category that only makes the reports wider. + */ +public class CodePushErrorCode { + + /** The network did not carry the download: the socket dropped, timed out, or never opened. */ + public static final String NETWORK = "CODE_PUSH_NETWORK"; + + /** The server answered, with a status that is not a body to install. */ + public static final String HTTP = "CODE_PUSH_HTTP"; + + /** + * The downloaded contents do not hash to the release's package hash, or hold no JS + * bundle by the name the app looks for. Downloading them again cannot help. + */ + public static final String INTEGRITY = "CODE_PUSH_INTEGRITY"; + + /** Nothing here has a word for it, and inventing one would only be a guess. */ + public static final String UNKNOWN = "CODE_PUSH_UNKNOWN"; + + private CodePushErrorCode() { + } + + /** + * The category of a failure, read through its causes: the download wraps some of what it + * catches, and the wrapper is never the part that says what went wrong. + */ + public static String of(Throwable error) { + for (Throwable cause = error; cause != null; cause = cause.getCause()) { + if (cause instanceof CodePushHttpException) { + return HTTP; + } + + if (cause instanceof CodePushInvalidUpdateException) { + return INTEGRITY; + } + + if (isTransportFailure(cause)) { + return NETWORK; + } + } + + return UNKNOWN; + } + + /** + * Whether the download failed because the network did not carry it. + * + * A server that answered is not this, however it answered: the connection worked, and + * asking a different URL over it is worth doing. A connection that never opened or that + * dropped is, and every URL behind it is equally out of reach. + */ + public static boolean isNetworkFailure(Throwable error) { + return NETWORK.equals(of(error)); + } + + private static boolean isTransportFailure(Throwable error) { + // `SocketException` is the one that covers the reported majority - the connection + // reset and the connection aborted an app being backgrounded mid-download leaves + // behind - along with the connection that was refused or had no route. + return error instanceof SocketTimeoutException + || error instanceof SocketException + || error instanceof UnknownHostException + || error instanceof SSLException; + } +} diff --git a/android/app/src/main/java/com/microsoft/codepush/react/CodePushNativeModule.java b/android/app/src/main/java/com/microsoft/codepush/react/CodePushNativeModule.java index 4a9f1963..e3276683 100644 --- a/android/app/src/main/java/com/microsoft/codepush/react/CodePushNativeModule.java +++ b/android/app/src/main/java/com/microsoft/codepush/react/CodePushNativeModule.java @@ -402,10 +402,10 @@ public void doFrame(long frameTimeNanos) { } catch (CodePushInvalidUpdateException e) { CodePushUtils.log(e); mSettingsManager.saveFailedUpdate(CodePushUtils.convertReadableToJsonObject(updatePackage)); - promise.reject(e); + promise.reject(CodePushErrorCode.of(e), e.getMessage(), e); } catch (IOException | CodePushUnknownException e) { CodePushUtils.log(e); - promise.reject(e); + promise.reject(CodePushErrorCode.of(e), e.getMessage(), e); } } }); diff --git a/android/app/src/test/java/com/microsoft/codepush/react/CodePushErrorCodeTest.java b/android/app/src/test/java/com/microsoft/codepush/react/CodePushErrorCodeTest.java new file mode 100644 index 00000000..dbe09942 --- /dev/null +++ b/android/app/src/test/java/com/microsoft/codepush/react/CodePushErrorCodeTest.java @@ -0,0 +1,70 @@ +package com.microsoft.codepush.react; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +import java.io.IOException; +import java.net.ConnectException; +import java.net.SocketException; +import java.net.SocketTimeoutException; +import java.net.UnknownHostException; + +import javax.net.ssl.SSLHandshakeException; + +public class CodePushErrorCodeTest { + + @Test + public void namesAConnectionThatDroppedAsANetworkFailure() { + // The message the reported majority of Android failures arrive with. + assertEquals(CodePushErrorCode.NETWORK, + CodePushErrorCode.of(new SocketException("Software caused connection abort"))); + } + + @Test + public void namesAConnectionThatTimedOutAsANetworkFailure() { + assertEquals(CodePushErrorCode.NETWORK, CodePushErrorCode.of(new SocketTimeoutException("timeout"))); + } + + @Test + public void namesAConnectionThatNeverOpenedAsANetworkFailure() { + assertEquals(CodePushErrorCode.NETWORK, CodePushErrorCode.of(new ConnectException("Connection refused"))); + assertEquals(CodePushErrorCode.NETWORK, CodePushErrorCode.of(new UnknownHostException("cdn.example.test"))); + assertEquals(CodePushErrorCode.NETWORK, CodePushErrorCode.of(new SSLHandshakeException("handshake failed"))); + } + + @Test + public void namesAnErrorStatusAsAnHttpFailureRatherThanANetworkOne() { + Throwable error = new CodePushHttpException("https://cdn.example.test/full.zip", 503); + + assertEquals(CodePushErrorCode.HTTP, CodePushErrorCode.of(error)); + assertFalse("a server that answered is not a network that failed", + CodePushErrorCode.isNetworkFailure(error)); + } + + @Test + public void namesAnUpdateThatIsNotWhatItClaimedAsAnIntegrityFailure() { + assertEquals(CodePushErrorCode.INTEGRITY, + CodePushErrorCode.of(new CodePushInvalidUpdateException("The update contents failed the data integrity check."))); + } + + @Test + public void readsThroughTheWrapperADownloadCatchesItsFailuresIn() { + Throwable wrapped = new CodePushUnknownException("Error closing IO resources.", + new SocketException("Connection reset")); + + assertEquals(CodePushErrorCode.NETWORK, CodePushErrorCode.of(wrapped)); + assertTrue(CodePushErrorCode.isNetworkFailure(wrapped)); + } + + @Test + public void leavesAFailureNothingHasAWordForUnnamed() { + Throwable error = new IOException("the disk is full"); + + assertEquals(CodePushErrorCode.UNKNOWN, CodePushErrorCode.of(error)); + assertFalse("an unnamed failure must not be retried as if the network caused it", + CodePushErrorCode.isNetworkFailure(error)); + } +} diff --git a/docs/telemetry-callbacks.ko.md b/docs/telemetry-callbacks.ko.md index 63ebc968..cc8dadfe 100644 --- a/docs/telemetry-callbacks.ko.md +++ b/docs/telemetry-callbacks.ko.md @@ -23,6 +23,26 @@ > [!NOTE] > 타입 정의에서 `onRolloutSkipped`는 두 번째 `error` 매개변수를 선언하지만, 런타임은 항상 `label`만 전달합니다. +## `error`가 담고 있는 것 + +리포트는 메시지가 아니라 `error.code`로 묶으세요. 메시지는 iOS에서 현지화되고 Android에서는 예외 자체의 문구입니다. + +| 플랫폼 | `error.code` | 예 | +|---|---|---| +| iOS | [`NSURLError`](https://developer.apple.com/documentation/foundation/1508628-url_loading_system_error_codes) 코드를 문자열로, CodePush가 직접 던진 오류는 `-1` | `"-1005"`, 연결이 끊김 | +| Android | 실패의 카테고리 | `"CODE_PUSH_NETWORK"`, 연결이 끊김 | + +Android 카테고리는 다음과 같습니다. + +| Code | 뜻 | 다시 받아 볼 가치 | +|---|---|---| +| `CODE_PUSH_NETWORK` | 연결이 끊겼거나, 시간이 초과됐거나, 열리지 않았습니다. | 네트워크가 돌아오면 있음 | +| `CODE_PUSH_HTTP` | 서버가 400 이상으로 응답했습니다. 상태 코드는 메시지에 있습니다. | 상태 코드에 따라 다름 | +| `CODE_PUSH_INTEGRITY` | 다운로드한 내용의 hash가 릴리스의 package hash와 다르거나, 그 안에 앱이 찾는 이름의 JS 번들이 없습니다. | 없음 | +| `CODE_PUSH_UNKNOWN` | 그 밖의 경우입니다. | 알 수 없음 | + +`CodePush.sync()`도 같은 오류로 거절되므로, 반환값을 기다리는 호출자에게는 이 콜백이 필요 없습니다. + ## 등록 [앱에 CodePush 적용하기](../README.md#4-codepush-ify-your-app)의 `CodePush({ ... })` 래퍼에 콜백을 전달하면, `checkFrequency` 값과 무관하게 직접 호출한 `CodePush.sync()`를 포함한 모든 sync에서 실행됩니다. diff --git a/docs/telemetry-callbacks.md b/docs/telemetry-callbacks.md index 88584000..ba58a36a 100644 --- a/docs/telemetry-callbacks.md +++ b/docs/telemetry-callbacks.md @@ -30,6 +30,28 @@ the sync failed before a release was resolved. > The typings declare a second `error` parameter on `onRolloutSkipped`, but the runtime only > ever passes the label. +## What `error` carries + +Group reports by `error.code`, not by the message: the message is localized on iOS and is +the exception's own words on Android. + +| Platform | `error.code` | Example | +|---|---|---| +| iOS | the [`NSURLError`](https://developer.apple.com/documentation/foundation/1508628-url_loading_system_error_codes) code as a string, or `-1` for an error CodePush raised itself | `"-1005"`, the connection dropped | +| Android | the category of the failure | `"CODE_PUSH_NETWORK"`, the connection dropped | + +Android categories: + +| Code | Means | Worth downloading again | +|---|---|---| +| `CODE_PUSH_NETWORK` | The connection dropped, timed out, or never opened. | Once the network is back | +| `CODE_PUSH_HTTP` | The server answered with a status of 400 or above. The status is in the message. | Depends on the status | +| `CODE_PUSH_INTEGRITY` | The downloaded contents do not hash to the release's package hash, or hold no JS bundle by the name the app looks for. | No | +| `CODE_PUSH_UNKNOWN` | Anything else. | Unknown | + +`CodePush.sync()` rejects with the same error, so a caller that awaits it does not need +this callback. + ## Registering them Pass them to the `CodePush({ ... })` wrapper from From 8cf219f632baf043984049114042841baeade9d6 Mon Sep 17 00:00:00 2001 From: Floyd Kim Date: Wed, 26 Aug 2026 20:08:32 +0900 Subject: [PATCH 05/10] fix: stop falling back to the full archive when the connection failed A release published with a binary patch offers up to three archives, and the client moves on to the next one whenever it cannot use the one it has. A connection that dropped, timed out, or never opened was read that way too: the client fell back, and downloaded the full archive - the largest of the three, from nothing - over the network that had just failed to carry the smallest. So a download that had already failed spent a second, longer download to fail again, and reported the second failure as if the first had not happened. It now ends with what the network did. A server that answered is untouched by this: a 404 on one archive says nothing about the next, so the client still moves on the way it always has. --- .../codepush/react/CodePushUpdateManager.java | 26 ++++++++-- .../CodePushUpdateManagerDownloadTest.java | 48 +++++++++++++++++ docs/diff-updates.ko.md | 4 +- docs/diff-updates.md | 7 ++- ios/CodePush/CodePush.h | 1 + ios/CodePush/CodePushErrorUtils.m | 31 +++++++++++ ios/CodePush/CodePushPackage.m | 19 +++++-- ios/CodePushTests/CodePushErrorUtilsTests.m | 51 +++++++++++++++++++ 8 files changed, 179 insertions(+), 8 deletions(-) create mode 100644 ios/CodePushTests/CodePushErrorUtilsTests.m diff --git a/android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java b/android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java index 62b24429..5d624ff1 100644 --- a/android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java +++ b/android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java @@ -218,8 +218,9 @@ private static String optArchiveDownloadUrl(JSONObject updatePackage, String dow /** * Installs the update from one of its patch archives. * - * Every way this can fail ends the same way, with the caller moving on to the next - * archive, so none of it is reported to the caller as an error. The ladder cannot loop: + * Every verdict on the archive ends the same way, with the caller moving on to the next + * one, so none of it is reported to the caller as an error. A network that did not carry + * the archive is not a verdict on it and is raised instead. The ladder cannot loop: * which archive comes next is the caller's decision alone, and the full archive at its * end is downloaded by a call that is not allowed to take the patch path, so it has no * failure of its own to fall back from. @@ -228,10 +229,13 @@ private static String optArchiveDownloadUrl(JSONObject updatePackage, String dow * whoever asked for the download * @return true when the update was installed, false when the caller has to move on to * the next archive + * @throws IOException when the network did not carry the archive, which is not a verdict + * on the archive and so is not a reason to try another one */ private boolean tryDownloadArchivePackage(JSONObject updatePackage, String expectedBundleFileName, DownloadProgressCallback progressCallback, - String archiveDownloadUrl, ArchiveAttemptLog patchAttempt) { + String archiveDownloadUrl, ArchiveAttemptLog patchAttempt) + throws IOException { try { ArchiveRestoreResult patchResult = downloadAndInstallPackage(updatePackage, expectedBundleFileName, progressCallback, archiveDownloadUrl, true, patchAttempt); @@ -243,6 +247,22 @@ private boolean tryDownloadArchivePackage(JSONObject updatePackage, String expec CodePushUtils.log("The " + patchAttempt.currentArchive() + " archive failed (" + patchResult.getFailureReason() + "). Falling back."); } catch (Exception | OutOfMemoryError e) { + if (CodePushErrorCode.isNetworkFailure(e)) { + // The network is what failed, not the archive, and the full archive is behind + // the same network - only larger, and started over from nothing. Falling back + // here would spend a second download to reach the failure already in hand. + CodePushUtils.log("The " + patchAttempt.currentArchive() + + " archive could not be downloaded. Giving up on the download."); + if (e instanceof IOException) { + throw (IOException) e; + } + + // A network failure read out of a wrapper this package raised, which the + // caller catches by its own type rather than by `IOException`. + throw new CodePushUnknownException( + "The " + patchAttempt.currentArchive() + " archive could not be downloaded.", e); + } + // Applying a patch is the one path that holds a whole bundle in memory, so // running out of it is a failure this has to absorb like any other: by the time // it lands here the arrays are unreachable, and the full archive is downloaded diff --git a/android/app/src/test/java/com/microsoft/codepush/react/CodePushUpdateManagerDownloadTest.java b/android/app/src/test/java/com/microsoft/codepush/react/CodePushUpdateManagerDownloadTest.java index 7f12fe48..7da09ab3 100644 --- a/android/app/src/test/java/com/microsoft/codepush/react/CodePushUpdateManagerDownloadTest.java +++ b/android/app/src/test/java/com/microsoft/codepush/react/CodePushUpdateManagerDownloadTest.java @@ -346,6 +346,54 @@ public void skipsThePatchArchiveWhenTheAssetDiffCannotBeDownloaded() throws IOEx assertInstalledContents(updateHash, updateContents); } + @Test + public void givesUpTheDownloadWhenTheNetworkCannotCarryTheAssetDiff() throws IOException { + // A refused connection is the network failing rather than a verdict on the archive, + // and the archives behind it are behind the same network - the full one only larger + // and started over from nothing. + String unreachableDiffUrl = "http://127.0.0.1:" + portNothingListensOn() + "/diff.zip"; + String patchUrl = serve("/patch.zip", zipOf(patchArchiveContents())); + String fullUrl = serve("/full.zip", zipOf(fullArchiveContents())); + + try { + updateManager(applierWriting(TARGET_BUNDLE)).downloadPackage( + updatePackageWithAssetDiff(mPackageHash, fullUrl, patchUrl, unreachableDiffUrl), + BUNDLE_FILE_NAME, ignoreProgress()); + fail("a network that carried nothing must not be reported as an installed update"); + } catch (IOException e) { + assertEquals(CodePushErrorCode.NETWORK, CodePushErrorCode.of(e)); + } + + assertTrue("no archive behind the same network is asked for", mServer.requestedPaths().isEmpty()); + assertFalse("nothing that never arrived reaches the package folder", mPackageFolder.exists()); + } + + @Test + public void stillFallsBackWhenTheServerRefusesTheAssetDiffWithAnErrorStatus() throws IOException { + // A server that answered is not a network that failed: the connection worked, so the + // archives behind it are worth asking for. + Map updateContents = assetDiffTargetContents(); + String updateHash = packageHashOf(updateContents); + String diffUrl = mServer.urlOf("/missing-diff.zip"); + String patchUrl = serve("/patch.zip", zipOf(patchArchiveContentsForAssetDiffTarget())); + String fullUrl = serve("/full.zip", zipOf(updateContents)); + + JSONObject patchResult = updateManager(applierWriting(TARGET_BUNDLE)).downloadPackage( + updatePackageWithAssetDiff(updateHash, fullUrl, patchUrl, diffUrl), BUNDLE_FILE_NAME, ignoreProgress()); + + assertEquals(Arrays.asList("/missing-diff.zip", "/full.zip"), mServer.requestedPaths()); + assertInstalledContents(updateHash, updateContents); + assertFallbackResult(patchResult, null); + } + + /** A loopback port that is opened only to be closed, so connecting to it is refused. */ + private static int portNothingListensOn() throws IOException { + ServerSocket socket = new ServerSocket(0, 0, InetAddress.getByName("127.0.0.1")); + int port = socket.getLocalPort(); + socket.close(); + return port; + } + @Test public void installsFromThePatchArchiveWhenTheAssetDiffUrlIsAnEmptyString() throws IOException { // An empty slot is not an archive on offer. Attempting it would fail for want of a diff --git a/docs/diff-updates.ko.md b/docs/diff-updates.ko.md index 6feadd2f..50f9a768 100644 --- a/docs/diff-updates.ko.md +++ b/docs/diff-updates.ko.md @@ -85,12 +85,14 @@ binary patch로 배포한 릴리스는 **asset diff 아카이브**도 함께 담 | 2 | binary patch | 항상 쓸 수 있습니다. 모든 클라이언트가 가진 앱 바이너리의 번들을 기준으로 만들기 때문입니다. | | 3 | full | 항상 쓸 수 있습니다. 설치된 업데이트가 없어도 됩니다. | -이 순서에는 예외가 두 가지 있습니다. +이 순서에는 예외가 세 가지 있습니다. **모든 클라이언트가 셋을 다 시도하지는 않습니다.** 쓸 수 있는 asset diff가 없는 클라이언트는 binary patch에서 시작합니다. 앱 바이너리의 번들을 실행 중이거나, 새 릴리스가 diff를 만들지 않은 업데이트를 실행 중인 경우입니다. **asset diff가 실패해도 항상 binary patch로 넘어가지는 않습니다.** diff가 asset 영역에서 실패했을 때만 넘어갑니다. 설치된 업데이트와 병합하지 못했거나(`asset_merge_failed`), 병합된 내용이 package hash 검증에 실패한 경우(`package_verification_failed`)입니다. 그 밖의 실패는 두 아카이브가 공유하는 bundle patch에서 일어나므로 binary patch도 같은 방식으로 실패합니다. 이때 클라이언트는 곧바로 full 아카이브를 내려받습니다. +**연결이 실패하면 다운로드가 거기서 멈춥니다.** 다음 아카이브도 같은 네트워크 뒤에 있고 full 아카이브는 셋 중 가장 큽니다. 이어서 시도해 봐야 더 느리게 실패할 뿐이므로, 클라이언트는 연결 오류를 그대로 알립니다. 서버가 응답한 경우는 다릅니다. 한 아카이브의 404는 다음 아카이브를 건너뛸 이유가 되지 않습니다. + `onUpdateArchiveResult`는 시도한 아카이브를 모두 보고합니다. [텔레메트리 콜백](telemetry-callbacks.ko.md#onupdatearchiveresult가-보고하는-내용)을 참고하세요. ### 배포 조건 diff --git a/docs/diff-updates.md b/docs/diff-updates.md index aa4e6ed1..11a4573b 100644 --- a/docs/diff-updates.md +++ b/docs/diff-updates.md @@ -118,7 +118,7 @@ A client tries the archives in this order and stops at the first one it can inst | 2 | Binary patch | always. It is built against the bundle in the app binary, which every client has. | | 3 | Full | always. It needs nothing installed. | -There are two exceptions to this order. +There are three exceptions to this order. **Not every client tries all three.** One with no asset diff to use - it is running the bundle in the app binary, or an update this release was not diffed against - starts at the @@ -131,6 +131,11 @@ only when the diff failed on its asset side: the merge with the installed update both archives carry, so the binary patch would fail there the same way and the client goes straight to the full archive. +**A failed connection stops the download.** The next archive is behind the same network, +and the full one is the largest of the three, so trying it would only fail again more +slowly. The client reports the connection error instead. A server that answered is +different: a 404 on one archive is no reason to skip the next. + `onUpdateArchiveResult` reports every archive that was tried - see [Telemetry callbacks](telemetry-callbacks.md#what-onupdatearchiveresult-reports). diff --git a/ios/CodePush/CodePush.h b/ios/CodePush/CodePush.h index 3b0ba794..252477a7 100644 --- a/ios/CodePush/CodePush.h +++ b/ios/CodePush/CodePush.h @@ -158,6 +158,7 @@ failCallback:(void (^)(NSError *err))failCallback; + (NSError *)errorWithMessage:(NSString *)errorMessage; + (BOOL)isCodePushError:(NSError *)error; ++ (BOOL)isNetworkFailure:(NSError *)error; @end diff --git a/ios/CodePush/CodePushErrorUtils.m b/ios/CodePush/CodePushErrorUtils.m index 97dede4a..49af8cba 100644 --- a/ios/CodePush/CodePushErrorUtils.m +++ b/ios/CodePush/CodePushErrorUtils.m @@ -17,4 +17,35 @@ + (BOOL)isCodePushError:(NSError *)err return err != nil && [CodePushErrorDomain isEqualToString:err.domain]; } +/* + * Whether the request failed because the network did not carry it. + * + * A server that answered is not this, however it answered: the connection worked, and + * asking a different URL over it is worth doing. A connection that never opened or that + * dropped is, and every URL behind it is equally out of reach. + * + * Named from the codes rather than the domain, because `NSURLErrorDomain` also covers a + * URL that was malformed or a scheme that is not supported - failures of the request + * rather than of the network under it. + */ ++ (BOOL)isNetworkFailure:(NSError *)err +{ + if (err == nil || ![NSURLErrorDomain isEqualToString:err.domain]) { + return NO; + } + + switch (err.code) { + case NSURLErrorTimedOut: + case NSURLErrorCannotFindHost: + case NSURLErrorCannotConnectToHost: + case NSURLErrorNetworkConnectionLost: + case NSURLErrorDNSLookupFailed: + case NSURLErrorNotConnectedToInternet: + case NSURLErrorSecureConnectionFailed: + return YES; + default: + return NO; + } +} + @end \ No newline at end of file diff --git a/ios/CodePush/CodePushPackage.m b/ios/CodePush/CodePushPackage.m index 83947dc9..eba94811 100644 --- a/ios/CodePush/CodePushPackage.m +++ b/ios/CodePush/CodePushPackage.m @@ -122,9 +122,10 @@ + (void)downloadPackage:(NSDictionary *)updatePackage * only fail the same way, and trying them would put more doomed downloads in front of the * full one. * - * Every way the ladder can end, the update is installed - by an archive of the queue or - * by the full download behind it - so no failure here reaches the caller as an error, and - * the result retells the attempts one by one. + * Every verdict on an archive ends with the update installed - by an archive of the queue + * or by the full download behind it - so no verdict reaches the caller as an error, and the + * result retells the attempts one by one. A network that did not carry the archive is not a + * verdict on it: it is raised, because the full download is behind the same network. */ + (void)tryNextArchive:(NSArray *)archivesToTry attemptsSoFar:(NSMutableArray *)attempts @@ -201,6 +202,18 @@ + (void)tryNextArchive:(NSArray *)archivesToTry firstAttemptStartTime:firstAttemptStartTime]); } failCallback:^(NSError *err) { + if ([CodePushErrorUtils isNetworkFailure:err]) { + // The network is what failed, not the archive, and the full + // archive is behind the same network - only larger, and + // started over from nothing. Falling back here would spend a + // second download to reach the failure already in hand. + CPLog(@"The %@ archive could not be downloaded (%@). Giving up on the download.", + archive, err.localizedDescription); + [self deleteBinaryPatchFolder]; + failCallback(err); + return; + } + CPLog(@"The %@ archive could not be applied (%@). Falling back.", archive, err.localizedDescription); // An error raised after the bundle was restored is the restored // update failing the checks every update passes before it is diff --git a/ios/CodePushTests/CodePushErrorUtilsTests.m b/ios/CodePushTests/CodePushErrorUtilsTests.m new file mode 100644 index 00000000..ba3baa67 --- /dev/null +++ b/ios/CodePushTests/CodePushErrorUtilsTests.m @@ -0,0 +1,51 @@ +#import +#import "CodePush.h" + +@interface CodePushErrorUtilsTests : XCTestCase +@end + +@implementation CodePushErrorUtilsTests + +static NSError *urlError(NSInteger code) +{ + return [NSError errorWithDomain:NSURLErrorDomain code:code userInfo:nil]; +} + +- (void)testNamesAConnectionThatDroppedAsANetworkFailure +{ + XCTAssertTrue([CodePushErrorUtils isNetworkFailure:urlError(NSURLErrorNetworkConnectionLost)]); + XCTAssertTrue([CodePushErrorUtils isNetworkFailure:urlError(NSURLErrorTimedOut)]); +} + +- (void)testNamesAConnectionThatNeverOpenedAsANetworkFailure +{ + XCTAssertTrue([CodePushErrorUtils isNetworkFailure:urlError(NSURLErrorNotConnectedToInternet)]); + XCTAssertTrue([CodePushErrorUtils isNetworkFailure:urlError(NSURLErrorCannotConnectToHost)]); + XCTAssertTrue([CodePushErrorUtils isNetworkFailure:urlError(NSURLErrorDNSLookupFailed)]); + XCTAssertTrue([CodePushErrorUtils isNetworkFailure:urlError(NSURLErrorSecureConnectionFailed)]); +} + +- (void)testDoesNotNameAMalformedRequestAsANetworkFailure +{ + // The request never reached a network to fail on, so retrying it anywhere else is + // no more likely to work. + XCTAssertFalse([CodePushErrorUtils isNetworkFailure:urlError(NSURLErrorBadURL)]); + XCTAssertFalse([CodePushErrorUtils isNetworkFailure:urlError(NSURLErrorUnsupportedURL)]); +} + +- (void)testDoesNotNameAnErrorStatusAsANetworkFailure +{ + // What the download handler raises for a status of 400 or above: the connection + // worked, so the archives behind it are worth asking for. + NSError *error = [CodePushErrorUtils errorWithMessage:@"Received 503 response from https://cdn.example.test/full.zip"]; + + XCTAssertTrue([CodePushErrorUtils isCodePushError:error]); + XCTAssertFalse([CodePushErrorUtils isNetworkFailure:error]); +} + +- (void)testDoesNotNameANilErrorAsANetworkFailure +{ + XCTAssertFalse([CodePushErrorUtils isNetworkFailure:nil]); +} + +@end From fc946ef1a6716030e3ef57afe6b6f9b8337e8e88 Mon Sep 17 00:00:00 2001 From: Floyd Kim Date: Wed, 26 Aug 2026 20:21:30 +0900 Subject: [PATCH 06/10] fix(android): stop failing a download the server sent without a length The bytes received were compared against `getContentLength()` whatever it answered, and it answers -1 for a body a server sends with no `Content-Length` - a length no read total can match, so every one of those downloads was refused as if it had arrived short. Nothing had reported it, which fits: the request asks for `identity` encoding and a CDN answers that with a length. It took a server that chose otherwise. A length the server never declared is nothing to check against, so it is no longer checked against. --- .../codepush/react/CodePushUpdateManager.java | 5 ++- .../CodePushUpdateManagerDownloadTest.java | 44 ++++++++++++++++++- 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java b/android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java index 5d624ff1..cee2b515 100644 --- a/android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java +++ b/android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java @@ -378,7 +378,10 @@ ArchiveRestoreResult downloadAndInstallPackage(JSONObject updatePackage, String progressCallback.call(new DownloadProgress(totalBytes, receivedBytes)); } - if (totalBytes != receivedBytes) { + // Only against a length the server declared. `getContentLength()` answers -1 + // for a body sent without one, which no read total matches - so comparing anyway + // would fail every download a server chooses to send that way. + if (totalBytes >= 0 && totalBytes != receivedBytes) { throw new CodePushUnknownException("Received " + receivedBytes + " bytes, expected " + totalBytes); } diff --git a/android/app/src/test/java/com/microsoft/codepush/react/CodePushUpdateManagerDownloadTest.java b/android/app/src/test/java/com/microsoft/codepush/react/CodePushUpdateManagerDownloadTest.java index 7da09ab3..18391e5a 100644 --- a/android/app/src/test/java/com/microsoft/codepush/react/CodePushUpdateManagerDownloadTest.java +++ b/android/app/src/test/java/com/microsoft/codepush/react/CodePushUpdateManagerDownloadTest.java @@ -157,6 +157,18 @@ public void failsTheDownloadWhenTheServerAnswersTheArchiveWithAnErrorStatus() th assertFalse("nothing the server refused reaches the package folder", mPackageFolder.exists()); } + @Test + public void installsAnUpdateTheServerSentWithoutDeclaringItsLength() throws IOException { + // `getContentLength()` answers -1 for a body sent with no `Content-Length`, which no + // read total matches - so a download checked against it anyway could never arrive. + String fullUrl = mServer.serveWithoutContentLength("/full.zip", zipOf(fullArchiveContents())); + + updateManager(applierWriting(TARGET_BUNDLE)).downloadPackage( + fullUpdatePackage(mPackageHash, fullUrl), BUNDLE_FILE_NAME, ignoreProgress()); + + assertInstalledContents(); + } + @Test public void fallsBackToTheFullArchiveWhenApplyingThePatchRunsOutOfMemory() throws IOException { String patchUrl = serve("/patch.zip", zipOf(patchArchiveContents())); @@ -700,7 +712,10 @@ private String serve(String path, byte[] body) { private static class TestArchiveServer { private final ServerSocket mSocket; + private static final long NO_CONTENT_LENGTH = -1; + private final Map mBodies = new HashMap<>(); + private final Map mDeclaredLengths = new HashMap<>(); private final List mRequestedPaths = Collections.synchronizedList(new ArrayList()); TestArchiveServer() throws IOException { @@ -720,6 +735,18 @@ synchronized String serve(String path, byte[] body) { return urlOf(path); } + /** Serves a body under a `Content-Length` of the server's choosing rather than its own. */ + synchronized String serveClaimingLength(String path, byte[] body, long declaredLength) { + mBodies.put(path, body); + mDeclaredLengths.put(path, declaredLength); + return urlOf(path); + } + + /** Serves a body with no `Content-Length` at all, which the client reads until it closes. */ + synchronized String serveWithoutContentLength(String path, byte[] body) { + return serveClaimingLength(path, body, NO_CONTENT_LENGTH); + } + /** The URL of a path this server answers - with a 404, when nothing is served there. */ String urlOf(String path) { return "http://127.0.0.1:" + mSocket.getLocalPort() + path; @@ -771,8 +798,17 @@ private void respond(Socket connection) throws IOException { if (body == null) { response.write(bytes("HTTP/1.1 404 Not Found\r\nContent-Length: 0\r\nConnection: close\r\n\r\n")); } else { - response.write(bytes("HTTP/1.1 200 OK\r\nContent-Length: " + body.length - + "\r\nConnection: close\r\n\r\n")); + Long declaredLength = declaredLengthFor(path); + if (declaredLength == null) { + response.write(bytes("HTTP/1.1 200 OK\r\nContent-Length: " + body.length + + "\r\nConnection: close\r\n\r\n")); + } else if (declaredLength == NO_CONTENT_LENGTH) { + response.write(bytes("HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n")); + } else { + response.write(bytes("HTTP/1.1 200 OK\r\nContent-Length: " + declaredLength + + "\r\nConnection: close\r\n\r\n")); + } + response.write(body); } response.flush(); @@ -787,6 +823,10 @@ private void respond(Socket connection) throws IOException { private synchronized byte[] bodyFor(String path) { return mBodies.get(path); } + + private synchronized Long declaredLengthFor(String path) { + return mDeclaredLengths.get(path); + } } private static byte[] zipOf(Map contents) throws IOException { From dbabd8c48383fe2c2f04fd330442d67a88a35226 Mon Sep 17 00:00:00 2001 From: Floyd Kim Date: Wed, 26 Aug 2026 20:22:29 +0900 Subject: [PATCH 07/10] fix(android): read a download that stopped short as a network failure The bytes stopped arriving partway through a body the server had declared the length of, and the count was the only thing that noticed - the socket raised nothing, so this arrived as an unnamed I/O error and was classified as one. That put it in the same bucket as a full disk, and it cost the patch archives their point: an archive cut off mid-download was read as an archive that could not be used, so the client fell back and asked for the full one - the largest of the three - over the connection that had just stopped delivering. It is a type of its own now, and named a network failure everywhere one is acted on. --- .../codepush/react/CodePushErrorCode.java | 6 +++++- .../CodePushIncompleteDownloadException.java | 17 +++++++++++++++ .../codepush/react/CodePushUpdateManager.java | 2 +- .../codepush/react/CodePushErrorCodeTest.java | 9 ++++++++ .../CodePushUpdateManagerDownloadTest.java | 21 +++++++++++++++++++ 5 files changed, 53 insertions(+), 2 deletions(-) create mode 100644 android/app/src/main/java/com/microsoft/codepush/react/CodePushIncompleteDownloadException.java diff --git a/android/app/src/main/java/com/microsoft/codepush/react/CodePushErrorCode.java b/android/app/src/main/java/com/microsoft/codepush/react/CodePushErrorCode.java index 1dd5022c..151af2af 100644 --- a/android/app/src/main/java/com/microsoft/codepush/react/CodePushErrorCode.java +++ b/android/app/src/main/java/com/microsoft/codepush/react/CodePushErrorCode.java @@ -74,9 +74,13 @@ private static boolean isTransportFailure(Throwable error) { // `SocketException` is the one that covers the reported majority - the connection // reset and the connection aborted an app being backgrounded mid-download leaves // behind - along with the connection that was refused or had no route. + // + // A download that stopped short belongs here too. The socket raised nothing for it, + // so only the byte count says the network dropped the rest of the body. return error instanceof SocketTimeoutException || error instanceof SocketException || error instanceof UnknownHostException - || error instanceof SSLException; + || error instanceof SSLException + || error instanceof CodePushIncompleteDownloadException; } } diff --git a/android/app/src/main/java/com/microsoft/codepush/react/CodePushIncompleteDownloadException.java b/android/app/src/main/java/com/microsoft/codepush/react/CodePushIncompleteDownloadException.java new file mode 100644 index 00000000..37f95c16 --- /dev/null +++ b/android/app/src/main/java/com/microsoft/codepush/react/CodePushIncompleteDownloadException.java @@ -0,0 +1,17 @@ +package com.microsoft.codepush.react; + +import java.io.IOException; + +/** + * The response stopped before it had delivered the length it declared. + * + * A type of its own rather than one more unnamed I/O error, because this is a network + * failure the socket never raised one for: the bytes simply stopped arriving, and only the + * count says so. Everywhere a network failure is acted on has to act on this one too. + */ +public class CodePushIncompleteDownloadException extends IOException { + + public CodePushIncompleteDownloadException(long receivedBytes, long declaredBytes) { + super("Received " + receivedBytes + " bytes, expected " + declaredBytes); + } +} diff --git a/android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java b/android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java index cee2b515..e88bd409 100644 --- a/android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java +++ b/android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java @@ -382,7 +382,7 @@ ArchiveRestoreResult downloadAndInstallPackage(JSONObject updatePackage, String // for a body sent without one, which no read total matches - so comparing anyway // would fail every download a server chooses to send that way. if (totalBytes >= 0 && totalBytes != receivedBytes) { - throw new CodePushUnknownException("Received " + receivedBytes + " bytes, expected " + totalBytes); + throw new CodePushIncompleteDownloadException(receivedBytes, totalBytes); } isZip = ByteBuffer.wrap(header).getInt() == 0x504b0304; diff --git a/android/app/src/test/java/com/microsoft/codepush/react/CodePushErrorCodeTest.java b/android/app/src/test/java/com/microsoft/codepush/react/CodePushErrorCodeTest.java index dbe09942..79b76bca 100644 --- a/android/app/src/test/java/com/microsoft/codepush/react/CodePushErrorCodeTest.java +++ b/android/app/src/test/java/com/microsoft/codepush/react/CodePushErrorCodeTest.java @@ -35,6 +35,15 @@ public void namesAConnectionThatNeverOpenedAsANetworkFailure() { assertEquals(CodePushErrorCode.NETWORK, CodePushErrorCode.of(new SSLHandshakeException("handshake failed"))); } + @Test + public void namesADownloadThatStoppedShortAsANetworkFailure() { + // The socket raised nothing for it, so only the byte count says the body was cut off. + Throwable error = new CodePushIncompleteDownloadException(1024, 4096); + + assertEquals(CodePushErrorCode.NETWORK, CodePushErrorCode.of(error)); + assertTrue(CodePushErrorCode.isNetworkFailure(error)); + } + @Test public void namesAnErrorStatusAsAnHttpFailureRatherThanANetworkOne() { Throwable error = new CodePushHttpException("https://cdn.example.test/full.zip", 503); diff --git a/android/app/src/test/java/com/microsoft/codepush/react/CodePushUpdateManagerDownloadTest.java b/android/app/src/test/java/com/microsoft/codepush/react/CodePushUpdateManagerDownloadTest.java index 18391e5a..94f5d805 100644 --- a/android/app/src/test/java/com/microsoft/codepush/react/CodePushUpdateManagerDownloadTest.java +++ b/android/app/src/test/java/com/microsoft/codepush/react/CodePushUpdateManagerDownloadTest.java @@ -157,6 +157,27 @@ public void failsTheDownloadWhenTheServerAnswersTheArchiveWithAnErrorStatus() th assertFalse("nothing the server refused reaches the package folder", mPackageFolder.exists()); } + @Test + public void givesUpTheDownloadWhenThePatchArchiveArrivesShort() throws IOException { + // The connection carried part of the body and stopped. The full archive is behind + // the same connection and is larger, so there is nothing to fall back to. + byte[] patchArchive = zipOf(patchArchiveContents()); + String patchUrl = mServer.serveClaimingLength("/patch.zip", patchArchive, patchArchive.length + 1024); + String fullUrl = serve("/full.zip", zipOf(fullArchiveContents())); + + try { + updateManager(applierWriting(TARGET_BUNDLE)) + .downloadPackage(updatePackage(fullUrl, patchUrl), BUNDLE_FILE_NAME, ignoreProgress()); + fail("a download that stopped short must not be reported as installed"); + } catch (IOException e) { + assertEquals(CodePushErrorCode.NETWORK, CodePushErrorCode.of(e)); + } + + assertEquals("the full archive is behind the connection that just stopped", + Arrays.asList("/patch.zip"), mServer.requestedPaths()); + assertFalse("nothing that arrived short reaches the package folder", mPackageFolder.exists()); + } + @Test public void installsAnUpdateTheServerSentWithoutDeclaringItsLength() throws IOException { // `getContentLength()` answers -1 for a body sent with no `Content-Length`, which no From 8630c3d0776d8052adeac6b8a39117023c593f27 Mon Sep 17 00:00:00 2001 From: Floyd Kim Date: Wed, 26 Aug 2026 20:23:21 +0900 Subject: [PATCH 08/10] fix(android): settle the download promise whatever the download hits The catch listed `IOException` and this package's unchecked exception, which covered what a download usually fails with and not what it rarely does. Two went past it: a download URL that is not a URL, and an archive naming a path outside the folder it unpacks into. Both ran on the background executor, which has nowhere to report an exception to, so the promise was never settled either way. JS was left waiting on a download that had already stopped, with no timeout of its own to end the wait - the one failure shape worse than a rejection. The download now settles its promise whatever it hits, and what it hit is classified the same way as everything else. --- .../codepush/react/CodePushNativeModule.java | 7 ++++++- .../codepush/react/CodePushErrorCodeTest.java | 11 +++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/android/app/src/main/java/com/microsoft/codepush/react/CodePushNativeModule.java b/android/app/src/main/java/com/microsoft/codepush/react/CodePushNativeModule.java index e3276683..79c4ec20 100644 --- a/android/app/src/main/java/com/microsoft/codepush/react/CodePushNativeModule.java +++ b/android/app/src/main/java/com/microsoft/codepush/react/CodePushNativeModule.java @@ -403,7 +403,12 @@ public void doFrame(long frameTimeNanos) { CodePushUtils.log(e); mSettingsManager.saveFailedUpdate(CodePushUtils.convertReadableToJsonObject(updatePackage)); promise.reject(CodePushErrorCode.of(e), e.getMessage(), e); - } catch (IOException | CodePushUnknownException e) { + } catch (Throwable e) { + // Anything at all, because a promise is waiting on this. A failure that + // leaves this method without settling it leaves JS waiting on a download + // that is no longer running, with nothing to time it out. The narrower + // catch this replaces let two through: a download URL that is not a URL, + // and an archive naming a path outside the folder it unpacks into. CodePushUtils.log(e); promise.reject(CodePushErrorCode.of(e), e.getMessage(), e); } diff --git a/android/app/src/test/java/com/microsoft/codepush/react/CodePushErrorCodeTest.java b/android/app/src/test/java/com/microsoft/codepush/react/CodePushErrorCodeTest.java index 79b76bca..d5cced6f 100644 --- a/android/app/src/test/java/com/microsoft/codepush/react/CodePushErrorCodeTest.java +++ b/android/app/src/test/java/com/microsoft/codepush/react/CodePushErrorCodeTest.java @@ -8,6 +8,7 @@ import java.io.IOException; import java.net.ConnectException; +import java.net.MalformedURLException; import java.net.SocketException; import java.net.SocketTimeoutException; import java.net.UnknownHostException; @@ -68,6 +69,16 @@ public void readsThroughTheWrapperADownloadCatchesItsFailuresIn() { assertTrue(CodePushErrorCode.isNetworkFailure(wrapped)); } + @Test + public void namesTheFailuresThatUsedToEscapeTheDownloadUncaught() { + // Neither is an `IOException`, so the download's old catch let them past and left + // the promise waiting on it unsettled. They are classified like anything else now. + assertEquals(CodePushErrorCode.UNKNOWN, + CodePushErrorCode.of(new CodePushMalformedDataException("not a url", new MalformedURLException()))); + assertEquals(CodePushErrorCode.UNKNOWN, + CodePushErrorCode.of(new IllegalStateException("File is outside extraction target directory."))); + } + @Test public void leavesAFailureNothingHasAWordForUnnamed() { Throwable error = new IOException("the disk is full"); From 5ed0f6035f7cefaa8f2ff2d79597e8b830aaa896 Mon Sep 17 00:00:00 2001 From: Floyd Kim Date: Wed, 26 Aug 2026 21:07:05 +0900 Subject: [PATCH 09/10] fix: try the patch archive when the server does not serve the asset diff A diff that failed before its bundle was restored was read as a failure of the bundle patch, which every archive of a release carries byte for byte - so the patch archive was passed over as one that could only fail the same way. That holds for an applier that refused the patch. It does not hold for a server that answered the diff's URL with a status: the patch archive is at a URL of its own, and diffs are published one per recent version, so they are the first thing a retention policy clears out while the patch archive stays. So a release whose diff had been cleaned up downloaded the largest archive it has, with a patch archive sitting untried behind a 404 that said nothing about it. The two platforms tell the cases apart the same way now, from whether the archive was answered with a status. The iOS suite serves its archives as files, which have no status to answer with, so the status case is covered by the Android suite and by the error utils tests. --- .../codepush/react/ArchiveAttemptLog.java | 23 +++++-- .../codepush/react/CodePushUpdateManager.java | 17 ++--- .../CodePushUpdateManagerDownloadTest.java | 62 ++++++++++++------- docs/diff-updates.ko.md | 2 +- docs/diff-updates.md | 14 +++-- ios/CodePush/CodePush.h | 2 + ios/CodePush/CodePushDownloadHandler.m | 3 +- ios/CodePush/CodePushErrorUtils.m | 24 +++++++ ios/CodePush/CodePushPackage.m | 17 +++-- ios/CodePushTests/CodePushErrorUtilsTests.m | 13 ++++ ios/CodePushTests/CodePushPackageTests.m | 12 ++-- 11 files changed, 136 insertions(+), 53 deletions(-) diff --git a/android/app/src/main/java/com/microsoft/codepush/react/ArchiveAttemptLog.java b/android/app/src/main/java/com/microsoft/codepush/react/ArchiveAttemptLog.java index 56d0569a..48a4843f 100644 --- a/android/app/src/main/java/com/microsoft/codepush/react/ArchiveAttemptLog.java +++ b/android/app/src/main/java/com/microsoft/codepush/react/ArchiveAttemptLog.java @@ -43,6 +43,9 @@ private static class Attempt { String fallbackReason; long durationMs = -1; + /** Whether the server answered this archive's URL with a status instead of the archive. */ + boolean wasNotServed; + Attempt(String archive) { this.archive = archive; } @@ -72,14 +75,25 @@ void recordBundleRestored(long applyDurationMs) { /** * Whether the current archive's attempt got as far as restoring the bundle. A failure - * after that point is on the asset side of the archive - and every archive carries the - * same bundle patch, so this is what decides whether the patch archive is worth trying - * after a failed diff. + * after that point is on the asset side of the archive, which the archives do not share - + * unlike the bundle patch, which they carry byte for byte. */ boolean currentAttemptRestoredBundle() { return current().applyDurationMs >= 0; } + /** + * Whether the current archive never arrived, because the server answered its URL with a + * status rather than with the archive. + * + * A verdict on one URL, and the archives are at URLs of their own: a release whose diff + * has been cleaned up still has its patch archive. This is the one failure before the + * bundle is restored that says nothing about the archives left to try. + */ + boolean currentAttemptWasNotServed() { + return current().wasNotServed; + } + /** * The attempt at the current archive ended in one of the reasons the appliers report. * @@ -104,7 +118,8 @@ void recordFallback(String failureReason) { * here would put a value on the wire that no platform reports, so the fallback is * reported without a reason. */ - void recordFallbackAfterError() { + void recordFallbackAfterError(Throwable error) { + current().wasNotServed = CodePushErrorCode.HTTP.equals(CodePushErrorCode.of(error)); recordFallback(currentAttemptRestoredBundle() ? ArchiveRestoreResult.REASON_PACKAGE_VERIFICATION_FAILED : null); } diff --git a/android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java b/android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java index e88bd409..8835d8e8 100644 --- a/android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java +++ b/android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java @@ -180,12 +180,15 @@ public JSONObject downloadPackage(JSONObject updatePackage, String expectedBundl return patchAttempt.result(); } - // A diff that failed before its bundle was restored failed in the bundle patch, - // which the patch archive carries byte for byte - it would fail the same way, - // and trying it would only put a second doomed download in front of the full - // one. A failure after the restore is on the asset side, which the patch - // archive does not share. - patchArchiveWorthTrying = patchArchiveWorthTrying && patchAttempt.currentAttemptRestoredBundle(); + // The patch archive is worth trying when nothing about how the diff failed + // implicates it. A diff that failed after restoring its bundle failed on its + // asset side, which the patch archive does not share; a diff the server never + // served is a verdict on one URL, and the patch archive is at another. Anything + // else failed in the bundle patch both archives carry byte for byte, so the + // patch archive would fail the same way and trying it would only put a second + // doomed download in front of the full one. + patchArchiveWorthTrying = patchArchiveWorthTrying + && (patchAttempt.currentAttemptRestoredBundle() || patchAttempt.currentAttemptWasNotServed()); } if (patchArchiveWorthTrying) { @@ -267,7 +270,7 @@ private boolean tryDownloadArchivePackage(JSONObject updatePackage, String expec // running out of it is a failure this has to absorb like any other: by the time // it lands here the arrays are unreachable, and the full archive is downloaded // to disk in chunks rather than held. - patchAttempt.recordFallbackAfterError(); + patchAttempt.recordFallbackAfterError(e); CodePushUtils.log(e); CodePushUtils.log("The " + patchAttempt.currentArchive() + " archive could not be applied. Falling back."); diff --git a/android/app/src/test/java/com/microsoft/codepush/react/CodePushUpdateManagerDownloadTest.java b/android/app/src/test/java/com/microsoft/codepush/react/CodePushUpdateManagerDownloadTest.java index 94f5d805..54b3ca6b 100644 --- a/android/app/src/test/java/com/microsoft/codepush/react/CodePushUpdateManagerDownloadTest.java +++ b/android/app/src/test/java/com/microsoft/codepush/react/CodePushUpdateManagerDownloadTest.java @@ -359,10 +359,11 @@ public void fallsBackToThePatchArchiveWhenTheAssetDiffManifestDoesNotNameTheFile } @Test - public void skipsThePatchArchiveWhenTheAssetDiffCannotBeDownloaded() throws IOException { - // A diff that never arrived left no verdict at all: nothing says the patch archive - // is any better off, and the full download is the one that cannot fail - so a - // client is never walked through two doomed downloads on its way there. + public void triesThePatchArchiveWhenTheServerDoesNotServeTheAssetDiff() throws IOException { + // A 404 is a verdict on the URL it was asked of. Diffs are published one per recent + // version and are the first thing a retention policy clears out, while the patch + // archive at its own URL stays - so nothing about a diff that is gone says the patch + // archive is. Map updateContents = assetDiffTargetContents(); String updateHash = packageHashOf(updateContents); String diffUrl = mServer.urlOf("/missing-diff.zip"); @@ -372,9 +373,29 @@ public void skipsThePatchArchiveWhenTheAssetDiffCannotBeDownloaded() throws IOEx JSONObject patchResult = updateManager(applierWriting(TARGET_BUNDLE)).downloadPackage( updatePackageWithAssetDiff(updateHash, fullUrl, patchUrl, diffUrl), BUNDLE_FILE_NAME, ignoreProgress()); - assertEquals(Arrays.asList("/missing-diff.zip", "/full.zip"), mServer.requestedPaths()); - assertFallbackResult(patchResult, null); - assertEquals("asset-diff", patchResult.optString("archive", null)); + assertEquals("the full archive is not downloaded when the patch archive installs", + Arrays.asList("/missing-diff.zip", "/patch.zip"), mServer.requestedPaths()); + assertEquals("applied", patchResult.optString("status", null)); + assertEquals("binary-patch", patchResult.optString("archive", null)); + assertEquals(2, patchResult.optJSONArray("attempts").length()); + assertInstalledContents(updateHash, updateContents); + } + + @Test + public void skipsThePatchArchiveWhenTheAssetDiffFailsInItsBundlePatch() throws IOException { + // Both archives carry that patch byte for byte, so an applier that refused it here + // would refuse it there - and trying it would put a second doomed download in front + // of the full one. + Map updateContents = assetDiffTargetContents(); + String updateHash = packageHashOf(updateContents); + String diffUrl = serve("/diff.zip", zipOf(assetDiffArchiveContents(DROPPED_ASSET_PATH))); + String patchUrl = serve("/patch.zip", zipOf(patchArchiveContentsForAssetDiffTarget())); + String fullUrl = serve("/full.zip", zipOf(updateContents)); + + JSONObject patchResult = updateManager(applierRefusingThePatch()).downloadPackage( + updatePackageWithAssetDiff(updateHash, fullUrl, patchUrl, diffUrl), BUNDLE_FILE_NAME, ignoreProgress()); + + assertEquals(Arrays.asList("/diff.zip", "/full.zip"), mServer.requestedPaths()); assertEquals(1, patchResult.optJSONArray("attempts").length()); assertInstalledContents(updateHash, updateContents); } @@ -401,23 +422,6 @@ public void givesUpTheDownloadWhenTheNetworkCannotCarryTheAssetDiff() throws IOE assertFalse("nothing that never arrived reaches the package folder", mPackageFolder.exists()); } - @Test - public void stillFallsBackWhenTheServerRefusesTheAssetDiffWithAnErrorStatus() throws IOException { - // A server that answered is not a network that failed: the connection worked, so the - // archives behind it are worth asking for. - Map updateContents = assetDiffTargetContents(); - String updateHash = packageHashOf(updateContents); - String diffUrl = mServer.urlOf("/missing-diff.zip"); - String patchUrl = serve("/patch.zip", zipOf(patchArchiveContentsForAssetDiffTarget())); - String fullUrl = serve("/full.zip", zipOf(updateContents)); - - JSONObject patchResult = updateManager(applierWriting(TARGET_BUNDLE)).downloadPackage( - updatePackageWithAssetDiff(updateHash, fullUrl, patchUrl, diffUrl), BUNDLE_FILE_NAME, ignoreProgress()); - - assertEquals(Arrays.asList("/missing-diff.zip", "/full.zip"), mServer.requestedPaths()); - assertInstalledContents(updateHash, updateContents); - assertFallbackResult(patchResult, null); - } /** A loopback port that is opened only to be closed, so connecting to it is refused. */ private static int portNothingListensOn() throws IOException { @@ -600,6 +604,16 @@ public byte[] readBaseBundle(String bundleFileName) { return new CodePushUpdateManager(mDocumentsDirectory, binaryPatch); } + /** An applier that refuses the bundle patch, which every archive of a release carries. */ + private static CodePushBinaryPatch.PatchApplier applierRefusingThePatch() { + return new CodePushBinaryPatch.PatchApplier() { + @Override + public int apply(byte[] base, byte[] patch, String outputPath, long expectedTargetSize) { + return RESULT_APPLY_FAILED; + } + }; + } + private static CodePushBinaryPatch.PatchApplier applierWriting(final byte[] restoredBundle) { return new CodePushBinaryPatch.PatchApplier() { @Override diff --git a/docs/diff-updates.ko.md b/docs/diff-updates.ko.md index 50f9a768..20b74992 100644 --- a/docs/diff-updates.ko.md +++ b/docs/diff-updates.ko.md @@ -89,7 +89,7 @@ binary patch로 배포한 릴리스는 **asset diff 아카이브**도 함께 담 **모든 클라이언트가 셋을 다 시도하지는 않습니다.** 쓸 수 있는 asset diff가 없는 클라이언트는 binary patch에서 시작합니다. 앱 바이너리의 번들을 실행 중이거나, 새 릴리스가 diff를 만들지 않은 업데이트를 실행 중인 경우입니다. -**asset diff가 실패해도 항상 binary patch로 넘어가지는 않습니다.** diff가 asset 영역에서 실패했을 때만 넘어갑니다. 설치된 업데이트와 병합하지 못했거나(`asset_merge_failed`), 병합된 내용이 package hash 검증에 실패한 경우(`package_verification_failed`)입니다. 그 밖의 실패는 두 아카이브가 공유하는 bundle patch에서 일어나므로 binary patch도 같은 방식으로 실패합니다. 이때 클라이언트는 곧바로 full 아카이브를 내려받습니다. +**asset diff가 실패해도 항상 binary patch로 넘어가지는 않습니다.** diff가 asset 영역에서 실패했다면 넘어갑니다. 설치된 업데이트와 병합하지 못했거나(`asset_merge_failed`), 병합된 내용이 package hash 검증에 실패한 경우(`package_verification_failed`)입니다. 서버가 diff의 URL에 400 이상으로 응답했을 때도 넘어갑니다. 두 아카이브는 서로 다른 URL에 있으니, diff를 받지 못했다고 해서 binary patch도 받지 못하리라 단정할 수 없습니다. 그 밖의 실패는 두 아카이브가 byte 단위로 똑같이 담고 있는 bundle patch에서 일어납니다. binary patch도 같은 방식으로 실패하므로 곧바로 full 아카이브를 내려받습니다. **연결이 실패하면 다운로드가 거기서 멈춥니다.** 다음 아카이브도 같은 네트워크 뒤에 있고 full 아카이브는 셋 중 가장 큽니다. 이어서 시도해 봐야 더 느리게 실패할 뿐이므로, 클라이언트는 연결 오류를 그대로 알립니다. 서버가 응답한 경우는 다릅니다. 한 아카이브의 404는 다음 아카이브를 건너뛸 이유가 되지 않습니다. diff --git a/docs/diff-updates.md b/docs/diff-updates.md index 11a4573b..727d9b10 100644 --- a/docs/diff-updates.md +++ b/docs/diff-updates.md @@ -124,12 +124,14 @@ There are three exceptions to this order. bundle in the app binary, or an update this release was not diffed against - starts at the binary patch. -**A failed asset diff does not always reach the binary patch.** It moves on to that archive -only when the diff failed on its asset side: the merge with the installed update failing -(`asset_merge_failed`), or the merged contents failing the package hash -(`package_verification_failed`). Anything else the diff fails on lives in the bundle patch -both archives carry, so the binary patch would fail there the same way and the client goes -straight to the full archive. +**A failed asset diff does not always reach the binary patch.** It does when the diff failed +on its asset side: the merge with the installed update failing (`asset_merge_failed`), or the +merged contents failing the package hash (`package_verification_failed`). It also does when +the server answered the diff's URL with a status of 400 or above. The two archives are at +URLs of their own, so a diff that could not be fetched is no reason to expect the binary +patch cannot be either. Anything else the diff fails on lives in the bundle patch both +archives carry byte for byte. The binary patch would fail there the same way, so the client +goes straight to the full archive. **A failed connection stops the download.** The next archive is behind the same network, and the full one is the largest of the three, so trying it would only fail again more diff --git a/ios/CodePush/CodePush.h b/ios/CodePush/CodePush.h index 252477a7..06a69ed1 100644 --- a/ios/CodePush/CodePush.h +++ b/ios/CodePush/CodePush.h @@ -157,8 +157,10 @@ failCallback:(void (^)(NSError *err))failCallback; @interface CodePushErrorUtils : NSObject + (NSError *)errorWithMessage:(NSString *)errorMessage; ++ (NSError *)errorWithMessage:(NSString *)errorMessage httpStatusCode:(NSInteger)statusCode; + (BOOL)isCodePushError:(NSError *)error; + (BOOL)isNetworkFailure:(NSError *)error; ++ (BOOL)isHttpStatusError:(NSError *)error; @end diff --git a/ios/CodePush/CodePushDownloadHandler.m b/ios/CodePush/CodePushDownloadHandler.m index d52e9e2e..4173f8c1 100644 --- a/ios/CodePush/CodePushDownloadHandler.m +++ b/ios/CodePush/CodePushDownloadHandler.m @@ -54,7 +54,8 @@ - (void)connection:(NSURLConnection *)connection didReceiveResponse:(NSURLRespon if (statusCode >= 400) { [self.outputFileStream close]; [connection cancel]; - NSError *err = [CodePushErrorUtils errorWithMessage:[NSString stringWithFormat: @"Received %ld response from %@", (long)statusCode, self.downloadUrl]]; + NSError *err = [CodePushErrorUtils errorWithMessage:[NSString stringWithFormat: @"Received %ld response from %@", (long)statusCode, self.downloadUrl] + httpStatusCode:statusCode]; self.failCallback(err); return; } diff --git a/ios/CodePush/CodePushErrorUtils.m b/ios/CodePush/CodePushErrorUtils.m index 49af8cba..0bb37674 100644 --- a/ios/CodePush/CodePushErrorUtils.m +++ b/ios/CodePush/CodePushErrorUtils.m @@ -4,6 +4,13 @@ @implementation CodePushErrorUtils static NSString *const CodePushErrorDomain = @"CodePushError"; static const int CodePushErrorCode = -1; +/* + * The status a server answered a download with, on the error raised for it. + * + * Carried in the user info rather than in the error code, because the code is what JS reads + * an error by and every error of this domain has always been -1 there. + */ +static NSString *const CodePushHttpStatusCodeKey = @"CodePushHttpStatusCode"; + (NSError *)errorWithMessage:(NSString *)errorMessage { @@ -12,11 +19,28 @@ + (NSError *)errorWithMessage:(NSString *)errorMessage userInfo:@{ NSLocalizedDescriptionKey: NSLocalizedString(errorMessage, nil) }]; } ++ (NSError *)errorWithMessage:(NSString *)errorMessage httpStatusCode:(NSInteger)statusCode +{ + return [NSError errorWithDomain:CodePushErrorDomain + code:CodePushErrorCode + userInfo:@{ NSLocalizedDescriptionKey: NSLocalizedString(errorMessage, nil), + CodePushHttpStatusCodeKey: @(statusCode) }]; +} + + (BOOL)isCodePushError:(NSError *)err { return err != nil && [CodePushErrorDomain isEqualToString:err.domain]; } +/* + * Whether the download failed because the server answered it with a status rather than with + * a body to install. + */ ++ (BOOL)isHttpStatusError:(NSError *)err +{ + return [self isCodePushError:err] && err.userInfo[CodePushHttpStatusCodeKey] != nil; +} + /* * Whether the request failed because the network did not carry it. * diff --git a/ios/CodePush/CodePushPackage.m b/ios/CodePush/CodePushPackage.m index eba94811..bd37d581 100644 --- a/ios/CodePush/CodePushPackage.m +++ b/ios/CodePush/CodePushPackage.m @@ -116,11 +116,12 @@ + (void)downloadPackage:(NSDictionary *)updatePackage /* * Tries the first archive of the queue, and decides what a failure of it means for the * rest. A failure after the bundle was restored is on the asset side of the archive, so - * the next archive - which does not share it - is worth trying. A failure before that - * point is in the bundle patch every archive carries byte for byte, or is something no - * verdict exists for, and either way the remaining archives are passed over: they could - * only fail the same way, and trying them would put more doomed downloads in front of the - * full one. + * the next archive - which does not share it - is worth trying, and so is one the server + * never served, which is a verdict on one URL rather than on the archives at the others. + * A failure between those is in the bundle patch every archive carries byte for byte, or + * is something no verdict exists for, and either way the remaining archives are passed + * over: they could only fail the same way, and trying them would put more doomed downloads + * in front of the full one. * * Every verdict on an archive ends with the update installed - by an archive of the queue * or by the full download behind it - so no verdict reaches the caller as an error, and the @@ -144,6 +145,9 @@ + (void)tryNextArchive:(NSArray *)archivesToTry // Set once the applier has restored the bundle, which is also what tells a failure // that follows apart from one that came before. __block NSNumber *applyDurationMs = nil; + // Set when the server answered this archive's URL with a status instead of the archive, + // which is a verdict on that URL and not on the archives at the others. + __block BOOL archiveWasNotServed = NO; void (^giveUpAttempt)(NSString *failureReason) = ^(NSString *failureReason) { [self deleteBinaryPatchFolder]; @@ -152,7 +156,7 @@ + (void)tryNextArchive:(NSArray *)archivesToTry applyDurationMs:applyDurationMs attemptStartTime:attemptStartTime]]; - if (applyDurationMs != nil && [remainingArchives count] > 0) { + if ((applyDurationMs != nil || archiveWasNotServed) && [remainingArchives count] > 0) { [self tryNextArchive:remainingArchives attemptsSoFar:attempts firstAttemptStartTime:firstAttemptStartTime @@ -214,6 +218,7 @@ + (void)tryNextArchive:(NSArray *)archivesToTry return; } + archiveWasNotServed = [CodePushErrorUtils isHttpStatusError:err]; CPLog(@"The %@ archive could not be applied (%@). Falling back.", archive, err.localizedDescription); // An error raised after the bundle was restored is the restored // update failing the checks every update passes before it is diff --git a/ios/CodePushTests/CodePushErrorUtilsTests.m b/ios/CodePushTests/CodePushErrorUtilsTests.m index ba3baa67..44ea03ba 100644 --- a/ios/CodePushTests/CodePushErrorUtilsTests.m +++ b/ios/CodePushTests/CodePushErrorUtilsTests.m @@ -43,6 +43,19 @@ - (void)testDoesNotNameAnErrorStatusAsANetworkFailure XCTAssertFalse([CodePushErrorUtils isNetworkFailure:error]); } +- (void)testNamesAnErrorStatusApartFromTheOtherErrorsCodePushRaises +{ + NSError *status = [CodePushErrorUtils errorWithMessage:@"Received 404 response from https://cdn.example.test/diff.zip" + httpStatusCode:404]; + NSError *other = [CodePushErrorUtils errorWithMessage:@"Received empty response from https://cdn.example.test/diff.zip"]; + + XCTAssertTrue([CodePushErrorUtils isHttpStatusError:status]); + XCTAssertFalse([CodePushErrorUtils isHttpStatusError:other]); + XCTAssertFalse([CodePushErrorUtils isHttpStatusError:[NSError errorWithDomain:NSURLErrorDomain + code:NSURLErrorTimedOut + userInfo:nil]]); +} + - (void)testDoesNotNameANilErrorAsANetworkFailure { XCTAssertFalse([CodePushErrorUtils isNetworkFailure:nil]); diff --git a/ios/CodePushTests/CodePushPackageTests.m b/ios/CodePushTests/CodePushPackageTests.m index 2dca9ed3..d19521e5 100644 --- a/ios/CodePushTests/CodePushPackageTests.m +++ b/ios/CodePushTests/CodePushPackageTests.m @@ -632,10 +632,14 @@ - (void)testMergesAnAssetDiffWhoseManifestDeletesNothing { [self assertInstalledContentsOf:packageHash matchStaging:updateStaging]; } -- (void)testSkipsThePatchArchiveWhenTheAssetDiffCannotBeDownloaded { - // A diff that never arrived left no verdict at all: nothing says the patch archive is - // any better off, and the full download is the one that cannot fail - so a client is - // never walked through two doomed downloads on its way there. +- (void)testSkipsThePatchArchiveWhenTheAssetDiffUrlAnswersWithNoStatusAtAll { + // A URL that answers with nothing - no archive and no status to read it by - left no + // verdict of any kind, and the full download is the one that cannot fail, so a client is + // never walked through two doomed downloads on its way there. A server that answers a + // status is the other case: that is a verdict on one URL and the patch archive is at + // another, so it is tried. The archives here are served as files, which have no status + // to answer with, so that case is covered by the Android suite and by + // CodePushErrorUtilsTests rather than here. [self installPackageWithContents:[self stageInstalledArchiveContents]]; NSString *updateStaging = [self stageAssetDiffTargetContents]; NSString *packageHash = CPTestFolderHash(updateStaging); From ea52baf10b5ed81664cb302fa759f2979fb43001 Mon Sep 17 00:00:00 2001 From: Floyd Kim Date: Wed, 26 Aug 2026 21:43:33 +0900 Subject: [PATCH 10/10] test: cover a diff the server does not serve end to end The client now keeps the patch archive when the diff's URL is answered with a status, and iOS had nothing exercising it: the package tests serve their archives as files, which have no status to answer with, so the download handler's whole error-status path has never been covered there. The mock server of the E2E suite serves over HTTP and already 404s a path it holds nothing for, so the scenario is a release published whole and then left with its diff archive deleted - the shape of one whose diff a retention policy cleared out while the archives at the other URLs stayed. What it pins is the pair the other assertions cannot tell apart on their own: the archives the app asked for, in order, and the reasons its own callback reported for them. --- e2e/helpers/asset-diff-fixtures.ts | 17 +++++++++++++++++ e2e/helpers/asset-diff-phase.ts | 15 +++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/e2e/helpers/asset-diff-fixtures.ts b/e2e/helpers/asset-diff-fixtures.ts index eec3d36e..e0c66c57 100644 --- a/e2e/helpers/asset-diff-fixtures.ts +++ b/e2e/helpers/asset-diff-fixtures.ts @@ -168,3 +168,20 @@ export function dropDiffArchiveManifestDeletions(archivePath: string): void { fs.writeFileSync(manifestPath, JSON.stringify(manifest)); }); } + +/** + * Deletes the published diff archive, leaving the release that offers it untouched. + * + * The release history still names the diff's URL, so the client asks for it and the + * server answers 404 - the shape of a release whose diff has been cleaned up while the + * archives at the other URLs are still there. Nothing is corrupted and nothing is + * applied: what this isolates is a client deciding what a status answered at one URL says + * about the archives at the others. + */ +export function removeDiffArchive(archivePath: string): void { + if (!fs.existsSync(archivePath)) { + throw new Error(`There is no diff archive at "${archivePath}" to remove`); + } + + fs.rmSync(archivePath); +} diff --git a/e2e/helpers/asset-diff-phase.ts b/e2e/helpers/asset-diff-phase.ts index 0ee25da2..30220440 100644 --- a/e2e/helpers/asset-diff-phase.ts +++ b/e2e/helpers/asset-diff-phase.ts @@ -20,6 +20,7 @@ import { assertReleaseOffersDiff, corruptDiffArchiveAsset, dropDiffArchiveManifestDeletions, + removeDiffArchive, } from "./asset-diff-fixtures"; import { assertReleaseOffersPatch, @@ -192,6 +193,20 @@ export async function runAssetDiffPhase(context: AssetDiffPhaseContext): Promise expectedArchiveResult: "fallback:asset-diff:asset-diff=target_verification_failed", }); + // A diff the server does not serve is not a verdict on the archives it does. Diffs are + // published one per recent version and are the first thing a retention policy clears + // out, while the patch archive at its own URL stays - so a 404 on the diff must not cost + // the patch archive its try. This is the one failure before the bundle is restored that + // still reaches the patch archive. + await runDiffScenario({ + name: "diff the server does not serve falls back to the patch archive", + baseVersion: "1.4.9", + updateVersion: "1.4.10", + breakDiff: () => removeDiffArchive(findAssetDiffArchive(platform, releaseIdentifier)), + expectedDownloads: ["asset-diff", "binary-patch"], + expectedArchiveResult: "applied:binary-patch:asset-diff=no-verdict:binary-patch=applied", + }); + // A manifest that names no files to delete is not one with nothing to delete - the CLI // writes the key on every release, an empty list included. Merging past its absence would // keep the asset the update dropped and install contents the release never published, so