-
Notifications
You must be signed in to change notification settings - Fork 0
fix(react-native-inspector): Fix Promise method calls in FetchHook to use proper this binding #84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Check if client is React Native Inspector before handling Network.getResponseBody - For RN Inspector clients, forward request to native instead of handling in Rust server - For regular clients (e.g., Reactotron), continue handling in Rust server response_bodies - This allows RN Inspector to retrieve response body from C++ g_responseData via nativeGetNetworkResponseBody
…FetchHook - Fix text() and catch() calls to use callWithThis with proper this binding - Fix 'Cannot read property constructor of undefined' error - Ensure Promise chain works correctly with proper context binding This fixes the issue where response body extraction failed due to incorrect this context when calling Promise methods.
📊 Test Coverage Report - React Native InspectorSummary
File CoverageClick to expand
Generated by test coverage script |
📊 Test Coverage Report - ServerSummary
File CoverageClick to expand
Generated by Rust test coverage script (cargo-llvm-cov) |
Integration Tests1 tests 1 ✅ 21s ⏱️ Results for commit f974c8f. ♻️ This comment has been updated with latest results. |
📊 Test Results✅ Test Status: PASSED📸 ScreenshotsScreenshots are available as artifacts. Download them here. Artifact name: Screenshot files
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes Promise method calls in React Native Inspector's FetchHook by using proper this binding through callWithThis instead of call. It re-enables FetchHook to work alongside XHRHook for proper response body extraction from React Native fetch requests.
Changes:
- Changed Promise method calls (
clone(),text(),then(),catch()) to usecallWithThisfor proper context binding - Re-enabled FetchHook and implemented collaboration with XHRHook using global flags
- Added request forwarding logic in Rust server to differentiate React Native Inspector from other clients
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react-native-inspector/cpp/network/FetchHook.cpp | Fixed Promise method calls to use callWithThis, added fetch request tracking flags, improved response body extraction with pre-extraction fallback |
| packages/react-native-inspector/cpp/network/XHRHook.cpp | Added fetch request detection logic, prevented duplicate event transmission for fetch requests, improved collaboration with FetchHook |
| packages/react-native-inspector/cpp/network/NetworkGlobals.h | Added global variables for fetch request tracking (g_isFetchRequestActive, g_activeFetchRequestId, g_fetchRequestMutex) |
| packages/react-native-inspector/cpp/network/NetworkGlobals.cpp | Implemented global variables for fetch request tracking |
| packages/react-native-inspector/cpp/NetworkHook.cpp | Re-enabled FetchHook alongside XHRHook |
| crates/server/src/socket_server/devtools_handler.rs | Added logic to detect React Native Inspector clients and forward Network.getResponseBody requests to native code |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add documentation comment for g_activeFetchRequestId thread safety - Remove unnecessary dummy hookedFetch initialization - Fix race condition by keeping flag active until Promise resolution - Clear flag in Promise handlers (onFulfilled/onRejected) instead of immediately - Add check to ensure firstPromise is valid before returning - Remove duplicate requestId assignment in XHRHook
📱 Maestro Android Test Results✅ PASSED Maestro Android tests have completed. Check the workflow run for details. |
📱 Maestro iOS Test Results✅ PASSED Maestro iOS tests have completed. Check the workflow run for details. |
Problem
When extracting response bodies from network requests using the
fetchAPI in React Native, the following issues occurred:Cannot read property 'then' of undefined- Promise chain connection failureCannot read property 'constructor' of undefined- Context issue when callingtext()methodCannot read property '_noBody' of undefined- Response body extraction failureRoot Cause
fetchAPI uses XHR internally, but response body extraction requires processing at the Fetch API levelFetchHook, when calling Promise methods (text(),then(),catch()),call()was used instead ofcallWithThis(), causing incorrectthiscontext bindingSolution
Re-enable FetchHook
Use
callWithThisfor Promise method callstext()method: BindclonedResponseasthisthen()method: BindtextPromiseObjasthiscatch()method: BindfirstPromiseObjasthisImprove collaboration logic between Fetch and XHR
g_isFetchRequestActive,g_activeFetchRequestId)requestWillBeSenttransmission in XHRHook when it's a Fetch requestg_responseDataafter extraction in FetchHookresponseReceivedandloadingFinishedevents in XHRHookChanges
NetworkHook.cpp: Enable FetchHookFetchHook.cpp: UsecallWithThisfor Promise method calls and improve response body extraction logicXHRHook.cpp: Add Fetch request detection and collaboration logicNetworkGlobals.h/cpp: Add global variables for Fetch request trackingTesting
Network.getResponseBodyreturns the correct response bodyRelated Issues
Fixes the issue where response bodies from React Native network requests were not displayed in the network tab