feat(inspector): serve source maps to DevTools via Network.loadNetwor… by edusperoni · Pull Request #1969 · NativeScript/android · GitHub

…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

Summary by CodeRabbit

New Features
- Faster debugger messaging with immediate responses for some inspector requests.
- Improved DevTools integration: rewritten source-map URLs, on-device resource streaming (read/close), and network/file resource access via the debugger.
Bug Fixes
- Cleans up streamed resources on disconnect and improves pause behavior to avoid nested pause issues.

…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

📝 Walkthrough

Walkthrough

This 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.

Changes

Socket-Thread Fast Path and Resource Streaming

Layer / File(s)	Summary
Java socket-thread fast path and JNI bridge test-app/app/src/main/java/com/tns/AndroidJsV8Inspector.java, test-app/runtime/src/main/cpp/com_tns_AndroidJsV8Inspector.cpp	Java declares native handleMessageOnSocketThread(String message) and calls it in onMessage before enqueueing; JNI entry point safely forwards the message to C++ and returns JSON response or null for queued dispatch.
C++ API declarations and infrastructure headers test-app/runtime/src/main/cpp/JsV8InspectorClient.h, test-app/runtime/src/main/cpp/JsV8InspectorClient.cpp	Header adds <atomic>, <map>, <mutex> and declares handleMessageOnSocketThread, ResourceStream struct, and DevTools handler methods (HandleLoadNetworkResource, HandleIORead, HandleIOClose); implementation adds small includes and a JSON alias.
Source map rewriting and socket-thread message dispatch test-app/runtime/src/main/cpp/JsV8InspectorClient.cpp	Implements anonymous-namespace utilities for JSON/base64/source-map rewriting; handleMessageOnSocketThread parses messages and serves Network/IO domains with base64 chunking and protocol responses; manages nested pause state atomically, clears resource streams on disconnect, and rewrites outgoing notifications before sending to the Android bridge.

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

NativeScript/android#1967: Adds similar websocket “fast path” plumbing (JNI → JsV8InspectorClient) and related socket-thread inspector handling.

Poem

🐰 A nimble rabbit on the thread,

Hops bytes and source maps overhead,
Streams and pauses neatly queued,
Fast paths dance where data's viewed,
JNI bridges send the tune—debugging sped.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.83% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately summarizes the main objective: serving source maps to DevTools via Network.loadNetworkResource, which is the core feature added across all modified files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches 📝 Generate docstrings

Create stacked PR
Commit on current branch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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

📥 Commits

Reviewing files that changed from the base of the PR and between bfd7650 and f45830a.

📒 Files selected for processing (5)

test-app/app/src/main/java/com/tns/AndroidJsV8Inspector.java
test-app/runtime/src/main/cpp/JsV8InspectorClient.cpp
test-app/runtime/src/main/cpp/JsV8InspectorClient.h
test-app/runtime/src/main/cpp/com_tns_AndroidJsV8Inspector.cpp
test-app/runtime/src/main/cpp/third_party/json.hpp

… connections

🧹 Nitpick comments (1)

test-app/runtime/src/main/cpp/JsV8InspectorClient.cpp (1)
483-497: 💤 Low value

Consider adding defensive empty check in addVariants.

Accessing p[0] when p is empty is undefined behavior in C++. While the current call sites guarantee non-empty inputs (line 475 validates path, and percent_decode of 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

📥 Commits

Reviewing files that changed from the base of the PR and between f45830a and 7a6e3d9.

📒 Files selected for processing (1)

test-app/runtime/src/main/cpp/JsV8InspectorClient.cpp

coderabbitai Bot reviewed Jun 11, 2026

View reviewed changes

Comment thread test-app/runtime/src/main/cpp/JsV8InspectorClient.cpp

fix(inspector): clear resource streams on disconnect to prevent stale…

7a6e3d9

… connections

coderabbitai Bot reviewed Jun 11, 2026

View reviewed changes

rigor789 approved these changes Jun 12, 2026

View reviewed changes

rigor789 merged commit 55da2da into main Jun 12, 2026
9 of 10 checks passed

rigor789 deleted the feat/source-map-handling branch June 12, 2026 15:51

coderabbitai Bot mentioned this pull request Jun 12, 2026

feat(inspector): attach Chrome DevTools to Web Worker isolates #1973

Merged

feat(inspector): serve source maps to DevTools via Network.loadNetwor… by edusperoni · Pull Request #1969 · NativeScript/android · GitHub

Uh oh!

Uh oh!

feat(inspector): serve source maps to DevTools via Network.loadNetwor… - #1969

feat(inspector): serve source maps to DevTools via Network.loadNetwor…#1969
rigor789 merged 2 commits into
mainfrom
feat/source-map-handling

edusperoni commented Jun 11, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

Uh oh!

coderabbitai Bot commented Jun 11, 2026 •
edited
Loading

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

feat(inspector): serve source maps to DevTools via Network.loadNetwor… by edusperoni · Pull Request #1969 · NativeScript/android · GitHub

Uh oh!

Uh oh!

feat(inspector): serve source maps to DevTools via Network.loadNetwor… - #1969

feat(inspector): serve source maps to DevTools via Network.loadNetwor…#1969rigor789 merged 2 commits intomainNativeScript/android:mainfrom feat/source-map-handlingNativeScript/android:feat/source-map-handlingCopy head branch name to clipboard

Conversation

edusperoni commented Jun 11, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

feat(inspector): serve source maps to DevTools via Network.loadNetwor…#1969
rigor789 merged 2 commits into
mainfrom
feat/source-map-handling

edusperoni commented Jun 11, 2026 •
edited by coderabbitai Bot
Loading

coderabbitai Bot commented Jun 11, 2026 •
edited
Loading