Skip to content

Commit 9ef28ea

Browse files
[Android] Fix BundleDownloader Content-Type matching for Metro's "application/javascript; charset=UTF-8"
Regression introduced in the streaming refactor (e9e200a). Metro actually serves the JS bundle chunk with Content-Type: application/javascript; charset=UTF-8 not the bare `application/javascript` my refactor was matching against. With strict equality, `onChunkHeader` returned null (so the streaming sink was never opened), the body was buffered into the in-memory accumulator, and `onChunkComplete` hit the `else -> Unit` branch and silently dropped it. Symptoms in dev on the RNTester emulator: heap spiked to ~200 MB during the download, then dropped back, but the dev loading view sat at "Bundling 99%" forever because callback.onSuccess() was never fired. Fix: parse Content-Type properly. New private helper `mediaType(...)` strips the parameter portion (`; charset=...`) and lower-cases the result, and the two routing predicates (`isJsBundleChunk` and `isProgressChunk`) compare against the parsed media type. Also: the else branch now logs a warning instead of silently dropping, so a future Metro change that introduces yet another Content-Type variant will be visible in logcat rather than stranding the user at 99%. Updated `LargeMultipartSource` to emit the real Metro Content-Type (`application/javascript; charset=UTF-8`) so the perf tests guard against this exact regression class going forward. The perf tests pass unchanged \u2014 the streaming path is still taken, output-size still equals payloadBytes, and peak heap is still bounded. Verified against a live Metro dev server: with the synthetic source now matching production, `BundleDownloaderPerfTest` would have caught this bug. `MultipartStreamReaderTest` (the 4 existing correctness cases) is unaffected and still green.
1 parent c869a8d commit 9ef28ea

3 files changed

Lines changed: 28 additions & 7 deletions

File tree

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/BundleDownloader.kt

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ public class BundleDownloader public constructor(private val client: OkHttpClien
243243

244244
@Throws(IOException::class)
245245
override fun onChunkHeader(headers: Map<String, String>): BufferedSink? {
246-
if (headers["Content-Type"] != "application/javascript") return null
246+
if (!isJsBundleChunk(headers)) return null
247247
val effectiveStatus = effectiveStatus(headers)
248248
if (effectiveStatus != 200) return null
249249
// Stream the JS bundle straight to disk — never materialize in heap.
@@ -265,8 +265,8 @@ public class BundleDownloader public constructor(private val client: OkHttpClien
265265
finalizeStreamedBundle(headers)
266266
return
267267
}
268-
when (headers["Content-Type"]) {
269-
"application/javascript" -> {
268+
when {
269+
isJsBundleChunk(headers) -> {
270270
// Bundle returned with an error status — it was buffered so we can surface a useful
271271
// diagnostic to the developer.
272272
val buffered = body ?: Buffer()
@@ -280,8 +280,12 @@ public class BundleDownloader public constructor(private val client: OkHttpClien
280280
callback,
281281
)
282282
}
283-
"application/json" -> dispatchProgressJson(body)
284-
else -> Unit // Unknown chunk type; ignore as before.
283+
isProgressChunk(headers) -> dispatchProgressJson(body)
284+
else -> {
285+
// Unknown chunk type. Log so a future Metro change is visible in logcat instead of
286+
// silently stranding the dev loading view at 99%.
287+
FLog.w(TAG, "Ignoring multipart chunk with unrecognized Content-Type: ${headers["Content-Type"]}")
288+
}
285289
}
286290
}
287291

@@ -339,6 +343,20 @@ public class BundleDownloader public constructor(private val client: OkHttpClien
339343

340344
private fun effectiveStatus(headers: Map<String, String>): Int =
341345
headers["X-Http-Status"]?.toIntOrNull() ?: outerStatus
346+
347+
/**
348+
* Extract the media type (the part before `;`) from a Content-Type header, lower-cased.
349+
* Metro sends e.g. `application/javascript; charset=UTF-8`, so a bare-string equality
350+
* check would miss the bundle chunk and leave the dev loading view stranded.
351+
*/
352+
private fun mediaType(headers: Map<String, String>): String? =
353+
headers["Content-Type"]?.substringBefore(';')?.trim()?.lowercase()
354+
355+
private fun isJsBundleChunk(headers: Map<String, String>): Boolean =
356+
mediaType(headers) == "application/javascript"
357+
358+
private fun isProgressChunk(headers: Map<String, String>): Boolean =
359+
mediaType(headers) == "application/json"
342360
}
343361

344362
@Throws(IOException::class)

packages/react-native/ReactAndroid/src/test/java/com/facebook/react/devsupport/LargeMultipartSource.kt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,11 @@ internal class LargeMultipartSource(
4242
// NB: the reader's delimiter is "\r\n--<boundary>\r\n", so the preamble must end with CRLF.
4343
// A bare CRLF is the minimal valid preamble.
4444
private val preamble: ByteArray = "\r\n".toByteArray(Charsets.UTF_8)
45+
// Note: matches what Metro actually sends. The `charset=UTF-8` parameter exists to catch
46+
// any future regression to bare-string Content-Type matching in BundleDownloader.
4547
private val headers: ByteArray =
4648
("--$boundary\r\n" +
47-
"Content-Type: application/javascript\r\n" +
49+
"Content-Type: application/javascript; charset=UTF-8\r\n" +
4850
"Content-Length: $payloadBytes\r\n" +
4951
"\r\n")
5052
.toByteArray(Charsets.UTF_8)

packages/react-native/ReactAndroid/src/test/java/com/facebook/react/devsupport/MultipartStreamReaderPerfTest.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,8 @@ class MultipartStreamReaderPerfTest {
102102
// and surfaced via onChunkComplete.
103103
assertThat(success).isTrue
104104
assertThat(discardingSink.bytesWritten).isEqualTo(payloadBytes)
105-
assertThat(receivedHeaders["Content-Type"]).isEqualTo("application/javascript")
105+
assertThat(receivedHeaders["Content-Type"])
106+
.isEqualTo("application/javascript; charset=UTF-8")
106107
assertThat(bufferDeliveredViaComplete)
107108
.`as`("Body must be streamed to the sink, not delivered as a Buffer")
108109
.isFalse

0 commit comments

Comments
 (0)