Commit 32fe244
fix(react-native): pass the protocol from bundle URL to HMR client on Android (#48998)
Summary:
This is another attempt at fixing the Android HMR client for HTTPS proxied Metro instances. The previous one unintentionally [caused the following error](#48970 (comment)):
```
java.lang.AssertionError: Method overloading is unsupported: com.facebook.react.devsupport.HMRClient#setup
```
This PR removes the overloading, and only adds the `scheme` property as a parameter to the existing `.setup` method. Aligning with the exact behavior we have on iOS.
The alternative fix, which should NOT be backward breaking (if this is) - is to move this "infer the protocol from the bundle URL" to the JS side of the HMR client. Where we don't just always default to `http`, but instead default to `https IF port === 443, otherwise http`. It's a bit more hacky, but shouldn't cause any other issues. _**Ideally**_, we have the same working behavior on both Android and iOS without workarounds.
<details><summary>Alternative workaround</summary>
See [this change](main...byCedric:react-native:patch-2).
<img width="1179" alt="image" src="https://github.com/user-attachments/assets/47c365bc-6df8-43e6-ad7d-5a667e350cd4" />
</details>
See full explanation on #48970
> We've noticed that the HMR on Android doesn't seem to be connecting when using a HTTPS-proxied Metro instance, where the proxy is hosted through Cloudflare. This is only an issue on Android - not iOS - and likely caused by the HMR Client not being set up properly on Android.
>
>- On Android, we run `.setup('android', <bundleEntryPath>, <proxiedMetroHost>, <proxiedMetroPort>, <hmrEnabled>)` in the [**react/devsupport/DevSupportManagerBase.java**](https://github.com/facebook/react-native/blob/53d94c3abe3fcd2168b512652bc0169956bffa39/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java#L689-L691) file.
>- On iOS, we run `[self.callableJSModules invokeModule:@"HMRClient" method:@"setup" withArgs:@[ RCTPlatformName, path, host, RCTNullIfNil(port), @(isHotLoadingEnabled), scheme ]];` in the [**React/CoreModules
/RCTDevSettings.mm**](https://github.com/facebook/react-native/blob/53d94c3abe3fcd2168b512652bc0169956bffa39/packages/react-native/React/CoreModules/RCTDevSettings.mm#L488-L491) file.
>
>Notice how Android does not pass in the scheme/protocol of the bundle URL, while iOS actually does? Unfortunately, because the default protocol (`http`) mismatches on Android when using HTTPS proxies, we actually try to connect the HMR client over `http` instead of `https` - while still using port 443 - which is rejected by Cloudflare's infrastructure even before we can redirect or mitigate this issue. And the rejection is valid, as we basically try to connect on `http://<host>:443` (the source URL is `https`, so the port is infered as `443`).
>
>This change adds scheme propagation to Android, exactly like we do on iOS for the HMR Client.
## Changelog:
[ANDROID] [FIXED] Pass the bundle URL protocol when setting up HMR client on Android
<!-- Help reviewers and the release process by writing your own changelog entry.
Pick one each for the category and type tags:
[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message
For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
Pull Request resolved: #48998
Test Plan:
See full explanation on #48970
> It's a little bit hard to test this out yourself, since you'd need a HTTPS-based proxy and reject HTTP connections for HTTPS/WSS Websocket requests.
>
>You can set this up through:
>- `bun create expo@latest ./test-app`
>- `cd ./test-app`
>- `touch .env`
>- Set `EXPO_PACKAGER_PROXY_URL=https://<proxied-metro-hostname>` in **.env**
>- Set `REACT_NATIVE_PACKAGER_HOSTNAME=<proxied-metro-hostname>` in **.env**
>- `bun run start`
>
>Setting both these envvars, the bundle URL in the manifest is set to `https://...` - which triggers this HMR issue on Android. You can validate the **.env** setup through:
>
>```bash
>curl "http://localhost:8081" -H "expo-platform: android" | jq .launchAsset.url
>```
>
>This should point the entry bundle URL towards the `EXPO_PACKAGER_PROXY_URL`.
Reviewed By: cortinico
Differential Revision: D68768351
Pulled By: javache
fbshipit-source-id: 49bf1dc60f11b2af6e57177141270632d62ab5641 parent bb1e3cd commit 32fe244
File tree
3 files changed
+8
-3
lines changed- packages/react-native/ReactAndroid
- api
- src/main/java/com/facebook/react/devsupport
3 files changed
+8
-3
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2188 | 2188 | | |
2189 | 2189 | | |
2190 | 2190 | | |
2191 | | - | |
| 2191 | + | |
2192 | 2192 | | |
2193 | 2193 | | |
2194 | 2194 | | |
| |||
Lines changed: 3 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
665 | 665 | | |
666 | 666 | | |
667 | 667 | | |
| 668 | + | |
668 | 669 | | |
669 | 670 | | |
670 | 671 | | |
671 | | - | |
| 672 | + | |
| 673 | + | |
672 | 674 | | |
673 | 675 | | |
674 | 676 | | |
| |||
Lines changed: 4 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
26 | 26 | | |
27 | 27 | | |
28 | 28 | | |
| 29 | + | |
| 30 | + | |
29 | 31 | | |
30 | | - | |
| 32 | + | |
| 33 | + | |
31 | 34 | | |
32 | 35 | | |
33 | 36 | | |
| |||
0 commit comments