Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/50071
Pull Request resolved: https://github.com/facebook/react-native/pull/50070
Changelog:
[General][Internal] Send code and close reasons when we close the connection to any websockets from the dev middleware
This should improve the debuggability of our code.
Reviewed By: robhogan
Differential Revision: D71314509
fbshipit-source-id: 1d6fc57a8601bcea78e95a87d423c7c46c51e799
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49922
Spamming the DB with "heartbeat" events with close to 0 ping didn't give us any useful information. Instead, report high ping situations.
Changelog:
[General][Internal] Remove reporting all device and debugger heartbeat events, only report heartbeats with high round trip latency
Reviewed By: GijsWeterings
Differential Revision: D70707457
fbshipit-source-id: 6bc8a55fd32dfa60fb4f7764ac00a6218417dcb5
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49726
The `InspectorProxy` debug logging became incredibly spammy making it almost unuseful unless the output is manipulated so this commit batches all these cdp messages and only desplays how many were received in the span of 5s. If no messages are received, the throttle is not triggered.
To get the actual CDP messages logged, we still log these to `Metro:InspectorProxyCDPMessages`.
Changelog:
[General][Internal] batch debug logging for cdp messages
Reviewed By: robhogan
Differential Revision: D70324724
fbshipit-source-id: a269302f52e18af6c4be651758c042596abdbad8
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49635
Changelog:
[General][Internal] - report proxy idle state in heartbeat events
The proxy is considered Idle if it didn't receive any messages for 10 seconds.
Reviewed By: robhogan
Differential Revision: D70078637
fbshipit-source-id: 5b8d9a3a6e3e2568e40b3ada0a41e76b277fe66d
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49618
Changelog:
[General][Internal] - create a heartbeat for device, and not only for debugger.
Introducing a heartbeat for the connection between the proxy and the device similarly to the one between the proxy and the debugger. This will allow us to:
* Most importantly I'd like to track the ping-pong roundtrip to the device as well, to see if we have any anomalies there
* Terminate the connection if it is abandoned for 60seconds- this might have a real effect in some case where the device runs remotely
* Also keep that connection alive if the other side disconnects after a period of inactivity. While a no-op in our case, this is an implementation detail. It is a no-op because the WebSocket on the Device is implemented by us and is not supposed to drop connections like the browser does.
Reviewed By: robhogan
Differential Revision: D69990715
fbshipit-source-id: 6bb3a2ed3eaffff9535aa2d0fc8cff0262af022f
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49552
Changelog:
[General][Internal] - expand debugger events to have DebuggerSessionIDs
Also moved the handling of these to a shared function
Reviewed By: huntie
Differential Revision: D69917817
fbshipit-source-id: 2374ac5b5dc0040b0e15028ab89fbe78026bc296
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49499
Changelog:
[General][Internal] - warn when user tries to open DevTools, when not in debug mode
Reviewed By: huntie
Differential Revision: D69784836
fbshipit-source-id: aa60762f08f5dea1f91f4fd1ffc86eeb7f94e2fc
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49441
Changelog:
[General][Internal] - keep heartbeat between inspector proxy and debugger going even if not idle
When heartbeat was only used to keep the connection alive and to detect debugger timeouts, it was enough to send a ping every time the connection was idle for 10 seconds.
Now, when we use the heartbeat as a way to track how good is the round trip time between the inspector proxy and the debugger, we would like to make this tracking more reliable by sending a ping 10 seconds after each pong, even if the connection is not idle.
It also simplifies the code and makes it more clear by removing the confusing `shouldSetTerminateTimeout` variable.
Reviewed By: hoxyq
Differential Revision: D69665738
fbshipit-source-id: c8175b54d2b3df32ee60b316ec7bcf7bc0f0a1ee
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49358
When the network is under strain, the code responsible for detecting if the inspector proxy's connection to the client has been lost may incorrectly assume the connection is dead. This false positive occurs because the system assumes that if a pong is not received within 5 seconds of a ping, the other side has disconnected. However, I was able to consistently reproduce scenarios where a delay of more than 5 seconds (even more than 20 seconds) was followed by a return to normal ping-pong communication without any issues.
Since I can't think of any issues with increasing this number, I'm increasing it to 60s.
Changelog:
[General][Fixed] - Disconnections of DevTools when the network is under significant strain.
Reviewed By: robhogan, huntie
Differential Revision: D69523906
fbshipit-source-id: 50db1e7bbe690b42421bc226aa30fd6571ba2257
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49353
This change adds an opt-in to restore JavaScript log streaming via the Metro dev server, [removed from React Native core in 0.77](https://reactnative.dev/blog/2025/01/21/version-0.77#removal-of-consolelog-streaming-in-metro).
Users can opt into this legacy behaviour by adding the `--client-logs` flag to `npx react-native-community/cli start`.
- The default experience remains without streamed JS logs.
- The existing "JavaScript logs have moved! ..." notice is printed in all cases, and we do not advertise the new flag for new users.
- Under non-Community CLI dev servers (i.e. Expo), log streaming is restored implicitly.
We will clean up this functionality again when we eventually remove JS log streaming over `HMRClient`, tasked in T214991636.
**Implementation notes**
- Logs are always sent over `HMRClient` (previous status quo), even with log streaming off in the dev server. This is a necessary evil to be able to flag this functionality in a user-accessible place, and to move fast for 0.78.
- Necessarily, emitting `fusebox_console_notice` moves to the dev server itself, on first device (Fusebox) connection.
Changelog:
[General][Added] - Add opt in for legacy Metro log streaming via `--client-logs` flag
Reviewed By: robhogan
Differential Revision: D69469039
fbshipit-source-id: be99d02a3b1c977a59bf7d2726f0e6cf2e60b28a
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49001
In {D42973408}, `Debugger.scriptParsed` was tweaked to be intercepted in `inspector-proxy`, which:
1. Rewrote the `sourceMapURL` to be relative to debugger.
2. Attempted to fetch the contents of the source map from `sourceMapURL` after re-writing again to a server-relative URL, and if successful replaced `sourceMapURL` with a base64 data URL.
1 is still needed until we have `Network.loadNetworkResource`, but 2 was only needed for frontends that did not support http fetch, and is not needed with Fusebox.
Changelog: [General][Changed] `Debugger.scriptParsed` now includes the field `sourceMapURL` as a (rewritten) remote url as opposed to base64 data url
Reviewed By: robhogan
Differential Revision: D68708899
fbshipit-source-id: 95242582c79ce4e9a573d4a3e639b0dc3290869e
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/47968
Updates the Inspector Proxy to report + log when a profiling build target (experimental) is registered. This notifies the developer that debugging is available for these app(s), which will not otherwise fetch development bundles from Metro.
Changelog: [Internal]
Reviewed By: rubennorte
Differential Revision: D66501771
fbshipit-source-id: e06dee279158094ad5c70bf8e6a90e7c983de48a
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/47880
The previous diffs in this stack have aimed to make URL rewriting by inspector-proxy robust to any configuration of device->server, debugger->server and server->server connections.
Though rewriting was originally introduced to support Android emulator networking, we can now expand it to cover other use cases, like the device reaching the server over an internet address not reachable from the dev machine, or the debugger routing to the server through a tunnel on a different port, without needing CORS workarounds.
Changelog
[General][Fixed] dev-middleware: Rewrite URLs in the inspector proxy to cover all configurations, not just Android emulators.
Reviewed By: huntie
Differential Revision: D66247355
fbshipit-source-id: e9201ebc1f7f5fe2119c71cd4d7b4ca895645404
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/47876
Currently, if a device is connected to the bundler via, say, `http://10.0.2.2:8081` (Android default), and a debugger is opened on `http://localhost:8081`, we rewrite hostnames `10.0.2.2.`<->`localhost` so that URLs are correct relative to each.
However, if the debugger is on a different port or protocol, this breaks down - because we only rewrite hostnames.
This fixes that by using the debugger's connecting `Host` header and `encrypted` state to derive the base URL of the server relative to the frontend. We then update the rewriting logic to use this actual full origin (protocol + host) in place of the device-relative origin.
Changelog:
[General][Fixed] dev-middleware: Fix URL rewriting where device and debugger reach the server on different ports/protocols.
Reviewed By: huntie
Differential Revision: D66077627
fbshipit-source-id: 01f6565149caa34b1e9e50dd58deb0122485657c
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/47872
Largely a refactoring to the way we currently rewrite `url`/`urlRegex` in `Debugger.setBreakpointByURL` CDP requests (debugger->target).
Rewriting regexes is fragile, it only really works if we can assume `'localhost'` appears literally and that ports and protocols don't need changing. The intention here is to freeze the current behaviour so as not to break anyone relying on it (if anyone is), and decouple it from more robust rewriting we want to generalise.
Also adds simple regex escaping to host names (always IPv4 addresses) we inject into regex patterns, since the previous approach could've led to false matches in unlikely edge cases.
Changelog:
[General][Fixed] dev-middleware: Regex-escape IP addresses in urlRegex replacements
Reviewed By: huntie
Differential Revision: D66238782
fbshipit-source-id: 4cd0029081d68c193e36d3713057ffdc7ef0656f
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/47685
Currently, we assume any URL with a hostname of `10.0.2.2` or `10.0.3.2` (device-relative) is eligible for rewriting to `localhost` (frontend-relative), because we assume the device is an Android emulator. We rewrite these URLs between device and dev machine so that the rewritten URLs are reachable from the dev machine.
This diff narrows this logic so that we'll only rewrite URLs where the hostname matches the pre-existing list *and* this matches the host the device is actually connected on, according to its headers from the original connection.
The main motivation for this change is to unblock removing assumptions about device-reachable vs server-reachable hosts. Later in the stack we'll drop the hardcoded listing of `10.0.2.2` etc in favour of identifying URLs that target the dev server, from whatever network.
There's also an edge case fix here that `10.0.2.2` etc might actually refer to a remote LAN server, and not be an Android emulator's alias for for an emulator host.
Changelog:
[General][Fixed] RN DevTools: Don't assume 10.0.2.2 is an alias for localhost unless it's used to establish a connection to the server
Reviewed By: huntie
Differential Revision: D66058704
fbshipit-source-id: bad28717b0c9b1ca43e2ea3391cef13f87892e6c
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/47653
## Context
Currently, when `nativeSourceCodeFetching == false`, `inspector-proxy` attempts to pre-fetch source maps, given the URL from a `Debugger.scriptParsed` event, and embeds them into `Debugger.scriptParsed`'s `sourceMapURL` using a data URI.
This was originally to support frontends that did not perform HTTP requests or were blocked (eg by CORS), but we're retaining it for the moment because it's more performant than lazy loading the source map.
Similarly, we perform middleware->server fetches to respond to `Debugger.getScriptSource` events.
To make these fetches for URLs that target `10.0.2.2` (ie, addressable from within an Android emulator) (etc), we rewrite `10.0.2.2`->`localhost` and perform a `fetch` from the Node process running dev-middleware.
## The problem
Consider a setup where:
- Metro is running on a remote server, listening on `8081`.
- Dev machine tunnels `localhost:8082` -> remote `8081`.
- An app is running on an Android emulator on the dev machine, with bundle URL configured to `10.0.2.2:8082`.
In this case, we'll rewrite `10.0.2.2:8082` to `localhost:8082`, which *is* reachable and correct from the dev machine, but *not* from the machine where Metro is running, so the `fetch` of a source map from the inspector proxy will fail.
## Motivation
This might seem like a niche case, but it's part of fixing a series of unsafe assumptions that currently prevent us from running DevTools on an arbitrary port.
## This fix
Preserve the current behaviour (simple `10.0.2.2`<=>`localhost`) for URLs sent to the frontend, but construct a separate, server-relative URL, using the configured `serverBaseUrl`, for `fetch` calls within dev-middleware.
Changelog:
[General][Fixed] RN DevTools: Fix fetching sources and source maps when the dev-server is remote and not tunnelled via the same port+protocol.
Reviewed By: huntie
Differential Revision: D65993910
fbshipit-source-id: a0cdcf1644e97a2af3d8583f2da2aaa51276f68c
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/47628
`serverBaseUrl` is currently documented as:
> The base URL to the dev server, as addressible from the local developer machine
This is problematic in general because `dev-middleware` on a server doesn't necessarily know about where clients might be reaching it from, how tunnels or port-forwards are set up, etc., and this can change over the lifetime of the server and vary between clients.
Indeed, our own use of `serverBaseUrl` from both `community-cli-plugin` and internally simply sets it to the host and port the dev server is listening on - ie it's the address of the dev server accessible *from the server*.
This PR changes the docs, redefining `serverBaseUrl`, to match the way we currently specify it.
One usage where we *do* want the previously documented behaviour is in responses to `/json/list` (`getPageDescriptions`) where the URLs in the response should be reachable by a browser requesting `/json/list`.
Here, we use the request (host header, etc.) to attempt to get working base URL.
History:
It should be mentioned that this is the latest in a series of changes like this:
- https://github.com/facebook/react-native/pull/39394
- https://github.com/facebook/react-native/pull/39456
Learning from those:
- This change does *not* break Android emulators, which routes `10.0.2.2` to localhost, or other routed devices, because `/open-debugger` still uses server-relative URLs, and now formally delegates to `BrowserLauncher` to decide what to do with those URLs (internally, VSCode / `xdg-open` handles port forwarding)
- Middleware configuration is no longer required to specify how it is reachable from clients.
This sets up some subsequent changes for more robust handling of tunnelled connections.
Changelog:
[General][Breaking] dev-middleware: Frameworks should specify `serverBaseUrl` relative to the middleware host.
Reviewed By: huntie
Differential Revision: D65974487
fbshipit-source-id: 1face8fc7715df387f75b329e80932d8543ee419
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/47652
## Background
When the `nativeSourceCodeFetching` capability is disabled, `inspector-proxy` rewrites URLs exchanged over CDP between device and frontend so that URLs are addressable from CDT - in particular, when using an Android emulator `10.0.2.2` (host's address from within the emulator) is rewritten to and from `localhost` (the equivalent address reachable from the host).
Previously - before we implemented `Network.loadNetworkResource`, or on old frontends that don't attempt to use that method - this worked reasonably well. A `fetch` from CDT to Metro would succeed on the rewritten URL.
## Problem
Since we implemented `Network.loadNetworkResource`, but disabled the `nativeSourceCodeFetching` capability, source fetching is broken under Android emulators. We're rewriting URLs to be frontend-relative, but then attempting to fetch them through the device, because as far as CDT is aware, `Network.loadNetworkResource` should still be tried first.
When `Network.loadNetworkResource` responds with a CDP *error*, CDT falls back to a local fetch (which would work), but when it responds with a CDP *result* of `success: false`, there is no fallback.
## Fix
This diff adds an interception guarded behind `nativeSourceCodeFetching == false`, which rejects any calls to `Network.loadNetworkResource` with a CDP error. This restores the previous behaviour from before `Network.loadNetworkResource` was implemented at all.
NOTE: An alternative approach would be to rewrite URLs back to device-relative for `Network.loadNetworkResource`, but IMO it's more correct for the frontend to respect that the device is asserting that it doesn't have that capability, and not to try to use it.
Changelog:
[Android][Fixed] RN DevTools: Fix source loading when using an Android emulator connecting to a dev server on the host.
Reviewed By: huntie
Differential Revision: D66074731
fbshipit-source-id: f2050c014cd5cfa546bff5e9d0412413a5daff35
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/47120
Fixes no-op behaviour of the "Open DevTools" Dev Menu item (bug on `main` introduced with D63329456).
This was caused by a change to the `description` field contents in our CDP `/json/list` response, when under Fusebox. In the `/open-debugger` call from the Dev Menu, we were still using the older `appId` param.
This did not affect `j` to debug, which uses the `target` param.
{F1937186832}
Changes:
In short: Matching against the `description` string is now fully eliminated for modern debugger targets.
- Update native Dev Menu implementation to omit `appId` parameter (`device` param alone is sufficient and fully precise on these platforms).
- Update `/open-debugger` implementation to ignore the `appId` parameter for modern targets, and document this in the `dev-middleware` README.
Changelog: [Internal]
Reviewed By: robhogan
Differential Revision: D64597581
fbshipit-source-id: 46f536e7d0a4ececab0d52f4c0704e8698466cd0
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/47098
Changelog: [Internal]
D63329456 updated the `description` field to be human-readable.
Unfortunately, InspectorProxy relies on this field to compare the incoming `/open-debugger` calls.
hoxyq discovered the symptom of Fusebox failing to launch with `No compatible apps connected. React Native DevTools can only be used with the Hermes engine.` in Metro
Reviewed By: hoxyq
Differential Revision: D64547367
fbshipit-source-id: deed6851f3ede2c74be2b492def1eba6e58c43e6
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46780
This is primarily a debugger server change to better-align the `title` and `description` fields (visible in via the CDP `/json/list` endpoint), reporting them directly from the device.
For React Native users, the net effect of this change is to improve/align the display name for each debugger target in the CLI multi-select menu (`j` to debug). This change may also be useful for discovery in non-Fusebox frontends such as VS Code (`vscode-expo` extension).
Changes:
- Rename `title` prop on `InspectorPageDescription`/`IInspector::addPage` to `description` (no call site changes).
- Add `deviceName` param to `InspectorPackagerConnection`.
- Move the page `description` to the `description` JSON field.
- Update `InspectorPackagerConnection::Impl::pages()` to return new `title` and `description` fields.
- Align `OpenDebuggerKeyboardHandler` to display `title` field.
- Deprecate the nonstandard `deviceName` field.
**Before**
```
[
{
"id": "3c9f24bedab0e73fca6a1b295030e7af9346a8c0-1",
"title": "React Native Bridgeless [C++ connection]",
"description": "com.facebook.RNTester",
...
}
```
The `description` field was previously the closest thing we had to a target identifier. Today, this is not needed, since we have the stable `reactNative.logicalDeviceId` field.
**After**
```
[
{
"id": "3c9f24bedab0e73fca6a1b295030e7af9346a8c0-1",
"title": "com.facebook.RNTester (iPhone 16 Pro)",
"description": "React Native Bridgeless [C++ connection]",
...
}
```
The `title` field is now more human readable and aligned with what we render in the window title of React Native DevTools. The `description` field describes the type of debugger target (the specific React Native implementation).
Changelog: [Internal]
Reviewed By: vzaidman
Differential Revision: D63329456
fbshipit-source-id: cfe98f77e31c729431005925cfc66e2780ef8c72
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/45069
Currently, `/json/list` returns pages within each device in the iteration order of a C++ `unordered_map`, which doesn't tell us anything useful. Page IDs happen to be sequential, but only as an implementation detail.
Change this contract so that we guarantee ordering reflects addition order, allowing clients to consistently select e.g. most recently added page for a given device.
The implementation of this is as simple as switching from an `unordered_map` to a key-ordered`map`, because we already assign keys (page IDs) with an incrementing integer. Within the inspector proxy, devices already use an insertion (connection)-ordered JS `Map`, so we just document this guarantee.
Changelog:
[General][Changed] Debugger: Make `/json/list` return connection-addition-ordered targets.
Reviewed By: huntie
Differential Revision: D58735947
fbshipit-source-id: 7a132cc5e750475792a2b845afc9a42424690bf1
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/45035
Changelog: [General][Fixed] Avoid a zombie state when opening a second debugger frontend concurrently.
The problem here was that we were sending proxy-protocol messages to the device in the wrong order (`disconnect` *after* `connect`):
{F1701266597}
The root cause was that we were depending on the outgoing debugger socket's async `close` event to trigger sending the `disconnect` message to the device. This would happen after we'd already (synchronously) sent the `connect` message.
With this diff, we send the `disconnect` message synchronously with calling `close()` on the debugger socket, which fixes the ordering problem at the source. To avoid sending duplicate `disconnect` messages (e.g. one before calling `close()` and one from the `close` event handler), we store some extra state on `Device` (`#connectedPageIds`).
Reviewed By: robhogan, huntie
Differential Revision: D58730634
fbshipit-source-id: 0f54af2e4f8071a8f6d97cc9e3d8a4ea89a46f43
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/45027
Changelog: [Internal]
Changes the device ID collision handling logic to reuse `Device` instances instead of creating new ones. This enables further refactoring of `Device` to improve session state isolation.
Reviewed By: hoxyq
Differential Revision: D58724884
fbshipit-source-id: bc11ce45ce8c80c58c32dcd1b07b28f1d1753a62
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/45010
D46482492 added logic for handing off state across "device" connections that have the same ID. This logic currently has no test coverage. It also contains a bug whereby the new device's pages are removed from the target listing endpoint (`/json`) when the *old* device's socket is closed.
This diff adds tests and fixes the bug.
Changelog: [General][Fixed] inspector-proxy no longer accidentally detaches connected devices.
## Next steps
It seems that the device ID handoff logic exists to paper over a deeper problem with the inspector proxy protocol (or its implementation in React Native): The React Native runtime should not routinely be creating new "device" connections without tearing down previous ones.
In followup diffs, I'll explore changing this behaviour for Fusebox, based on the new test coverage.
Reviewed By: robhogan
Differential Revision: D51013056
fbshipit-source-id: e0c17678cc747366a3b75cef18ca2a722fc93acd
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44811
Changelog:
[General][Fixed] - Debugger frontend socket-termination countdown now begins after the ping message is actually sent
The debugger is currently disconnected if a ping-pong message is missed.
This causes the debugger to be unusable if it happens to be lagging, e.g. when the initialisation is competing with the flood of log spam T191394188
There are a few ways to fix this as discused with motiz88 and robhogan:
1. Ensure the websocket has a chance to respond, e.g. in via web worker
1. Lengthen the time allowed for the pong resopnse
I've done some digging to find the root cause of the UI being blocked in CDT, However, profiling shows that most of the work is not simple to break up, i.e. the number of expensive re-layout calls. Diving into that rabbit hole could mean accidentally writing React.
Because we ping every 10 seconds, we could get un/lucky where CDT happens to be busy _at that exact moment_, making this a flaky symptom to fix, even if we lengthen the allowed time-to-respond.
# V2+
So upon further investigation, CDT websocket is actually responding to the pings in due time:
{F1679132204}
(CDT doesn't show the ping/pong API as frames, so a custom tick/tock message was used to visualise the timing)
Over here in dev-middleware, we currently start a timeout to terminate the socket after sending the ping:
https://www.internalfb.com/code/fbsource/[813870db697a8701f2512d25a7fed730f0ec6ed9]/xplat/js/react-native-github/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js?lines=306-307
If CDT doesn't respond in time, websocket would be terminated.
But we saw CDT respond immediately above, even during the log spam, so the delay must be coming from somewhere else.
The intuition is that during the log-spam, the middleware takes a perf hit too when it's processing the spam from the device and forwarding it to the CDT websocket.
We can confirm this by passing a "sent" callback via `socket.ping(cb)`:
https://github.com/websockets/ws/blob/9bdb58070d64c33a9beeac7c732aac0f4e7e18b7/lib/websocket.js#L246-L254
This gives us the timing between calling `socket.ping()` and when the ping is actually sent.
Regular, stress-free operation without log-spam shows most pings are sent within the same millisecond:
{F1679223326}
With the pong response grace period at 5 seconds, there's plenty of time for CDT to `pong` back. That's why it has been working in most cases.
However, during the log-spam, we easily see this send-sent delay over 5 seconds. In extreme cases, almost 30 seconds would have passed before middleware sent a message to CDT, which then responded under 2 seconds:
{F1679163335}
This means while CDT is getting flooded and has observable lag in the UI, the smoking gun is actually the middleware.
Digging a little deeper, we know that incoming messages from the target goes into a Promise queue, including the console logs:
https://www.internalfb.com/code/fbsource/[d5d312082e9c]/xplat/js/react-native-github/packages/dev-middleware/src/inspector-proxy/Device.js?lines=155-157
This means during the flood of logs from the target, the Promise queue keeps getting chained rapidly for each message.
Meanhile, the `ws` lib uses the underlying NodeJS `Socket.write` method for `ping(…)` and `send(…)`:
https://github.com/websockets/ws/blob/9bdb58070d64c33a9beeac7c732aac0f4e7e18b7/lib/sender.js#L349
…which is guaranteed to fire the callback asynchronously:
https://github.com/nodejs/help/issues/1504#issuecomment-422879594
Promise queue is in the macro task queue, which gets priority before the micro task queue. So if the Promise queue is not cleared yet, the websocket queue will have a hard time getting executed in time – explaining the extreme send-sent durations during a log spam.
The fix is simple:
1. Start the terminate-socket-timer until the `ping` is actually sent
1. Treat any incoming message (along with `pong`s) as a terminate-socket-timer reset
1. This also applies if `pong` comes in between `send` and `sent`, which can happen sometimes due to the async nature of the callback:
{F1679288626}
# V1
~~In this diff, a more forgiving mechanism is introduced, i.e. CDT is allowed to miss a ping-pong roundtrip 3 times before the websocket connection is terminated.~~
~~This allows a bit more breathing room for CDT's initialisation during log spam while maintaining the same ping-pong interval for VS Code to keep the auto SSH tunnel alive.~~
Reviewed By: huntie
Differential Revision: D58220230
fbshipit-source-id: 7111c9878492d8755a6110a5cdf4ef622265001d
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44835
As titled. The `vm` field is not part of the CDP spec and will not be used by the modern debugger frontend or proxy.
This change affects modern CDP targets only (using `InspectorPackagerConnection`). We aim to enable sharing of more detailed metadata over 1/ a new, dedicated CDP domain, and 2/ namespaced under the existing `reactNative` field (for the latter, strictly limited to metadata necessary for dev server functionality).
Changelog: [Internal]
(Note: `/json` endpoint behaviour is unchanged for legacy CDP targets)
Reviewed By: robhogan
Differential Revision: D58285587
fbshipit-source-id: dfef3a56b20486ba11891df9940f6c7bef59528e
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44499
Changelog: [internal]
As [discussed](https://fb.workplace.com/groups/react.devx.team/permalink/930483712103526/), we'll begin segmenting Telemetry signals by Fusebox/non-Fusebox.
* Add new flag in event reporter for `debugger_command` (other events in subsequent diffs)
* Add new column to the Scuba destination
Reviewed By: blakef
Differential Revision: D57140479
fbshipit-source-id: 7ea813b1b4d53a282873fa95c8ee82e5d6f3d1d3
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44498
Changelog: [internal]
Quick refactor to reduce commit noise in the following diff in the stack
Reviewed By: blakef
Differential Revision: D57140480
fbshipit-source-id: aa1fef83d5347b8a11651d3d5c4112b4adf7a7d5
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44092
In a setup where a device retrieves a bundle from `http://127.0.0.1:8081`, but this is tunnelled to a remote host with only an IPv6 stack (eg, FB dev servers), the host running the inspector-proxy will fail to fetch source or source maps from 127.0.0.1 despite typically being on the same host (indeed, process) as Metro.
This causes a surprising inconsistency where using a bundler URL of `localhost` from the device results in source maps being inlined into `Debugger.scriptParsed`, but using a bundler URL of `127.0.0.1` causes inspector-proxy to fall back to preserving URLs, which are typically fetched lazily by CDT later.
This should be unnecessary once we've implemented CDP `Network.loadNetworkResource` and removed `Debugger.scriptParsed` rewriting, but for now it brings IPv6 tunnelled servers in line with local servers.
Changelog:
[General][Changed] Inspector proxy: Rewrite 127.0.0.1 to localhost in source map URLs for better IPv4->IPv6 tunnelling support.
Reviewed By: motiz88
Differential Revision: D56138742
fbshipit-source-id: b65c9cc8225a0ed54cf32171f640ef9e6408c762
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44086
When a debugger frontend is connected to inspector-proxy via another proxy or tunnel that times out on idle (such as [VS Code's remote tunnel](https://github.com/microsoft/vscode/blob/main/src/vs/platform/tunnel/node/tunnelService.ts)), the connection between proxy and debugger may be dropped.
In addition, when the connection is dropped without a closing handshake, the proxy does *not* detect the disconnection - no disconnect is logged to the reporter and no notifications are sent to any connected devices.
This adds a mechanism using the WebSocket-standard `ping` and `pong` frames to:
1. Keep the connection alive
2. Detect when the debugger has gone away
Note that as all WebSocket clients already **must** reply to a ping with a pong, this is non-breaking for compliant implementations: https://datatracker.ietf.org/doc/html/rfc6455#section-5.5.2
Changelog:
[General][Added] Inspector proxy: Add ping/pong keepalive to debugger connections.
Reviewed By: hoxyq
Differential Revision: D56069185
fbshipit-source-id: e322de631c652a502f3d554c15ed5412a751ee04
Vitali Zaidman
Alex Hunt
Rob Hogan
Edmond Chui
Cedric van Putten
Moti Zilberman