feat(inspector): serve source maps to DevTools via Network.loadNetwor…#1969
Merged
Conversation
…kResource Chrome DevTools no longer fetches external source maps itself when debugging remote targets: it issues Network.loadNetworkResource to the target and reads the result back through IO.read/IO.close. None of these embedder-side CDP domains are implemented by V8's inspector, so external source maps failed and apps had to fall back to bloated inline-source-map builds. - Handle Network.loadNetworkResource natively: resolve the URL back to a file on disk and reply with a stream handle (success:false + net::ERR_FILE_NOT_FOUND when missing). - Implement IO.read (1MB base64 chunks; eof only on a final empty read, since the frontend discards data accompanying eof) and IO.close. - Reply with a JSON-RPC error for unsupported schemes (e.g. https) so DevTools keeps its existing fallback of fetching from the host. - Rewrite sourceMapURL in outgoing Debugger.scriptParsed / Debugger.scriptFailedToParse events from relative/file:// URLs to a custom nsruntime:// scheme. DevTools hard-excludes file:, data: and devtools: URLs from loading through the target, so without the rewrite it would never send Network.loadNetworkResource and instead try (and fail) to read device files from the host machine. data: and http(s) URLs are left untouched, keeping inline source maps working. - Allow opting out via nativescript.config.ts: android.disableSourceMapURLRewrite (or the same key at the top level). - Serve these messages on the websocket read thread (new native handleMessageOnSocketThread), since the main-thread queue is unavailable exactly when DevTools needs source maps: the pause loop bypasses dispatchMessage and a busy isolate never drains the queue. The handler is V8-free and returns the response for Java to send on the receiving socket. - Make Debugger.pause interrupt busy JS via Isolate::RequestInterrupt, skipped while already paused in the nested loop to avoid a spurious re-pause after resume. - Vendor nlohmann/json v3.12.0 (third_party/json.hpp, header-only) for CDP message handling outside V8. Ports NativeScript/ios#385 and NativeScript/ios#378 to Android. Refs: nodejs/node#58077
📝 WalkthroughWalkthroughThis PR extends the V8 inspector with a websocket-thread fast path that parses DevTools protocol messages on the socket thread (Network.loadNetworkResource, IO.read/close, Debugger.pause fast-path), bridges Java → JNI → C++, streams resources via in-memory ResourceStreams, rewrites source-map URLs to nsruntime://, and clears streams on disconnect. ChangesSocket-Thread Fast Path and Resource Streaming
Sequence Diagram(s)sequenceDiagram
participant JsV8InspectorWebSocket
participant handleMessageOnSocketThread
participant MessageParser
participant NetworkHandler as Network.loadNetworkResource
participant IOHandler as IO.read/IO.close
participant DebuggerHandler as Debugger.pause
JsV8InspectorWebSocket->>handleMessageOnSocketThread: message
handleMessageOnSocketThread->>MessageParser: parse protocol message
alt Network domain
MessageParser->>NetworkHandler: load resource URL
NetworkHandler->>handleMessageOnSocketThread: file content + base64 chunks
else IO domain
MessageParser->>IOHandler: read handle or close handle
IOHandler->>handleMessageOnSocketThread: stream chunk or close response
else Debugger.pause
MessageParser->>DebuggerHandler: RequestInterrupt if not in nested loop
DebuggerHandler->>handleMessageOnSocketThread: null (queued for dispatch)
else Other
MessageParser->>handleMessageOnSocketThread: null (queued for dispatch)
end
handleMessageOnSocketThread->>JsV8InspectorWebSocket: response string or null
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test-app/runtime/src/main/cpp/JsV8InspectorClient.cpp`:
- Around line 511-529: The code stores full file content in resourceStreams_
(handle created using lastStreamId_) but never removes those entries if the
DevTools session/socket drops, causing memory leaks; fix by tracking resource
stream handles per session (e.g., add a map from sessionId to vector of handles
and push the created handle in the block that sets resourceStreams_) and then
remove/erase those handles from resourceStreams_ during session teardown (the
same place that handles IO.close logic or session cleanup is performed, e.g., in
the method that closes the session or in IO.close), ensuring lastStreamId_ usage
remains unchanged; reference resourceStreams_, lastStreamId_, the handle
creation block, IO.close and FinishResponse to locate where to add the
per-session handle tracking and where to purge them on teardown.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7221c004-286f-47a0-8160-cea57404d3d4
📒 Files selected for processing (5)
test-app/app/src/main/java/com/tns/AndroidJsV8Inspector.javatest-app/runtime/src/main/cpp/JsV8InspectorClient.cpptest-app/runtime/src/main/cpp/JsV8InspectorClient.htest-app/runtime/src/main/cpp/com_tns_AndroidJsV8Inspector.cpptest-app/runtime/src/main/cpp/third_party/json.hpp
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test-app/runtime/src/main/cpp/JsV8InspectorClient.cpp (1)
483-497: 💤 Low valueConsider adding defensive empty check in
addVariants.Accessing
p[0]whenpis empty is undefined behavior in C++. While the current call sites guarantee non-empty inputs (line 475 validatespath, andpercent_decodeof a non-empty string shouldn't return empty), a defensive check would make the code more robust against future changes.🛡️ Optional defensive fix
auto addVariants = [&addCandidate](const std::string& p) { + if (p.empty()) return; addCandidate(p); if (p[0] != '/') { addCandidate(Constants::APP_ROOT_FOLDER_PATH + p); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test-app/runtime/src/main/cpp/JsV8InspectorClient.cpp` around lines 483 - 497, The lambda addVariants reads p[0] without guarding against an empty string; add a defensive check at the top of the lambda (in JsV8InspectorClient.cpp inside the addVariants lambda) to return early if p.empty() so addCandidate and the subsequent path manipulations (Constants::APP_ROOT_FOLDER_PATH concatenation and kDataData/kDataUser0 rfind/substr logic) never access p[0] or call substr on an empty input.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test-app/runtime/src/main/cpp/JsV8InspectorClient.cpp`:
- Around line 483-497: The lambda addVariants reads p[0] without guarding
against an empty string; add a defensive check at the top of the lambda (in
JsV8InspectorClient.cpp inside the addVariants lambda) to return early if
p.empty() so addCandidate and the subsequent path manipulations
(Constants::APP_ROOT_FOLDER_PATH concatenation and kDataData/kDataUser0
rfind/substr logic) never access p[0] or call substr on an empty input.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 560fa69e-e164-4711-9bfe-d41ef918f155
📒 Files selected for processing (1)
test-app/runtime/src/main/cpp/JsV8InspectorClient.cpp
rigor789
approved these changes
Jun 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…kResource
Chrome DevTools no longer fetches external source maps itself when debugging remote targets: it issues Network.loadNetworkResource to the target and reads the result back through IO.read/IO.close. None of these embedder-side CDP domains are implemented by V8's inspector, so external source maps failed and apps had to fall back to bloated inline-source-map builds.
Ports NativeScript/ios#385 and NativeScript/ios#378 to Android.
Refs: nodejs/node#58077
Summary by CodeRabbit