Commit Graph

125 Commits

Author SHA1 Message Date
Rob Hogan 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 Hunt 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 Hogan 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 Hogan 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
Edmond Chui af309127a4 Add support for launch ID (#43585)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43585

Changelog: [internal]

Related PR: https://github.com/facebookexperimental/rn-chrome-devtools-frontend/pull/27

Reviewed By: hoxyq

Differential Revision: D55164646

fbshipit-source-id: 0f25f150603a24654020093697e76d85d0d8cc02
2024-03-28 13:48:30 -07:00
Rob Hogan 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 Putten 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
Moti Zilberman 1ae379c6ec Migrate back to chromium-edge-launcher since Windows fix was merged (#43562)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43562

A resubmission of D55013623 (Pull Request resolved: https://github.com/facebook/react-native/pull/43524) with a fix for the internal `.flowconfig` issue that got the initial diff reverted.

 ---

The [Windows fix](https://github.com/cezaraugusto/chromium-edge-launcher/pull/1) was merged and published. We no longer need to use the fork.

## Changelog:

[INTERNAL] [FIXED] - Fix experimental debugger launch flow with Edge on Windows

Reviewed By: NickGerleman

Differential Revision: D55087731

fbshipit-source-id: 6fd28a32447ad07dacdf6cd77390e18489fd6cfb
2024-03-20 03:55:35 -07:00
Wendi Du ff5e1a605a Revert D55013623: Migrate back to chromium-edge-launcher since Windows fix was merged
Differential Revision:
D55013623

Original commit changeset: bff2aa2801dd

Original Phabricator Diff: D55013623

fbshipit-source-id: d7f19068327b5cf427750fbcae01d3c10f9c89b5
2024-03-18 14:29:02 -07:00
Tommy Nguyen 43ffce9fb3 Migrate back to chromium-edge-launcher since Windows fix was merged (#43524)
Summary:
The [Windows fix](https://github.com/cezaraugusto/chromium-edge-launcher/pull/1) was merged and published. We no longer need to use the fork.

## Changelog:

[INTERNAL] [FIXED] - Fix experimental debugger launch flow with Edge on Windows

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

Test Plan: n/a

Reviewed By: robhogan

Differential Revision: D55013623

Pulled By: motiz88

fbshipit-source-id: bff2aa2801dd0dcdd6975dca0a2ec2aa9864ff6f
2024-03-18 10:45:04 -07:00
Cedric van Putten 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 Putten 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 Putten 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 Putten 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 Zilberman 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
Moti Zilberman 451fffbb59 Use host-relative WebSocket URLs when launching new debugger
Summary:
Changelog: [Internal]

Uses the capability introduced in https://github.com/facebookexperimental/rn-chrome-devtools-frontend/pull/4 to avoid repeating the dev server's host:port in the `ws` / `wss` parameter we pass to the Chrome DevTools frontend. This gives us more flexibility to handle port forwarding and redirects outside of `dev-middleware`. This is mostly useful in Meta's internal VS Code remoting setup, but this particular change should work equally well in open source.

Reviewed By: huntie

Differential Revision: D54107316

fbshipit-source-id: 68d4dbf4849ca431274bfb0dc8a4e05981bdd5b5
2024-02-26 05:33:18 -08:00
Edmond Chui 032208d81a Update tests for URL assertions
Summary:
Changelog: [Internal]

Update tests to be more resilient against prod changes; and make it easier to read the actual vs expected values upon failure.

Reviewed By: motiz88

Differential Revision: D53762409

fbshipit-source-id: d627d5041295f645ed00aa5d0645419a9ac4a7f8
2024-02-14 10:37:13 -08:00
Edmond Chui 496724fbdb Remove URI encoding
Summary:
Changelog: [Internal]

Fixed double-encoding for the websocket url.
`URLSearchParams` already encode the values, passing a pre-encoded `encodeUriComponent` string will cause it to double-encode, making the value unreadable when decoding once.

Missed these lines while splitting the initial diff stack.
Added tests now.

Reviewed By: motiz88

Differential Revision: D53721568

fbshipit-source-id: cfaaa7eb50c40364c904e9ffc5698201df8ab22b
2024-02-14 05:20:46 -08:00
Edmond Chui 7e3e7684b1 refactor URL construction (#42948)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42948

Changelog: [Internal]

Refactor URL construction for DevTools.

Next diffs in the stack will add additional URL query params.

Support for both absolute and relative `devServerUrl`s maintained.

Reviewed By: hoxyq

Differential Revision: D53620915

fbshipit-source-id: 4a64c49c3479ede2add9f39a24448787d8609172
2024-02-12 04:08:14 -08:00
Alex Hunt 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 Hunt 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 Hunt 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 Hunt 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
Rob Hogan cfc0ba04a1 Re-enable InspectorProxy* Jest tests (#42682)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42682

Since D53125777, these should be safe to re-enable

Changelog:
[Internal] Re-enable dev-middleware InspectorProxy* tests

Reviewed By: huntie

Differential Revision: D53125778

fbshipit-source-id: 38911b5d6f424b46d33bb4dace18c892dc5c8c1a
2024-01-27 06:49:15 -08:00
Krzysztof Magiera 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 Zilberman ca2dde5906 Support launching experimental debugger for modern CDP targets (#42302)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42302

Changelog: [Internal][Added] Support launching experimental debugger frontend for CDP targets marked as "modern"

See the definition of "modern" targets in D50967795.

Reviewed By: hoxyq

Differential Revision: D52786332

fbshipit-source-id: 13718e9ddf3ec050049ef7ec9a77f6cf1a7f82ee
2024-01-18 07:52:51 -08:00
Moti Zilberman 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 Zilberman 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 Zilberman 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 Park 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śniewski 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
Cedric van Putten 8ef807bfb2 feature(dev-middleware): add enableNetworkInspector experiment (#41787)
Summary:
This enables the network panel/inspector by passing the `unstable_enableNetworkPanel=true` to the React Native JS Inspector. (See https://github.com/facebookexperimental/rn-chrome-devtools-frontend/pull/2)

By setting this inside the `experiments`, we can enable/disable network related CDP handlers within the proxy.

## Changelog:

[GENERAL] [ADDED] - Add `enableNetworkInspector` experiment to enable Network panel and CDP handlers in inspector proxy

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

Test Plan: TBD, will provide a repository using an Expo canary / RN 0.73.0-rc release.

Reviewed By: NickGerleman

Differential Revision: D51811892

Pulled By: huntie

fbshipit-source-id: 541d96b6f0735104a4050a24a152e1158871ed1d
2023-12-07 08:30:03 -08:00
Gijs Weterings 73411f4e08 Back out "Restore InspectorProxy tests - fix Windows by Babel-ignoring node_modules consistently"
Summary: Changelog: Internal

Reviewed By: gsathya

Differential Revision: D51447333

fbshipit-source-id: 56eb839671ca62269d59d48fd3c83df4f42a5111
2023-11-18 02:04:44 -08:00
Rob Hogan 59a2282b29 Restore InspectorProxy tests - fix Windows by Babel-ignoring node_modules consistently (#41526)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41526

CI failures in Windows JS tests recently (https://github.com/facebook/react-native/pull/41463) were caused by the triggering of Babel registration during tests, due to an import of `packages/dev-middleware` (index), breaking subsequent transformation of other tests.

## Root cause
Example of a problematic import:
https://github.com/facebook/react-native/blob/a5d8ea4579c630af1e4e0fe1d99ad9dc0915df86/packages/dev-middleware/src/__tests__/ServerUtils.js#L15

..which triggers a Babel registration:
https://github.com/facebook/react-native/blob/a5d8ea4579c630af1e4e0fe1d99ad9dc0915df86/packages/dev-middleware/src/index.js#L16-L18

That registration behaves differently on Windows due to the `ignore: [/\/node_modules\/\]`, which doesn't match against Windows path separators - Babel matches against system separators.

In particular, this changed whether `node_modules/flow-parser` was transformed when loading the RN Babel transformer. Transforming this file causes a `console.warn` from Babel due to its size:
> [BABEL] Note: The code generator has deoptimised the styling of /Users/robhogan/workspace/react-native/node_modules/flow-parser/flow_parser.js as it exceeds the max of 500KB.

This throws due to our setup:
https://github.com/facebook/react-native/blob/a5d8ea4579c630af1e4e0fe1d99ad9dc0915df86/packages/react-native/jest/local-setup.js#L27

This all manifests as the first test following a Babel registration (within the same Jest worker) that requires the RN Babel transformer throwing during script transformation.

## This change
This is the minimally disruptive change that makes Babel registration behaviour consistent between Windows and other platforms. The more durable solution here would be *not* to rely on any Babel registration for Jest, which has its own `ScriptTransformer` mechanism for running code from source. Given the fragile way our internal+OSS Babel set up hangs together that's a higher-risk change, so I'll follow up separately.

Changelog: [Internal]

Reviewed By: huntie

Differential Revision: D51424802

fbshipit-source-id: 8b733c0c159ee84690aef04abced682d126c6d27
2023-11-17 08:53:14 -08:00
Moti Zilberman 560e0f0005 Migrate to @rnx-kit/chromium-edge-launcher for Windows fix (#41367)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41367

Andrew Coates graciously published the Windows Edge launcher fix from https://github.com/cezaraugusto/chromium-edge-launcher/pull/1 as a new package (https://github.com/microsoft/rnx-kit/pull/2796), so let's pull that into `dev-middleware`.

Changelog: [Internal] - Fix experimental debugger launch flow with Edge on Windows

Reviewed By: robhogan

Differential Revision: D51086297

fbshipit-source-id: 3a8db351f71eb31a9609c987cdb4dc66f24f9403
2023-11-15 00:18:10 -08:00
Gijs Weterings 453e5c0d09 Temporarily disable InspectorProxy.* tests to unblock CI (#41463)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41463

S378983 Circle CI tests have been red for 5 days. There's a test setup issue somewhere in this test suite, until motiz88 can determine where exactly, let's disable them

T169943794 filed to follow up

Changelog: Internal

Reviewed By: cipolleschi

Differential Revision: D51271630

fbshipit-source-id: 7dbc61bb4c8df0d5360ba239a1f00c4270a691f3
2023-11-13 13:24:47 -08:00
Moti Zilberman 4c0d20dfbf Fix platform sensitivity in inspector-proxy tests (#41439)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41439

TSIA - should fix the [Windows test failure](https://app.circleci.com/pipelines/github/facebook/react-native/36174/workflows/055cabb6-cb41-4741-812a-59b06fc7c9b5/jobs/1200335/parallel-runs/0/steps/0-115?fbclid=IwAR38F8tE-831Rpe5J_KeFOmA5DARLWWYIAiLIgkXOLdEfMmSeaVOIGgPq5U) we saw on CircleCI.

Changelog: [Internal]

Reviewed By: robhogan

Differential Revision: D51254843

fbshipit-source-id: d45682c460d54b3d76d3a02416b75f0c11313fd7
2023-11-13 05:12:00 -08:00
Gabriel Donadel 1a61afddf7 Expose unstable_InspectorProxy and unstable_Device from dev-middleware (#41370)
Summary:
Recently, both `metro-inspector-proxy`(https://github.com/facebook/react-native/pull/39045) and `react-native-community/cli-plugin-metro`(https://github.com/facebook/react-native/pull/38795) were moved to this repo and in the process of moving these packages, the `exports` field inside package.json was added, only exporting the `index.js` file.

The problem is that Expo CLI (and possibly other community packages) rely on functions and classes that are not exported in the `index.js` file, e.g. Importing the InspectorProxy class from `react-native/dev-middleware/dist/inspector-proxy/InspectorProxy`. Normally this wouldn't be a problem and we would just import from `dist/` but due to the `exports` field, attempting to import from any other file not specified on this field will result in a `ERR_PACKAGE_PATH_NOT_EXPORTED` error.

As a short-term fix, we should create `unstable_`-prefixed exports of individual features Expo currently depends on.

## Changelog:

[INTERNAL] [CHANGED] - Expose unstable_InspectorProxy and unstable_Device from `react-native/dev-middleware`

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

Test Plan: N / A

Reviewed By: robhogan

Differential Revision: D51163134

Pulled By: blakef

fbshipit-source-id: e67adaedc4fc64131e4c9dd8383c9877b8202283
2023-11-10 04:45:51 -08:00
Moti Zilberman 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 Zilberman 4eb3e300a4 Add tests for reload message injection (#41340)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41340

Tests for D17100911 and D14503522.

Changelog: [Internal]

Reviewed By: robhogan

Differential Revision: D51013053

fbshipit-source-id: 65aba42e21fad891e458647c8421cd316518ec3c
2023-11-08 07:19:40 -08:00
Moti Zilberman 246657214d Add tests for Debugger.getScriptSource rewriting (#41338)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41338

Tests for D21401320 and D39629274.

Changelog: [Internal]

Reviewed By: robhogan

Differential Revision: D51013052

fbshipit-source-id: af25935d1d7fff90112720888b3728ce195ac5d1
2023-11-08 07:19:40 -08:00
Moti Zilberman 3afadef6d6 Add tests for source map fetching and URL rewriting hacks (#41339)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41339

Tests for D42973408, D14800976, and D15116579.

Changelog: [Internal]

Reviewed By: blakef

Differential Revision: D51013055

fbshipit-source-id: cff8a9d1dd22b642117594da16e6b74cf679aa34
2023-11-08 07:19:40 -08:00
Moti Zilberman 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 Zilberman c85b2da1e7 Add minimal test for "Improved Chrome Reloads" synthetic page (#41335)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41335

`inspector-proxy` has special behaviour to allow a debugger connection to persist across app reloads.

In the React Native runtime, a reload is modelled as the creation of an entirely new "page" with its own ID. To insulate the debugger from this detail, the proxy advertises a separate, synthetic page on each device, with ID `-1`, that always maps to the latest React Native page reported by that device.

Here we test the message forwarding part of this functionality. The proxy also injects CDP messages (in both directions) as part of simulating a reload, but that will be tested in a separate diff.

Changelog: [Internal]

Reviewed By: blakef

Differential Revision: D51002262

fbshipit-source-id: 296135177321a511ebbe7d9696e4e7a61275aa32
2023-11-08 07:19:40 -08:00
Moti Zilberman 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 Zilberman 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 Zilberman 0ea72807a6 Add tests for inspector-proxy's HTTP API (#41314)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41314

Changelog: [Internal]

Adds the beginning of a test suite for `inspector-proxy`. For maintainability, we only test functionality exposed from the `dev-middleware` boundary rather than instantiating `InspectorProxy` directly.

In this diff, the test coverage is far from complete, but this is a first stab at covering some basics. `InspectorProxyHttpApi-test` exercises the HTTP GET endpoints (`/json/list` and `/json/version`) as well as some device registration logic through the `/inspector/device` WebSocket. Some reusable helpers for server setup and device mocking are included in separate files.

As an overall strategy, I'm planning to add multiple test files that share helpers between them, not build out one massive test file with all the helpers inline. There will likely be some verbose tests when we start covering debugger-to-device communication, and I want to keep them as readable as possible.

Reviewed By: blakef

Differential Revision: D50980467

fbshipit-source-id: 962dae5a380451d6dac57eac23c4436550a39cf8
2023-11-08 07:19:40 -08:00
Moti Zilberman 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
Moti Zilberman 1d6b0f1420 Use exact, read-only types for protocol data structures (#41315)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41315

TSIA

Changelog: [Internal]

Reviewed By: hoxyq

Differential Revision: D50980466

fbshipit-source-id: 1f289b868f5c735396fe07899c558e417a4ee2ff
2023-11-06 11:01:02 -08:00
Moti Zilberman 7009634d38 Enable launching debugger by device ID (#41154)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41154

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

Building on byCedric's approach in https://github.com/facebook/metro/pull/991, adds support for passing a `device=...` argument to `/open-debugger` for more precise targeting.

Changelog: [Internal]

 ---

## Note on what "device" means in this context

In `dev-middleware` / `inspector-proxy`, "device" is something of a misnomer. It refers to a *logical device* containing one or more *pages*. In React Native, each app process forms its own logical device in which individual VMs register themselves as pages. An instance of `inspector-proxy` connects one or more *debuggers* (frontends) to one or more logical devices (one frontend to one page on one device).

The intent of the logical device ID is to help with target discovery and especially *re*discovery - to reduce the number of times users need to explicitly close and restart the debugger frontend (e.g. after an app crash).

If provided, the logical device ID:
1. SHOULD be stable for the current combination of physical device (or emulator instance) and app.
2. SHOULD be stable across installs/launches of the same app on the same device (or emulator instance), though it MAY be user-resettable (so as to not require any special privacy permissions).
3. MUST be unique across different apps on the same physical device (or emulator).
4. MUST be unique across physical devices (or emulators).
5. MUST be unique for each concurrent *instance* of the same app on the same physical device (or emulator).

NOTE: The uniqueness requirements are stronger (MUST) than the stability requirements (SHOULD). In particular, on platforms that allow multiple instances of the same app to run concurrently, requirements 1 and/or 2 MAY be violated in order to meet requirement 5. This will be relevant, for example, on desktop platforms.

In an upcoming diff, we will pass device IDs meeting these criteria from both iOS and Android.

Reviewed By: huntie, blakef

Differential Revision: D49954920

fbshipit-source-id: 45f2b50765dece34cbb93fa32abcdf3b0522391c
2023-10-23 05:50:36 -07:00