Commit Graph
68 Commits
Author SHA1 Message Date
Alex HuntandFacebook GitHub Bot 7f57018e25 Add profiling_target_registered event, log to terminal (#47968)
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
2024-11-27 11:42:31 -08:00
Rob HoganandFacebook GitHub Bot 74995bc90a dev-middleware: Generalise URL rewriting, don't only rewrite for Android emulators (#47880)
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
2024-11-22 10:26:43 -08:00
Rob HoganandFacebook GitHub Bot 5da7ebf99a dev-middleware: Don't assume device-relative and debugger-relative URLs have the same port (#47876)
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
2024-11-21 11:39:20 -08:00
Rob HoganandFacebook GitHub Bot aae3e03e57 dev-middleware: Refactor urlRegex rewriting, regex-escape IP4 addresses (#47872)
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
2024-11-21 10:09:41 -08:00
Rob HoganandFacebook GitHub Bot 69400be4fc dev-middleware: Only rewrite hostnames if they match device connection hosts (#47685)
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
2024-11-20 06:56:21 -08:00
Rob HoganandFacebook GitHub Bot d1b0e9a30b dev-middleware: Use serverBaseUrl for local->server fetches (#47653)
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
2024-11-18 16:23:53 -08:00
Rob HoganandFacebook GitHub Bot acf384a72e dev-middleware: Redefine "serverBaseUrl" as server-relative, '/json/list' by requestor (#47628)
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
2024-11-18 13:46:50 -08:00
Rob HoganandFacebook GitHub Bot ca9c56329f dev-middleware: Fix reliance on adb reverse when loading sources on Android (#47652)
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
2024-11-18 12:19:19 -08:00
Alex HuntandFacebook GitHub Bot 6f421230c1 Drop appId from /open-debugger calls, disable matching for modern targets (#47120)
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
2024-10-18 11:02:51 -07:00
Edmond ChuiandFacebook GitHub Bot 0d591d9dc3 add appId as a standalone field (#47098)
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
2024-10-17 12:47:05 -07:00
Alex HuntandFacebook GitHub Bot c430083fa0 Update modern inspector targets to directly send CDP title and description (#46780)
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
2024-10-02 07:30:31 -07:00
Cedric van PuttenandFacebook GitHub Bot 30a3e6e8df refactor(dev-middleware): drop node-fetch in favor of Node built-in fetch (#45227)
Summary:
Node 22 doesn't work well with `node-fetch@2`, as one of their polyfills is using the deprecated `punycode` module. This causes unnecessary warnings like:

<img width="986" alt="image" src="https://github.com/facebook/react-native/assets/1203991/13f66c5b-b6f4-4894-8576-ca9631d93f77">

Instead of upgrading to the [much larger `node-fetch@3`](https://packagephobia.com/result?p=node-fetch%403.3.2), this change drops `node-fetch` in favor of Node's own built-in `fetch` implementation (using [undici](https://github.com/nodejs/undici#readme)).

> Note, `react-native/dev-middleware` [already has the `engines.node >= 18`](https://github.com/facebook/react-native/blob/c7988c9c82793b6b41d4c9190a28ce1202410fa0/packages/dev-middleware/package.json#L38-L40) (which is required for fetch).

## Changelog:

[GENERAL] [CHANGED] - Drop `node-fetch` in favor of Node's built-in fetch from `undici` in `react-native/dev-middleware`

<!-- 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: https://github.com/facebook/react-native/pull/45227

Test Plan: See CI for passing tests

Reviewed By: NickGerleman

Differential Revision: D59202017

Pulled By: robhogan

fbshipit-source-id: 15da5d5602e63958b8a3dc581dc1512649f16c12
2024-07-03 09:14:02 -07:00
Moti ZilbermanandFacebook GitHub Bot a7adfef0bb Reject debugger connections to unknown page IDs (#45148)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/45148

Changelog: [General][Fixed] Reconnecting to an unknown inspector page no longer results in a zombie connection

TSIA

Reviewed By: hoxyq

Differential Revision: D58954759

fbshipit-source-id: 99c5caccc3cc917e0691e94326c7a35874f9a385
2024-06-24 12:59:53 -07:00
Rob HoganandFacebook GitHub Bot 53951d7fec Debugger: Make /json/list ordered (#45069)
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
2024-06-19 11:28:36 -07:00
Moti ZilbermanandFacebook GitHub Bot e55ea2daf1 Fix debugger handoff logic to prevent "zombie" state (#45035)
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
2024-06-18 09:39:35 -07:00
Moti ZilbermanandFacebook GitHub Bot 6695b6e3b8 Reuse Device instances when handing off connections based on device ID (#45027)
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
2024-06-18 09:39:35 -07:00
Moti ZilbermanandFacebook GitHub Bot db2c44a7eb Make inspector-proxy protocol types mutable by default (#45021)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/45021

Changelog: [Internal]

TSIA, hoisted from an upcoming refactor of `Device` in inspector-proxy.

Reviewed By: hoxyq

Differential Revision: D58723496

fbshipit-source-id: af272b08bc280d681bd7e1adb60d2a411a549864
2024-06-18 08:25:32 -07:00
Moti ZilbermanandFacebook GitHub Bot 4c6bff01b3 Fix bug in device ID collision handling, add tests (#45010)
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
2024-06-17 09:56:14 -07:00
Edmond ChuiandFacebook GitHub Bot 20462ca984 fix timing of terminating an unresponsive debugger socket (#44811)
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
2024-06-12 09:45:34 -07:00
Alex HuntandFacebook GitHub Bot d72ac96e8e Remove nonstandard "vm" field from modern CDP targets (#44835)
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
2024-06-11 08:48:45 -07:00
Alex HuntandFacebook GitHub Bot df19e597e3 Remove faviconUrl field from CDP list response (#44834)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44834

- Remove the nonstandard, unused `faviconUrl` field from CDP `/json` response targets (note: both legacy and modern targets).
- Reorder `PageDescription` members.

Changelog:
[General][Removed] - `react-native/dev-middleware`: Remove nonstandard `faviconUrl` field from CDP `/json` response

Reviewed By: hoxyq

Differential Revision: D58092090

fbshipit-source-id: a593be00464853a3fe179305efae5643d616573b
2024-06-07 08:23:45 -07:00
Edmond ChuiandFacebook GitHub Bot cc99e924ca add prefersFuseboxFrontend field to debugger_command (#44499)
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
2024-05-09 03:43:38 -07:00
Edmond ChuiandFacebook GitHub Bot 75dc42f4df extract param type (#44498)
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
2024-05-09 03:43:38 -07:00
Rob HoganandFacebook GitHub Bot a52bd7dd08 Inspector proxy: Rewrite 127.0.0.1 to localhost in source map URLs to support IPv4->IPv6 tunnels (#44092)
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
2024-04-16 03:00:13 -07:00
Rob HoganandFacebook GitHub Bot 704756352c Inspector proxy: Add ping/pong keepalive/heartbeat to debugger connection (#44086)
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
2024-04-15 08:55:05 -07:00
Alex HuntandFacebook GitHub Bot 0b23ff6631 Add capability flag to select rn_fusebox debugger frontend (#43688)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43688

Context: https://github.com/facebookexperimental/rn-chrome-devtools-frontend/pull/34.

Changelog: [Internal]

Reviewed By: EdmondChuiHW

Differential Revision: D55474522

fbshipit-source-id: 4f514dba228bfa4df41a0eb9687f2525cc32b2d7
2024-04-02 10:12:49 -07:00
Rob HoganandFacebook GitHub Bot e05319cc87 Inspector proxy: catch and report errors in device message handling (#43697)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43697

Currently, messages from a device are handled by async handlers added to a promise chain.

If a handler rejects, the end of the chain becomes a rejected promise, picked up only asynchronously by Metro's global `unhandledRejection` handler.

This triggers a warning from Node.js, and worse, prevents any `then()` callback chained by subsequent messages from being invoked at all.

Handlers *should* attempt to gracefully deal with errors (as we do with source map fetching errors, for example), but this diff adds a catch-all fallback for anything we might've missed (in this case, a frontend socket disconnecting while we're busy fetching a source map). Errors are caught and logged to EventReporter.

**To follow**: Gracefully handle socket disconnections while an async handler is working or queued.

Changelog:
[General][Fixed] Inspector proxy: prevent errors proxying a device message from blocking the handler queue or spamming logs.

Reviewed By: EdmondChuiHW

Differential Revision: D55482735

fbshipit-source-id: bb726218495e105f9cb4f723a1d110c9815abdef
2024-03-30 15:29:47 -07:00
Rob HoganandFacebook GitHub Bot fd1e5e7063 Inspector proxy: Fix "Buffer() is deprecated" warning (#43686)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43686

Under Node 20, the use of `new Buffer(string)` is deprecated and logs a warning. This replaces it with the recommended `Buffer.from(string)`.

Changelog:
[General][Fixed] FIx "Buffer() is deprecated" warning from debugger proxy.

Reviewed By: huntie

Differential Revision: D55472025

fbshipit-source-id: 8b5af9e2d7e026cbdf6aa68f71ff0f856fb164db
2024-03-28 15:17:02 -07:00
Rob HoganandFacebook GitHub Bot ac714b1c33 Inspector proxy: preserve ordering of messages from device (#43639)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43639

Currently, the `react-native/dev-middleware` inspector proxy intercepts `Debugger.scriptParsed` notifications from the target and replaces `sourceMapURL` with a data uri, via an async fetch from Metro. During this async fetch, other notifications from the debugger may pass through the proxy, which results in the frontend receiving them before `Debugger.scriptParsed`.

This reordering causes problems in breakpoint resolution and pausing, because `Debugger.breakpointResolved` and `Debugger.paused` events may reference `scriptId`s unknown to the frontend while the corresponding `Debugger.scriptParsed` is delayed.

In particular, breakpoint UI state and backend state can fall out of sync, and breakpoints hit may open to the incorrect source location.

This diff modifies the proxy to use a simple per-target promise queue to ensure messages are handled in the order they were received from the target.

Changelog:
[General][Fixed] Fix breakpoints opening to incorrect location or disappearing from debugger frontend UI.

Reviewed By: motiz88

Differential Revision: D55200617

fbshipit-source-id: 27c95f822266875ed668d0bf8a525da49554cafd
2024-03-25 13:02:00 -07:00
Cedric van PuttenandFacebook GitHub Bot 652c74104a fix(dev-middleware): create custom message handler for synthetic page (#43559)
Summary:
This is a follow-up bugfix for expo/expo#27425, related to:
 - https://github.com/facebook/react-native/issues/43291
 - https://github.com/facebook/react-native/issues/43307
 - https://github.com/facebook/react-native/issues/43310
 - https://github.com/facebook/react-native/issues/43364

The middleware API works as intended and can run our extended CDP events. Unfortunately, this only applies to an actual `Page` from the device, not for the `React Native Experimental (Improved Chrome Reloads)` synthetic / virtual page.

That's because the middleware instantiation gets aborted when the page can't be found in `this.#pages.get(pageId)`, which always returns `null` for this synthetic page.

## Changelog:

[GENERAL] [FIXED] Create custom message handler for synthetic page

Pull Request resolved: https://github.com/facebook/react-native/pull/43559

Test Plan: See added test case.

Reviewed By: motiz88

Differential Revision: D55129412

Pulled By: huntie

fbshipit-source-id: 9679d8fe68f3cb4104f4a042f93612b995baddc9
2024-03-20 08:59:46 -07:00
Cedric van PuttenandFacebook GitHub Bot 3f41fb5d5b feature(dev-middleware): add custom message handlers to extend CDP capabilities (#43291)
Summary:
This is a proposal for the `react-native/dev-middleware` package, to allow implementers to extend the CDP capabilities of the `InspectorProxy`. It's unfortunately needed until we can move to the native Hermes CDP layer.

At Expo, we extend the CDP capabilities of this `InspectorProxy` by injecting functionality on the device level. This proposed API does the same, but without having to overwrite internal functions of both the `InspectorProxy` and `InspectorDevice`.

A good example of this is the network inspector's capabilities. This currently works through the inspection proxy, and roughly like:
- Handle any incoming `Expo(Network.receivedResponseBody)` from the _**device**_, store it, and stop event from propagating
- Handle the incoming `Network.getResponseBody` from the _**debugger**_, return the data, and stop event from propagating.

This API brings back that capability in a more structured way.

## API:

```ts
import { createDevMiddleware } from 'react-native/dev-middleware';

const { middleware, websocketEndpoints } = createDevMiddleware({
  unstable_customInspectorMessageHandler: ({ page, deviceInfo, debuggerInfo }) => {
    // Do not enable handler for page other than "SOMETHING", or for vscode debugging
    // Can also include `page.capabilities` to determine if handler is required
    if (page.title !== 'SOMETHING' || debuggerInfo.userAgent?.includes('vscode')) {
      return null;
    }

    return {
      handleDeviceMessage(message) {
        if (message.type === 'CDP_MESSAGE') {
          // Do something and stop message from propagating with return `true`
          return true;
        }
      },
      handleDebuggerMessage(message) {
        if (message.type === 'CDP_MESSAGE') {
          // Do something and stop message from propagating with return `true`
          return true;
        }
      },
    };
  },
});
```

## Changelog:

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[GENERAL] [ADDED] - Add inspector proxy device message middleware API

Pull Request resolved: https://github.com/facebook/react-native/pull/43291

Test Plan: See added tests and code above

Reviewed By: huntie

Differential Revision: D54804503

Pulled By: motiz88

fbshipit-source-id: ae918dcd5b7e76d3fb31db4c84717567ae60fa96
2024-03-12 09:58:51 -07:00
Cedric van PuttenandFacebook GitHub Bot c62be78d99 feature(dev-middleware): use userAgent query parameter as fallback when header is unset (#43364)
Summary:
At Expo, we use [Expo Tools](https://github.com/expo/vscode-expo/blob/main/src/expoDebuggers.ts) to connect the [built-in vscode-js-debug](https://github.com/microsoft/vscode-js-debug) to Hermes.

Since there are a few differences in vscode vs chrome devtools, we need to enable a couple of modifications through the [`customMessageHandler` API](https://github.com/facebook/react-native/pull/43291). Unfortunately, vscode itself doesn't set the `user-agent` header when connecting to the inspector proxy. Becuase of that, we'd need a fallback to "manually" mark the debugger as being vscode ([we use this query parameter here](https://github.com/expo/vscode-expo/blob/main/src/expoDebuggers.ts#L208)).

This PR supports setting the `user-agent` through `?userAgent=` when the header is not set.

## Changelog:

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

[GENERAL] [ADDED] - Fallback to query parameter based `user-agent` when header is unset

Pull Request resolved: https://github.com/facebook/react-native/pull/43364

Test Plan:
- Install [Expo Tools](https://marketplace.visualstudio.com/items?itemName=expo.vscode-expo-tools)
- Start Metro with this change.
- Connect a device.
- Run the vscode command `"Expo: Debug Expo app ..."`
- Debugger should connect, and have it's user-agent marked as:
    `vscode/1.87.0 vscode-expo-tools/1.3.0`

Reviewed By: huntie

Differential Revision: D54804556

Pulled By: motiz88

fbshipit-source-id: 1ff558ba5350811ad042d08a713438e046759feb
2024-03-12 09:39:00 -07:00
Cedric van PuttenandFacebook GitHub Bot 3dee6d31b9 fix(dev-middleware): allow inspector proxy to fetch sourcemaps on lan connections (#43307)
Summary:
The inspector proxy is inlining source maps on `Debugger.scriptParsed` CDP events. The inlining prevents Chrome DevTools from downloading this remotely, as that's not supported in newer versions anymore.

The current implementation locks this inlining mechanism to just `localhost` and/or `127.0.0.1` addresses, making it incompatible with LAN or tunnel device connections.

This PR removes that limitation to allow source map inlining on these LAN and tunnel connections.

## Changelog:

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

[GENERAL][FIXED] Allow Inspector proxy to inline source maps on LAN connections

Pull Request resolved: https://github.com/facebook/react-native/pull/43307

Test Plan:
- See added test
- Start Metro and connect a device over LAN, open the chrome devtools

Reviewed By: huntie

Differential Revision: D54485247

Pulled By: robhogan

fbshipit-source-id: 6fcb0c6dd762d2f0a013497ba0a1126095b9130b
2024-03-11 03:48:49 -07:00
Cedric van PuttenandFacebook GitHub Bot f3ce7cd76e feature(dev-middleware): add inspector proxy nativeNetworkInspection target capabilty flag (#43310)
Summary:
This adds the `nativeNetworkInspection` target capability flag, to enable/disable the proxy-side network inspection handling.

## Changelog:

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

[GENERAL][ADDED] Add inspector proxy `nativeNetworkInspection` target capability flag

Pull Request resolved: https://github.com/facebook/react-native/pull/43310

Test Plan:
Once this lands, and is published through `react-native/dev-middleware`, we (Expo) can disable the proxy-side network inspection handling.

See https://github.com/expo/expo/pull/27425/commits/1a1b601a29fbc5766628238db7259121689f6cd6 on PR expo/expo#27425

Reviewed By: christophpurrer, motiz88

Differential Revision: D54486516

Pulled By: huntie

fbshipit-source-id: cc151349c816fb3866d3ec07af1a29a5f4ff9b00
2024-03-05 04:03:30 -08:00
Moti ZilbermanandFacebook GitHub Bot 4cfac8eea6 Restore Content-Length header in inspector-proxy JSON responses
Summary:
Changelog: [General][Fixed] Re-enable listing Hermes debugger targets in chrome://inspect, broken in 0.74 RC

Fixes https://github.com/facebook/react-native/issues/43259.

Reverts D52958725 and fixes the original `Content-Length` Unicode bug using a different approach.

Reviewed By: fabriziocucci

Differential Revision: D54409847

fbshipit-source-id: ed5bb464ab67f37535947646b124814d8bbf797c
2024-03-01 02:38:48 -08:00
Alex HuntandFacebook GitHub Bot 9ba56f6f5d Remove matching of Modern/Legacy page types (#42885)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42885

## Context

We're introducing the concept of **capability flags** to provide granular control of behaviours in the Inspector Proxy, to replace the recently added `type: 'Legacy' | 'Modern'` target switch.

A capability flag disables a specific feature/hack in the Inspector Proxy layer by indicating that the target supports one or more modern CDP features.

## This diff

Following D53355413, we're now able to remove the previous `type: 'Legacy' | 'Modern'` page concept, implemented in this diff.

Changelog: [Internal]

Reviewed By: robhogan

Differential Revision: D53358480

fbshipit-source-id: 62e53a1bd60760291ada3479121dfca9e1f6edbc
2024-02-06 04:09:53 -08:00
Alex HuntandFacebook GitHub Bot 74ea826464 Add nativeSourceCodeFetching capability flag (#42818)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42818

## Context

We're introducing the concept of **capability flags** to provide granular control of behaviours in the Inspector Proxy, to replace the recently added `type: 'Legacy' | 'Modern'` target switch.

A capability flag disables a specific feature/hack in the Inspector Proxy layer by indicating that the target supports one or more modern CDP features.

## This diff

Implements a second granular flag, `nativeSourceCodeFetching`, and adds tests for this.

Changelog: [Internal]

Reviewed By: robhogan

Differential Revision: D53352242

fbshipit-source-id: 94b62d84c731c903c5f99f8206d5c91bc501d030
2024-02-06 04:09:53 -08:00
Alex HuntandFacebook GitHub Bot 96db0fd1b6 Add support for target capability flags (#42817)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42817

## Context

We're introducing the concept of **capability flags** to provide granular control of behaviours in the Inspector Proxy, to replace the recently added `type: 'Legacy' | 'Modern'` target switch.

A capability flag disables a specific feature/hack in the Inspector Proxy layer by indicating that the target supports one or more modern CDP features.

## This diff

- Implements capability flags in `InspectorProxy`, via an optional `"capabilities"` key returned by a device's CDP server.
- Wires up an initial flag, `nativePageReloads`, to disable the legacy "React Native Experimental (Improved Chrome Reloads)" page and emulated page reload behaviour.

Changelog: [Internal]

Reviewed By: robhogan

Differential Revision: D53352244

fbshipit-source-id: 622fc6028174919b9bf776e3ac52724d97ca2734
2024-02-06 04:09:53 -08:00
Alex HuntandFacebook GitHub Bot 1429378bc0 Improve CDP Flow types (#42816)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42816

Some refactoring to formalise CDP protocol types (`protocol.js` is aligned with [Chrome's `protocol.d.ts` source](https://github.com/ChromeDevTools/devtools-protocol/blob/master/types/protocol.d.ts)), and to demarcate these from other types in the Inspector Proxy prototol.

Changelog: [Internal]

Reviewed By: robhogan

Differential Revision: D53352243

fbshipit-source-id: b2e413bdbd6164fd1b8adba6b5853ac107725b34
2024-02-06 04:09:53 -08:00
Krzysztof MagieraandFacebook GitHub Bot d16531e8a4 Remove Content-Length header from proxy inspector response (#42590)
Summary:
This change removes Content-Length header from proxy inspector response.

The presence of this header was resulting in the response being cropped under some circumstances because of erroneously calculated length.

The `Content-Length` header value represents the number of bytes in the response. In the code, `string.length` was used to calculate that value, but in JavaScript it gives the number of characters in a string instead of its size in bytes. Specifically, if there are some UTF characters in the string that occupy more than byte, there would be a mismatch in this size. This mismatch resulted in the response being cropped.

The easiest way to reproduce this problem is to set the simulator name to contain a two-byte UTF character.

This change works according to the HTTP spec, which states that when Content-Length is not present, the end of the response stream indicates the end of the response. Since in the code `response.end(data)` is use, it terminates the stream and hence there is no need to provide the length in the header.

## Changelog:

[GENERAL] [FIXED] - fix issue with debugger not working when device name contain two-byte UTF characters

Pull Request resolved: https://github.com/facebook/react-native/pull/42590

Test Plan:
1. Change your iOS simulator name to contain some two-byte UTF character (for example this one: "–")
2. Run metro and connect your app with it
3. Go to http://localhost:8081/json/list in your browser – see the response being marked invalid as it is cropped
4. Apply the change and see that the resulting JSON in the response is now correct
5. Open debugger workflow to confirm it sees the connected device

Reviewed By: robhogan

Differential Revision: D52958725

Pulled By: motiz88

fbshipit-source-id: 92c32893cbbf8552237585d824e4a44737fa3968
2024-01-22 09:26:44 -08:00
Moti ZilbermanandFacebook GitHub Bot 716c728c7a Distinguish between modern/legacy targets, conditionally disable hacks (#42303)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42303

Changelog: [Internal]

Adds a coarse-grained mechanism to `inspector-proxy` for distinguishing between legacy and modern debug targets. The guiding principles are:

1. `inspector-proxy` does not interfere in the CDP message stream between the debugger frontend and a modern target, or in the lifecycle of a target.
2. Legacy runtimes (current React Native, React Native Desktop, etc) that rely on `inspector-proxy`'s existing invasive semantics must continue to work seamlessly for now. We'll decide on the right time to deprecate/remove this legacy code in the future.

NOTE: This is an experimental addition to the proxy protocol that may be replaced at any time.

Reviewed By: hoxyq

Differential Revision: D50967795

fbshipit-source-id: bb9c39a8fe755ef3661e2c61507dd324d8dc8894
2024-01-18 07:52:51 -08:00
Moti ZilbermanandFacebook GitHub Bot 8ac08c8d18 Deduplicate pages by ID within each device (#42282)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42282

Changelog: [Internal] `inspector-proxy` now assumes each app will report pages with locally unique IDs.

In order to simplify some upcoming logic changes in `inspector-proxy`, in this diff we begin to enforce the assumption that each app ( = platform-specific implementation of `InspectorPackagerConnection`) assigns a locally unique ID to each inspector page. The inspector proxy will silently drop page descriptors that have conflicting IDs, and log a message to `debug()`.

NOTE: As an implementation detail, integrators may use `DEBUG=Metro:InspectorProxy` to see debug messages from `inspector-proxy`.

Reviewed By: huntie

Differential Revision: D50969752

fbshipit-source-id: a4e6faa91d97594fc5343ce4bee66233523cd175
2024-01-17 22:53:42 -08:00
Moti ZilbermanandFacebook GitHub Bot 7a69fe424d Migrate to private member syntax (#42299)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42299

Changelog: [Internal]

TSIA

Reviewed By: hoxyq

Differential Revision: D52798108

fbshipit-source-id: 9ba980b9fb439b1aeee3eb1b08535b95556d72bb
2024-01-17 08:03:34 -08:00
Ian ParkandFacebook GitHub Bot 46d304a101 Add 127.0.0.1 to allowlist for source map fetch
Summary:
Adds ipv4 localhost representation (127.0.0.1) to allowlisted hostnames for the inspector-proxy, to allow for proxied network requests fetching source maps.

Changelog:
[General][Fixed] Allow source map fetching in the inspector proxy from 127.0.0.1

Reviewed By: motiz88

Differential Revision: D52112849

fbshipit-source-id: e6d45fa2fdfd570b7ab4f36121ba615c9b06d442
2023-12-13 04:38:17 -08:00
Oskar KwaśniewskiandFacebook GitHub Bot 7d1a98c43b Lint source files, sort imports, remove unused imports (#41829)
Summary:
This PR lints source files using eslint. I've executed `yarn lint --fix` and also manually fixed some of eslint issues.

Before:

![CleanShot 2023-12-07 at 12 07 10@2x](https://github.com/facebook/react-native/assets/52801365/2b00cf23-e5a0-46b8-802f-adcb67224111)

After:

![CleanShot 2023-12-07 at 12 06 24@2x](https://github.com/facebook/react-native/assets/52801365/bb05b2c0-2b27-4f99-b7b4-cb47a51a3885)

## Changelog:

[GENERAL] [FIXED] - Lint source files, sort imports, remove unused ones

Pull Request resolved: https://github.com/facebook/react-native/pull/41829

Test Plan: CI Green

Reviewed By: christophpurrer

Differential Revision: D51979074

Pulled By: dmytrorykun

fbshipit-source-id: e11b90721e33f5e9949a0833e5f39fe7ba3d1067
2023-12-11 08:54:29 -08:00
Moti ZilbermanandFacebook GitHub Bot 1bcd28636f Handle source file / source map fetch errors correctly, add tests (#41342)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41342

While rewriting `Debugger.getScriptSource` messages to fetch code and source map over HTTP, we weren't checking the status code of the fetch calls. This diff fixes that and adds corresponding tests (as well as for the filesystem error case).

Changelog: [Internal]

Reviewed By: robhogan

Differential Revision: D51013054

fbshipit-source-id: 58e7bb9fcd6a3cf92329b43fb8a139093c80d305
2023-11-08 07:19:40 -08:00
Moti ZilbermanandFacebook GitHub Bot c6eccda2c7 Add tests for HTTPS-specific behaviour (#41341)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41341

* Extends `dev-middleware`'s test utilities to enable testing against an HTTPS server with a self-signed cert.
* Runs the CDP transport integration tests (D51002261) using both HTTP and HTTPS.
* Adds a test to explicitly cover the `ws=...` / `wss=...` variation in `devtoolsFrontendUrl` first introduced in D49158227.

Changelog: [Internal]

Reviewed By: blakef

Differential Revision: D51006835

fbshipit-source-id: df3db8cd865898248cd0d8f307f75949a7f313fd
2023-11-08 07:19:40 -08:00
Moti ZilbermanandFacebook GitHub Bot 38a878194b Add CDP transport tests (#41343)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41343

Changelog: [Internal]

Adds basic tests for two-way communication between a debugger (frontend) and a target (backend) using CDP over `inspector-proxy`.

Reviewed By: blakef

Differential Revision: D51002261

fbshipit-source-id: 44e571f89437c26e76ef6e6192b2bf6244665cf0
2023-11-08 07:19:40 -08:00
Moti ZilbermanandFacebook GitHub Bot 1602d42cec Clean up polling interval on device disconnection (#41331)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41331

`inspector-proxy`'s `Device` class currently leaks a `setInterval` handle. This is mostly harmless in current usage. In the test suite added up the stack, it shows up as a leak that prevents Jest from exiting cleanly, so let's clean it up properly.

Changelog: [Internal]

Reviewed By: blakef

Differential Revision: D51002263

fbshipit-source-id: ca36797ce1196aa049ceb3a8e96ee53d34893fdc
2023-11-08 07:19:40 -08:00
Moti ZilbermanandFacebook GitHub Bot d6e0bc714a Enable lint/sort-imports everywhere (#41334)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41334

TSIA.

Changelog: [Internal]

Reviewed By: robhogan

Differential Revision: D51025812

fbshipit-source-id: e10d437be775a6b80946483aa96460f34927f870
2023-11-06 12:59:38 -08:00