Skip to content

Commit

Permalink
fix(react-native): pass the protocol from bundle URL to HMR client on…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
byCedric authored and jakex7 committed Feb 4, 2025
1 parent 660e5b9 commit 4d1a3f8
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 3 deletions.
2 changes: 1 addition & 1 deletion packages/react-native/ReactAndroid/api/ReactAndroid.api
Original file line number Diff line number Diff line change
Expand Up @@ -2248,7 +2248,7 @@ public abstract interface class com/facebook/react/devsupport/HMRClient : com/fa
public abstract fun disable ()V
public abstract fun enable ()V
public abstract fun registerBundle (Ljava/lang/String;)V
public abstract fun setup (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;IZ)V
public abstract fun setup (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;IZLjava/lang/String;)V
}

public final class com/facebook/react/devsupport/InspectorFlags {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -615,8 +615,11 @@ private void resetCurrentContext(@Nullable ReactContext reactContext) {
// strip initial slash in path
String path = sourceUrl.getPath().substring(1);
String host = sourceUrl.getHost();
String scheme = sourceUrl.getProtocol();
int port = sourceUrl.getPort() != -1 ? sourceUrl.getPort() : sourceUrl.getDefaultPort();
mCurrentContext.getJSModule(HMRClient.class).setup("android", path, host, port, mDevSettings.isHotModuleReplacementEnabled());
mCurrentContext
.getJSModule(HMRClient.class)
.setup("android", path, host, port, mDevSettings.isHotModuleReplacementEnabled(), scheme);
} catch (MalformedURLException e) {
showNewJavaError(e.getMessage(), e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,11 @@ public interface HMRClient extends JavaScriptModule {
* @param host The host that the HMRClient should communicate with.
* @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).
*/
void setup(String platform, String bundleEntry, String host, int port, boolean isEnabled);
void setup(
String platform, String bundleEntry, String host, int port, boolean isEnabled, String scheme);

/** Registers an additional JS bundle with HMRClient. */
void registerBundle(String bundleUrl);
Expand Down

0 comments on commit 4d1a3f8

Please sign in to comment.