fix(react-native): pass the protocol from bundle URL to HMR client on Android #48970

byCedric · 2025-01-27T13:27:52Z

Summary:

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 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 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

Test Plan:

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:

curl "http://localhost:8081" -H "expo-platform: android" | jq .launchAsset.url

This should point the entry bundle URL towards the EXPO_PACKAGER_PROXY_URL.

… Android

byCedric · 2025-01-27T13:40:01Z

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

+   * @param isEnabled Whether HMR is enabled initially.
+   * @param scheme The protocol that the HMRClient should communicate with on the host (defaults to http).
+   */
+  void setup(String platform, String bundleEntry, String host, int port, boolean isEnabled, String scheme);


The HMR client both accepts .setup(...) and .setup(..., scheme) - thats why I duplicated the function and added it as another signature. But feel free to simplify this if it's unnecessary.

facebook-github-bot · 2025-01-27T14:56:16Z

@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2025-01-27T17:48:59Z

@javache merged this pull request in ba894c9.

react-native-bot · 2025-01-27T17:49:00Z

This pull request was successfully merged by @byCedric in ba894c9

^{When will my fix make it into a release? | How to file a pick request?}

cortinico · 2025-01-27T18:05:27Z

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

+   * @param port The port that the HMRClient should communicate with on the host.
+   * @param isEnabled Whether HMR is enabled initially.
+   * @param scheme The protocol that the HMRClient should communicate with on the host (defaults to http).
+   */


Heads up that this is a breaking change for any app/library that is implementing HMRClient in OSS.

Is there a way to make this not a breaking change? The React Native HMR client "spec" (JS side) allows both and defaults to scheme: http - while Android only allowed 1.

Not sure how we can do that in Java, making something optional

Wait, are you sure that this is a breaking change? This HMRClass seems pretty hardcoded into the system, so on the Java side I don't see how you can overwrite this.

Even if HMR Clients do not use the scheme prop, it still shouldn't be an issue in the JS side of the HMR client. If it worked without, it should still work with the scheme prop even when ignored.

Yup, as @javache pointed out, you're not supposed to implement this interface directly. I found no meaningful implementation of it, so it's safe to assume this is not a breaking change.

javache · 2025-01-27T22:39:27Z

This broke development internally.

java.lang.AssertionError: Method overloading is unsupported: com.facebook.react.devsupport.HMRClient#setup

react-native/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaScriptModuleRegistry.java

Line 62 in 3e2e8ec

throw new AssertionError(

facebook-github-bot · 2025-01-27T22:52:41Z

This pull request has been reverted by 6e9a462.

… 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: 49bf1dc60f11b2af6e57177141270632d62ab564

… Android (facebook#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](facebook#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](facebook/react-native@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 facebook#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. [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: facebook#48998 Test Plan: See full explanation on facebook#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: 49bf1dc60f11b2af6e57177141270632d62ab564

fix(react-native): pass the protocol from bundle URL to HMR client on…

d5c6b69

… Android

facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Expo Partner: Expo Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Jan 27, 2025

byCedric commented Jan 27, 2025

View reviewed changes

facebook-github-bot closed this in ba894c9 Jan 27, 2025

facebook-github-bot added the Merged This PR has been merged. label Jan 27, 2025

cortinico reviewed Jan 27, 2025

View reviewed changes

facebook-github-bot added the Reverted label Jan 27, 2025

byCedric mentioned this pull request Jan 28, 2025

fix(react-native): pass the protocol from bundle URL to HMR client on Android #48998

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(react-native): pass the protocol from bundle URL to HMR client on Android #48970

fix(react-native): pass the protocol from bundle URL to HMR client on Android #48970

byCedric commented Jan 27, 2025 •

edited

Loading

byCedric Jan 27, 2025

facebook-github-bot commented Jan 27, 2025

facebook-github-bot commented Jan 27, 2025

react-native-bot commented Jan 27, 2025

cortinico Jan 27, 2025 •

edited

Loading

byCedric Jan 27, 2025 •

edited

Loading

byCedric Jan 27, 2025 •

edited

Loading

cortinico Jan 28, 2025

javache commented Jan 27, 2025

facebook-github-bot commented Jan 27, 2025

fix(react-native): pass the protocol from bundle URL to HMR client on Android #48970

fix(react-native): pass the protocol from bundle URL to HMR client on Android #48970

Conversation

byCedric commented Jan 27, 2025 • edited Loading

Summary:

Changelog:

Test Plan:

byCedric Jan 27, 2025

Choose a reason for hiding this comment

facebook-github-bot commented Jan 27, 2025

facebook-github-bot commented Jan 27, 2025

react-native-bot commented Jan 27, 2025

cortinico Jan 27, 2025 • edited Loading

Choose a reason for hiding this comment

byCedric Jan 27, 2025 • edited Loading

Choose a reason for hiding this comment

byCedric Jan 27, 2025 • edited Loading

Choose a reason for hiding this comment

cortinico Jan 28, 2025

Choose a reason for hiding this comment

javache commented Jan 27, 2025

facebook-github-bot commented Jan 27, 2025

byCedric commented Jan 27, 2025 •

edited

Loading

cortinico Jan 27, 2025 •

edited

Loading

byCedric Jan 27, 2025 •

edited

Loading

byCedric Jan 27, 2025 •

edited

Loading