Hermes direct debugging not working as InstanceImpl does not call DevSupportManager.StartInspector.

[nichamp]

Problem Description

When using Hermes with a default project and the configuration described in https://github.com/microsoft/react-native-windows/blob/main/docs/inspector.md the RNW instance does not appear in the targets list for edge://inspect (or chrome://inspect) so it isn't possible to debug code with the built-in Hermes engine.

Under a debugger, one can observe that m_devSettings->jsiEngineOverride == JSIEngineOverride::Default in InstanceImpl::InstanceImpl (and the destructor too). If one steps into the block with the debugger, the instance will appear in the devices list (perhaps on refreshing by clicking Configure and Done again). Even if one sets InstanceSettings.JSIEngineOverride to JSIEngine.Hermes, the issue still occurs as DevSettings.jsiEngineOverride does not appear to get set anywhere to reflect the InstanceSettings' value. So it appears that this obsolete value should be removed (wuthout breaking non-Hermes scenarios) or set to match so that debugging works.

Steps To Reproduce

1. Create a new RNW project using --useHermes
2. Modify App.cpp to set UseWebDebugger=false and UseDirectDebugger=true (or use Ctrl+Shift+D later)
3. Run npx react-native windows (or build+deploy with VS and start the metro server manually)
4. Open edge://inspect or chrome://inspect and on the Devices tab, click Configure to add localhost:8081
5. Observe that the target doesn't appear in the devices list.

Expected Results

Debug builds (which include HermesInspector.dll) of the app using the Hermes runtime and are connected to a Metro bundler server and have developer support and direct debugging enabled should appear in debugging tools automatically with no additional configuration beyond that descrbed in https://github.com/microsoft/react-native-windows/blob/main/docs/inspector.md

CLI version

6.3.1

Environment

System:
    OS: Windows 10 10.0.19042
    CPU: (24) x64 AMD Ryzen Threadripper PRO 3945WX 12-Cores
    Memory: 37.71 GB / 63.86 GB
  Binaries:
    Node: 16.13.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.15 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
    npm: 8.1.0 - C:\Program Files\nodejs\npm.CMD
    Watchman: Not Found
  SDKs:
    Android SDK: Not Found
    Windows SDK:
      AllowDevelopmentWithoutDevLicense: Enabled
      AllowAllTrustedApps: Enabled
      Versions: 10.0.18362.0, 10.0.19041.0, 10.0.22000.0
  IDEs:
    Android Studio: Not Found
    Visual Studio: 16.11.32106.194 (Visual Studio Enterprise 2019)
  Languages:
    Java: Not Found
  npmPackages:
    @react-native-community/cli: Not Found
    react: 17.0.2 => 17.0.2
    react-native: 0.66.4 => 0.66.4
    react-native-windows: 0.66.7 => 0.66.7
  npmGlobalPackages:
    *react-native*: Not Found

Target Platform Version

10.0.19041

Target Device(s)

No response

Visual Studio Version

Visual Studio 2019

Build Configuration

Debug

Snack, code example, screenshot, or link to a repository

No response

[nichamp]

An additional note is that even with a fix for this issue, source mapping (for TS or JS) doesn't seem to work correctly unless I add m_devSettings->inlineSourceMap = true; in the constructor too. DevTools will look for index.map and fail otherwise.

I think this either needs to be the default with Hermes or maybe to have it exposed with InstanceSettings (maybe to allow arbitrary URL modifications too if desired). If there is an additional configuration step that I am missing otherwise, it should probably be added to https://github.com/microsoft/react-native-windows/blob/main/docs/inspector.md

Some additional notes:

(1) Ctrl+Shift+D and then Enable Break on First Line does not work. The code will reload and run fully rather than waiting for DevTools to attach.

(2) Attempting to reload the code (e.g. Reload Javascript with Ctrl+Shift+D) after Dev Tools have attached will hang forever on reloading. Sometimes if one closes devtools first it will reload, but otherwise I have observed the app window hang or even a crash + metro exiting occur.

[NickGerleman]

cc @mganandraj (also on Teams, if you two need to chat offline)

[mganandraj]

I've created a PR to ensure we start the inspector: #9426

[mganandraj]

@nichamp Thanks for sharing the issues ..
I'm also seeing the issues with the source map. Need to investigate what changed recently.
And, there are known issues with breaking on first line.
Adding @tudorms

[nichamp]

@mganandraj I suggest adding inlineSourceMap = true with your fix since that does work for me or is there a reason we should avoid that?

Are there already issues for Enable Break on First Line and hangs on reload?

[acoates-ms]

I would expect Enable Break on First Line to "hang" on instance reload (which will happen when you initially check it in the devmenu), until you connect the debugger and continue. This is so that you can debug the startup JS that would get run before you have time to attach? Is that the hang you are seeing?

The sourcemap issue is likely an issue around your metro config -- It should be querying metro for your sourcemap. -- Using inline source map should work ok on dev builds, it puts the sourcemap information into your bundle file. But having the sourcemap be stored separately is I think the default in core react-native, and I see no reason why react-native-windows would want to be different than core for that.

[nichamp]

I would expect Enable Break on First Line to "hang" on instance reload (which will happen when you initially check it in the devmenu), until you connect the debugger and continue. This is so that you can debug the startup JS that would get run before you have time to attach? Is that the hang you are seeing?

Enable Break on First Line itself doesn't work so doesn't appear to be the problem. I am seeing the hang less frequently today than when I left the comment. It does seem to occur if one reloads while the debugger is attached but I thought I had observed it even after disconnecting. I guess these issues need some more concrete observations on my part to identify their repro conditions.

I have also observed the metro server sometimes crashing after trying to connect again after successfully reloading the bundle though I haven't yet looked at whether Metro has any issue tracking it:

The sourcemap issue is likely an issue around your metro config -- It should be querying metro for your sourcemap. -- Using inline source map should work ok on dev builds, it puts the sourcemap information into your bundle file. But having the sourcemap be stored separately is I think the default in core react-native, and I see no reason why react-native-windows would want to be different than core for that.

I was seeing this even with a vanilla project create with the init script. Which config parameters do you think should be added? As I mentioned earlier, it seems like devtools attempts to get the map (index.map) for the wrong file (index.js) instead of index.bundle, so having RNW retrieve the bundle with the query param for the inline map was the obvious solution that I found at the time to make it work. It would only be for dev builds using Hermes direct debugging with a Metro bundler since the server's inspector proxy is needed for debugging (and so debugging a bundle file is likely not an option I am assuming)

[mganandraj]

The issues with loading source maps when debugging playground app seems to be because of the source mapping URL embedded in the bundle ... This is how the embedded sourceMappingUrl verb looks like:

},683,[53,1,507],"..\@react-native-windows\tester\src\js\components\RNTesterEmptyBookmarksState.windows.js");
__r(0);
//# sourceMappingURL=//localhost:8081/Samples/rntester.map?platform=windows&dev=true&hot=true&inlineSourceMap=false
//# sourceURL=http://localhost:8081/Samples/rntester.bundle?platform=windows&dev=true&hot=true&inlineSourceMap=false

Note that "http:" is missing from the sourceMappingUrl in the bundle served by metro .. I need to dig into the metro code to see what is going wrong here ..

devtools gets the sourceMappingUrl from the "ScriptParsed" inspector event raised by hermes .. hermes gets the sourceMap path from the JavaScript bundle passed to it .. It is unlikely that devtools tries to get the map from wrong file ..
@nichamp Could you try to open the bundle in a text editor and check the sourceMappingUrl embedded in it ? When served from metro, you could view the bundle by typing the metro http endpoint, for e.g. "http://localhost:8081/Samples/rntester.bundle?platform=windows&dev=true&hot=true&inlineSourceMap=false" for playground

[nichamp]

@mganandraj with the vanilla RNW app I see the following as you found:

//# sourceMappingURL=//localhost:8081/index.map?platform=windows&dev=true&hot=false&inlineSourceMap=false
//# sourceURL=http://localhost:8081/index.bundle?platform=windows&dev=true&hot=false&inlineSourceMap=false

[acoates-ms]

@nichamp can you verify if making the change from facebook/metro#763 locally fixes your sourceMappingURL issue.

[nichamp]

@acoates-ms with that fix it does change the bundle to

//# sourceMappingURL=http://localhost:8081/index.map?platform=windows&dev=true&hot=true&inlineSourceMap=false
//# sourceURL=http://localhost:8081/index.bundle?platform=windows&dev=true&hot=true&inlineSourceMap=false

but source maps still fail:

if one then tries to use "Ctrl+P" to open a file anyway, search shows:

but clicking that also fails because of an invalid path:

[acoates-ms]

Interesting. I think that might be an edge/chrome issue. Although it's also happening in vscode.
The URL in the sourcemappingURL is correct, and if you navigate manually to it, metro returns the sourcemap correctly.

I've been using flipper to debug, and flipper does seem to work for me. Not sure what its doing special since I'm pretty sure its just an electron app, which I would have thought would work the same as edge/chrome.
https://fbflipper.com/docs/getting-started/index/

[RedMickey]

Although it's also happening in vscode.

Hi @acoates-ms, could you please describe a bit the incorrect behavior in VS Code if you use React Native Tools extension?

When implementing support of React Native Windows, we also faced this peculiarity (//# sourceMappingURL=//localhost:8081/Samples/rntester.map...) and added a special handler for this case.

[acoates-ms]

@RedMickey
So I was testing this out in the react-native-windows repo. My first attempt at using the vscode extension actually failed due to the extension not working with canary builds of react-native, which react-native-windows's main branch uses.
microsoft/vscode-react-native#1734 should fix that.

It does look like your special case logic in the extension does allow debugging using that extension. -- I'm hoping to get a fix in metro so that that special case is no longer needed (although I suppose it'll stay around for older versions of RN)

[nichamp]

Interesting. I think that might be an edge/chrome issue. Although it's also happening in vscode. The URL in the sourcemappingURL is correct, and if you navigate manually to it, metro returns the sourcemap correctly.

I've been using flipper to debug, and flipper does seem to work for me. Not sure what its doing special since I'm pretty sure its just an electron app, which I would have thought would work the same as edge/chrome. https://fbflipper.com/docs/getting-started/index/

It's good to know that Flipper (didn't for me, but I probably wasn't using it correctly) and the VS Code plugin will work, but do you anticipate finding a fix for the Edge/Chrome devtools too? In the interim, it may be worth updating the RNW documentation to suggest Flipper/vscode-react-native.

[tudorms]

@nichamp it looks like the debugger not starting issue is fixed in main and in 0.67.1; can you confirm it works on your side as well?

We may need a follow-up for the source maps not loading in Edge / Chrome but I suspect that behavior is coming from upstream (repros in Android/iOS).

[nichamp]

My local compile-time patching workarounds for this issue and the source mapping are what we are using presently. I suspect it isn't worth updating to 0.67.1+ (unless there are other helpful fixes too) until the source mapping issue is addressed too so I can remove the patching for the entire file. Otherwise, I would have to update the patch for the updated RNW to still fix source maps.

[tudorms]

Is that locally applying acoates' Metro PR or a different local patch for source maps?

[nichamp]

His fix didn't seem to work for edge://inspect as discussed above. I am using the fix I described from when I filed the issue by forcing inline source maps in the scenario of using Hermes direct debugging

[acoates-ms]

@tudorms , it looks like you forced inlineSourceMaps to be false for hermes way back in this commit 60e137c, with the comment "* Disable inline sourcemaps for Hermes (it doesn't handle them well)". Any idea if that still applies. I agree that generally for the dev scenario, if we can load a bundle with inline sourcemaps it avoids a class of issues with trying to find the sourcemaps. So if there isn't an issue with that, we should probably remove the line that turns off inline sourcemaps for hermes.

[tudorms]

Next build of 0.67 (0.67.3 due on Feb 28) will include the fix for InlineSourceMap.

[chrisglein]

Next build of 0.67 (0.67.3 due on Feb 28) will include the fix for InlineSourceMap.

@tudorms Yep, that's released now. Does that resolve this issue? @acoates-ms Metro PR is still active. What remains here and @nichamp are you unblocked?

[acoates-ms]

I think the InlineSourceMap fixes should unblock the scenario here.

[mganandraj]

Debugging into chrome, it turns out that the regression in loading sourcemap from url is introduced by this change : https://chromium-review.googlesource.com/c/chromium/src/+/3227594/
The change enforces that the resources be fetched from the storage sandbox identified by the web content in the chrome tab. In the standalone inspector instance that we launch through chrome:\inspect, the web contents is null (A null check is added in a subsequent CR).

node debugging still works because file:// scheme is still supported.

I don't understand how it works with flipper. Flipper creates an electron 'webview' to load the inspector. Accordint to documentation, electron webview creates a standalone sandbox chrome instance, and it is very much a chromium webview. I expect the same issue to happen within webview, but i can't debug there yet. It is possible that electron version used by flipper is old enough to not have the above change.
And, if not so, chromium webviews are undergoing similar refactorings and the issue might soon appear there too.

Another note is that this issue is not windows specific. It affects Android/iOS too.

It looks like, there are fundamental security issues with loading source maps from localhost urls. Going forward it looks like inline source maps may be only option.

@tudorms i remember there used to be memory management issues with Hermes when loading inline source maps. The memory allocations used to grow to GBs. I hope that is fixed now.

[tudorms]

Thanks for the detailed investigation. Yes, the issue with inline source maps was fixed in upstream a while back, but we never got around to removing the hack: facebook/hermes@093d15fb

[aeulitz]

It appears that as of commit 95638a in RNW main, the titular issue is fixed. Using the RNW playground apps (both Win32 and UWP and Chrome/Edge inspect, I verified that

1. the debug target is listed and that
2. source code can be loaded via Ctrl+P

Commit 95638a fixes an issue where re-loading packages does not remove debug targets from the Chrome/Edge inspect list. I did notice hangs in the playground apps and in vanilla project apps (generated via the CLI) when re-loading after connecting with a debugger to the previous instance. I suggest we create a dedicated issue for this.