From a3cc2eea81794a0e073517223de8916f934d3196 Mon Sep 17 00:00:00 2001 From: Christoph Purrer Date: Thu, 6 Aug 2026 15:32:12 -0700 Subject: [PATCH] Retry the packager status probe, and always invoke its callback (#57852) Summary: Changelog: [Internal] Differential Revision: D115073900 --- .../react/devsupport/PackagerStatusCheck.kt | 118 +++++++----- .../devsupport/PackagerStatusCheckTest.kt | 169 ++++++++++++++++++ 2 files changed, 247 insertions(+), 40 deletions(-) create mode 100644 packages/react-native/ReactAndroid/src/test/java/com/facebook/react/devsupport/PackagerStatusCheckTest.kt diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/PackagerStatusCheck.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/PackagerStatusCheck.kt index d39b855518b..b2e5d2df398 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/PackagerStatusCheck.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/PackagerStatusCheck.kt @@ -30,56 +30,94 @@ internal class PackagerStatusCheck(private val client: OkHttpClient) { val statusURL = createPackagerStatusURL(host) val request = Request.Builder().url(statusURL).build() - client - .newCall(request) - .enqueue( - object : Callback { - override fun onFailure(call: Call, e: IOException) { - FLog.w( - ReactConstants.TAG, - "The packager does not seem to be running as we got an IOException requesting its status: ${e.message}", - ) - callback.onPackagerStatusFetched(false) - } + // A metro server reached over a slow link can exceed the client's connect timeout, or answer + // 5xx while it is still starting up. A single-shot probe reports those as "no dev server", + // which silently boots the bundle packaged in the app instead. Retry transport-level failures a + // bounded number of times before giving up. Retries are re-enqueued rather than delayed, + // because this client's dispatcher is shared with the bundle download and the packager + // websockets and must not be blocked. + fun attempt(attemptsLeft: Int) { + // Only for failures that a later attempt could plausibly resolve. + fun retryOrFail() { + if (attemptsLeft > 1) { + attempt(attemptsLeft - 1) + } else { + callback.onPackagerStatusFetched(false) + } + } - override fun onResponse(call: Call, response: Response) { - if (!response.isSuccessful) { - FLog.e( + client + .newCall(request) + .enqueue( + object : Callback { + override fun onFailure(call: Call, e: IOException) { + FLog.w( ReactConstants.TAG, - "Got non-success http code from packager when requesting status: ${response.code()}", + "The packager does not seem to be running as we got an IOException requesting its status from $statusURL: ${e.message}", ) - callback.onPackagerStatusFetched(false) - return + retryOrFail() } - val body = response.body() - if (body == null) { - FLog.e( - ReactConstants.TAG, - "Got null body response from packager when requesting status", - ) - callback.onPackagerStatusFetched(false) - return - } - val bodyString = - body.string() // cannot call body.string() twice, stored it into variable. - // https://github.com/square/okhttp/issues/1240#issuecomment-68142603 - if (PACKAGER_OK_STATUS != bodyString) { - FLog.e( - ReactConstants.TAG, - "Got unexpected response from packager when requesting status: $bodyString", - ) - callback.onPackagerStatusFetched(false) - return + + override fun onResponse(call: Call, response: Response) { + if (!response.isSuccessful) { + FLog.e( + ReactConstants.TAG, + "Got non-success http code from packager at $statusURL when requesting status: ${response.code()}", + ) + // 5xx can mean the dev server is still coming up; a 4xx will not change. + if (response.code() >= 500) { + retryOrFail() + } else { + callback.onPackagerStatusFetched(false) + } + return + } + val body = response.body() + if (body == null) { + FLog.e( + ReactConstants.TAG, + "Got null body response from packager at $statusURL when requesting status", + ) + callback.onPackagerStatusFetched(false) + return + } + // Reading the body can still fail mid-stream (e.g. the connection drops after + // the headers). OkHttp only logs a throw from onResponse, so without catching it + // here the callback would never fire and bundle loading would wait forever. + val bodyString = + try { + body.string() // cannot call body.string() twice, stored it into variable. + // https://github.com/square/okhttp/issues/1240#issuecomment-68142603 + } catch (e: IOException) { + FLog.w( + ReactConstants.TAG, + "Failed to read the packager status response from $statusURL: ${e.message}", + ) + retryOrFail() + return + } + if (PACKAGER_OK_STATUS != bodyString) { + // Something other than Metro answered; retrying will not change the answer. + FLog.e( + ReactConstants.TAG, + "Got unexpected response from packager at $statusURL when requesting status: $bodyString", + ) + callback.onPackagerStatusFetched(false) + return + } + callback.onPackagerStatusFetched(true) } - callback.onPackagerStatusFetched(true) - } - }, - ) + }, + ) + } + + attempt(MAX_STATUS_ATTEMPTS) } private companion object { private const val PACKAGER_OK_STATUS = "packager-status:running" private const val PACKAGER_STATUS_URL_TEMPLATE = "%s://%s/status" + private const val MAX_STATUS_ATTEMPTS = 3 private fun createPackagerStatusURL(host: String): String = String.format( diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/devsupport/PackagerStatusCheckTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/devsupport/PackagerStatusCheckTest.kt new file mode 100644 index 00000000000..4aa531a03ee --- /dev/null +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/devsupport/PackagerStatusCheckTest.kt @@ -0,0 +1,169 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +@file:Suppress("DEPRECATION_ERROR") // Conflicting okhttp versions + +package com.facebook.react.devsupport + +import java.io.IOException +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicBoolean +import java.util.concurrent.atomic.AtomicInteger +import okhttp3.Interceptor +import okhttp3.MediaType +import okhttp3.OkHttpClient +import okhttp3.Protocol +import okhttp3.Response +import okhttp3.ResponseBody +import org.assertj.core.api.Assertions.assertThat +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +// Robolectric provides a real android.util.Log; without it FLog throws "Stub!" on the OkHttp +// dispatcher thread and the status callback never runs. +@RunWith(RobolectricTestRunner::class) +class PackagerStatusCheckTest { + + /** One scripted answer to a single probe attempt. */ + private sealed interface Step { + /** Fail at the transport layer, as a down tunnel or refused connection would. */ + object TransportFailure : Step + + /** Answer with the given HTTP status code and (optional) body. */ + data class HttpResponse(val code: Int, val body: String = "") : Step + } + + /** + * Answers every call from [steps] in order, entirely in-process. CI runs these tests in a + * network-restricted sandbox where a real loopback server (MockWebServer) is unreachable, so + * responses are synthesized by an OkHttp application interceptor that never touches a socket. + * [requestCount] records how many probe attempts PackagerStatusCheck issued. + */ + private class ScriptedPackager(private val steps: List) : Interceptor { + val requestCount = AtomicInteger(0) + + override fun intercept(chain: Interceptor.Chain): Response { + val step = steps[requestCount.getAndIncrement()] + return when (step) { + is Step.TransportFailure -> throw IOException("simulated connection failure") + is Step.HttpResponse -> + Response.Builder() + .request(chain.request()) + .protocol(Protocol.HTTP_1_1) + .code(step.code) + .message(if (step.code in 200..299) "OK" else "Error") + .body(ResponseBody.create(TEXT_PLAIN, step.body)) + .build() + } + } + } + + /** Drives the check against [steps] and returns the reported status plus the attempt count. */ + private fun check(vararg steps: Step): Pair { + val packager = ScriptedPackager(steps.toList()) + // Disable OkHttp's own connection-failure retries so the count reflects only what + // PackagerStatusCheck itself re-enqueued. + val client = + OkHttpClient.Builder().retryOnConnectionFailure(false).addInterceptor(packager).build() + val status = AtomicBoolean(false) + val done = CountDownLatch(1) + + PackagerStatusCheck(client).run(PACKAGER_HOST) { packagerIsRunning -> + status.set(packagerIsRunning) + done.countDown() + } + + assertThat(done.await(30, TimeUnit.SECONDS)) + .describedAs("PackagerStatusCheck never invoked its callback") + .isTrue() + return status.get() to packager.requestCount.get() + } + + private fun runningResponse(): Step = Step.HttpResponse(200, PACKAGER_RUNNING_BODY) + + @Test + fun reportsRunningOnFirstSuccessfulResponse() { + val (isRunning, requestCount) = check(runningResponse()) + + assertThat(isRunning).isTrue() + assertThat(requestCount).isEqualTo(1) + } + + @Test + fun retriesTransportFailuresBeforeGivingUp() { + // Every attempt fails to connect - the transport-level failure a dev server behind a down + // tunnel produces. The callback must still be invoked exactly once, reporting "not running". + val packager = + ScriptedPackager( + listOf(Step.TransportFailure, Step.TransportFailure, Step.TransportFailure) + ) + val client = + OkHttpClient.Builder().retryOnConnectionFailure(false).addInterceptor(packager).build() + val callbackCount = AtomicInteger(0) + val status = AtomicBoolean(true) + val done = CountDownLatch(1) + + PackagerStatusCheck(client).run(PACKAGER_HOST) { packagerIsRunning -> + callbackCount.incrementAndGet() + status.set(packagerIsRunning) + done.countDown() + } + + assertThat(done.await(30, TimeUnit.SECONDS)) + .describedAs("PackagerStatusCheck never invoked its callback") + .isTrue() + assertThat(status.get()).isFalse() + assertThat(callbackCount.get()).isEqualTo(1) + assertThat(packager.requestCount.get()).isEqualTo(3) + } + + @Test + fun retriesAfterServerErrorAndSucceeds() { + val (isRunning, requestCount) = check(Step.HttpResponse(503), runningResponse()) + + assertThat(isRunning).isTrue() + assertThat(requestCount).isEqualTo(2) + } + + @Test + fun givesUpAfterBoundedNumberOfAttempts() { + val (isRunning, requestCount) = + check( + Step.HttpResponse(503), + Step.HttpResponse(503), + Step.HttpResponse(503), + ) + + assertThat(isRunning).isFalse() + assertThat(requestCount).isEqualTo(3) + } + + @Test + fun doesNotRetryClientError() { + val (isRunning, requestCount) = check(Step.HttpResponse(404)) + + assertThat(isRunning).isFalse() + assertThat(requestCount).isEqualTo(1) + } + + @Test + fun doesNotRetryWhenSomethingOtherThanMetroResponds() { + val (isRunning, requestCount) = check(Step.HttpResponse(200, "not metro")) + + assertThat(isRunning).isFalse() + assertThat(requestCount).isEqualTo(1) + } + + private companion object { + private const val PACKAGER_RUNNING_BODY = "packager-status:running" + // Never contacted: the interceptor short-circuits every call before any socket work. + private const val PACKAGER_HOST = "localhost:8081" + private val TEXT_PLAIN: MediaType? = MediaType.parse("text/plain; charset=utf-8") + } +}