[dev-server][dev-launcher] inspector network on ios #21265
Merged
Conversation
This file contains 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
ENG-7464 [network][dev-launcher][iOS] Intercept NSURLSession requests and send to metro-inspector-proxy
By using NSURLSessionConfiguration, we could possibly intercept all network requests sending through NSURLSession: https://blog.codavel.com/how-to-intercept-http-requests-on-an-ios-app Then to send CDP network events through metro-inspector-proxy to Chrome DevTools: https://chromedevtools.github.io/devtools-protocol/tot/Network/ ENG-7492 [react-native][ios] add sendWrappedEventToAllRemoteConnections to send CDP events
Currently react-native on iOS doesn't expose an interface for us to send events to metro-inspector-proxy. We should either proposing a PR for upstream or hack it. |
expo-bot
added
bot: needs changes
|
Hi there! 👋 I'm a bot whose goal is to ensure your contributions meet our guidelines. I've found some issues in your pull request that should be addressed (click on them for more details) 👇
|
|
@Kudo I think we are missing two important protocol messages, the It should probably work without them, but it might be nice to implement them to not over-send protocol events that aren't useful. 😄 But, I'm not 100% convinced the chrome debugger is sending these though. |
|
@byCedric devtools will sent the event |
|
Hi there! 👋 I'm a bot whose goal is to ensure your contributions meet our guidelines. I've found some issues in your pull request that should be addressed (click on them for more details) 👇
|
Kudo
force-pushed
the
@kudo/eng-7464-networkdev-launcherios-intercept
branch
from
5558c79
to
662f085
Compare
|
Hi there! 👋 I'm a bot whose goal is to ensure your contributions meet our guidelines. I've found some issues in your pull request that should be addressed (click on them for more details) 👇
|
3 tasks
5 tasks
lukmccall
approved these changes
Mar 1, 2023
byCedric
added a commit
that referenced
this pull request
Mar 8, 2023
…oxy` (#21449) # Why Fixes ENG-7467 Related #21265 This is an initial draft to extend the CDP functionality of `metro-inspector-proxy`. # How The implementation is slightly wonky around the `ExpoInspectorDevice`. We want to reuse as much as possible from `metro-inspector-proxy`, but we need to add stateful data per device. In order to achieve that, we generate a new class type, based on the user's installed `metro-inspector-proxy`. This makes everything less readable but should include future updates in these classes. As for the `ExpoInspectorProxy`, to avoid having to do the same thing, we just wrap the whole inspector class and reuse what we can. The device map is "linked" within the original inspector proxy instance, making the data available to all methods that need it. # Test Plan Enable this feature with `EXPO_USE_CUSTOM_INSPECTOR_PROXY=1` - [x] See tests for the actual CDP events we handle. - [ ] See tests on the "bootstrapping code" to create the inspector and devices. # Checklist <!-- Please check the appropriate items below if they apply to your diff. This is required for changes to Expo modules. --> - [ ] Documentation is up to date to reflect these changes (eg: https://docs.expo.dev and README.md). - [ ] Conforms with the [Documentation Writing Style Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md) - [ ] This diff will work correctly for `expo prebuild` & EAS Build (eg: updated a module plugin). --------- Co-authored-by: Evan Bacon <bacon@expo.io>
2 tasks
Kudo
added a commit
that referenced
this pull request
Mar 22, 2023
# Why close ENG-7631 # How this is a side effect of #21265 where we enable full feature devtools. the `Debugger.pause` events are sent by devtools frontend: https://github.com/ChromeDevTools/devtools-frontend/blob/e0d743c4f184589db10fb1d276b87dbe90498077/front_end/entrypoints/inspector_main/InspectorMain.ts#L62-L97. we reached the `waitForDebuggerInPage===true` case and then devtools frontend calls `debuggerModel.pause()`. to skip the call path, we could either set `targetType=tab` or not set `panel=sources`. i am kinda afraid to change targetType which may cause other side effects, so to change the panel instead. with this pr, the initial devtools tab would be the "console" tab. # Test Plan bare-expo + devtools + sending js `fetch` call
Kudo
added a commit
that referenced
this pull request
Apr 10, 2023
close ENG-7464 close ENG-7492 - [dev-server] enable more devtools features rather than v8only - [dev-launcher] intercept `URLSession` requests and send CDP network events through metro-inspector-proxy to devtools bare-expo + `useDevClient = YES` in AppDelegate.mm (this is to enable dev-launcher) (cherry picked from commit 6f74b01)
Kudo
added a commit
that referenced
this pull request
Apr 10, 2023
# Why close ENG-7631 # How this is a side effect of #21265 where we enable full feature devtools. the `Debugger.pause` events are sent by devtools frontend: https://github.com/ChromeDevTools/devtools-frontend/blob/e0d743c4f184589db10fb1d276b87dbe90498077/front_end/entrypoints/inspector_main/InspectorMain.ts#L62-L97. we reached the `waitForDebuggerInPage===true` case and then devtools frontend calls `debuggerModel.pause()`. to skip the call path, we could either set `targetType=tab` or not set `panel=sources`. i am kinda afraid to change targetType which may cause other side effects, so to change the panel instead. with this pr, the initial devtools tab would be the "console" tab. # Test Plan bare-expo + devtools + sending js `fetch` call (cherry picked from commit 80135d2)
Kudo
added a commit
that referenced
this pull request
Apr 10, 2023
# Why close ENG-7631 # How this is a side effect of #21265 where we enable full feature devtools. the `Debugger.pause` events are sent by devtools frontend: https://github.com/ChromeDevTools/devtools-frontend/blob/e0d743c4f184589db10fb1d276b87dbe90498077/front_end/entrypoints/inspector_main/InspectorMain.ts#L62-L97. we reached the `waitForDebuggerInPage===true` case and then devtools frontend calls `debuggerModel.pause()`. to skip the call path, we could either set `targetType=tab` or not set `panel=sources`. i am kinda afraid to change targetType which may cause other side effects, so to change the panel instead. with this pr, the initial devtools tab would be the "console" tab. # Test Plan bare-expo + devtools + sending js `fetch` call (cherry picked from commit 80135d2)
Kudo
added a commit
that referenced
this pull request
Apr 10, 2023
close ENG-7464 close ENG-7492 - [dev-server] enable more devtools features rather than v8only - [dev-launcher] intercept `URLSession` requests and send CDP network events through metro-inspector-proxy to devtools bare-expo + `useDevClient = YES` in AppDelegate.mm (this is to enable dev-launcher) (cherry picked from commit 6f74b01)
Kudo
added a commit
that referenced
this pull request
Apr 10, 2023
# Why close ENG-7631 # How this is a side effect of #21265 where we enable full feature devtools. the `Debugger.pause` events are sent by devtools frontend: https://github.com/ChromeDevTools/devtools-frontend/blob/e0d743c4f184589db10fb1d276b87dbe90498077/front_end/entrypoints/inspector_main/InspectorMain.ts#L62-L97. we reached the `waitForDebuggerInPage===true` case and then devtools frontend calls `debuggerModel.pause()`. to skip the call path, we could either set `targetType=tab` or not set `panel=sources`. i am kinda afraid to change targetType which may cause other side effects, so to change the panel instead. with this pr, the initial devtools tab would be the "console" tab. # Test Plan bare-expo + devtools + sending js `fetch` call (cherry picked from commit 80135d2)
byCedric
added a commit
that referenced
this pull request
Apr 11, 2023
…oxy` (#21449) Fixes ENG-7467 Related #21265 This is an initial draft to extend the CDP functionality of `metro-inspector-proxy`. The implementation is slightly wonky around the `ExpoInspectorDevice`. We want to reuse as much as possible from `metro-inspector-proxy`, but we need to add stateful data per device. In order to achieve that, we generate a new class type, based on the user's installed `metro-inspector-proxy`. This makes everything less readable but should include future updates in these classes. As for the `ExpoInspectorProxy`, to avoid having to do the same thing, we just wrap the whole inspector class and reuse what we can. The device map is "linked" within the original inspector proxy instance, making the data available to all methods that need it. Enable this feature with `EXPO_USE_CUSTOM_INSPECTOR_PROXY=1` - [x] See tests for the actual CDP events we handle. - [ ] See tests on the "bootstrapping code" to create the inspector and devices. <!-- Please check the appropriate items below if they apply to your diff. This is required for changes to Expo modules. --> - [ ] Documentation is up to date to reflect these changes (eg: https://docs.expo.dev and README.md). - [ ] Conforms with the [Documentation Writing Style Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md) - [ ] This diff will work correctly for `expo prebuild` & EAS Build (eg: updated a module plugin). --------- Co-authored-by: Evan Bacon <bacon@expo.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Why
close ENG-7464
close ENG-7492
How
URLSessionrequests and send CDP network events through metro-inspector-proxy to devtoolsTest Plan
bare-expo +
useDevClient = YESin AppDelegate.mm (this is to enable dev-launcher)Checklist
expo prebuild& EAS Build (eg: updated a module plugin).