[expo-dev-launcher] Add missing `Network.requestWillBeSentExtraInfo` event #21965

byCedric · 2023-04-03T16:50:32Z

Why

This fixes the last two remaining "weird debug information" in chrome devtools.

The request headers are marked as "⚠️ Provisional headers shown"
The Timing column is stuck on "Pending" instead of showing the actual request time.

Page	Screenshot
Overview
Headers tab
Timing tab

How

The two missing pieces of information are sent in the Network.requestWillBeSentExtraInfo event, under headers and connectTiming.requestTime.

I don't think we can actually get the connectionStart/connectionEnd timing, as described here. Instead of fetching that value, we just skip it and let chrome devtools only show the actual response time.

Test Plan

Make a request fetch('https://httpbin.org/anything')
Check the network tab
Should NOT show ⚠️ Provisional headers are shown
Should NOT show Pending under Timing

Checklist

Documentation is up to date to reflect these changes (eg: https://docs.expo.dev and README.md).
Conforms with the Documentation Writing Style Guide
This diff will work correctly for expo prebuild & EAS Build (eg: updated a module plugin).

lukmccall · 2023-04-03T16:58:21Z

...-launcher/android/src/main/java/expo/modules/devlauncher/network/DevLauncherNetworkLogger.kt

@@ -80,11 +80,25 @@ class DevLauncherNetworkLogger private constructor() {
        ))
      }
    }
-    val data = JSONObject(mapOf(
+    var data = JSONObject(mapOf(


I don't like changing from val to var if not needed. Just create a new variable with a meaningful name instead.

Alright, do you want me to change the others as well? I copied this from:

expo/packages/expo-dev-launcher/android/src/main/java/expo/modules/devlauncher/network/DevLauncherNetworkLogger.kt

Lines 130 to 135 in 974bd5c

params = mapOf(

"requestId" to requestId,

"timestamp" to now,

"encodedDataLength" to (response.body()?.contentLength() ?: 0),

)

data = JSONObject(mapOf(

Yes, it's a good idea. Thanks

packages/expo-dev-launcher/ios/Network/EXDevLauncherNetworkLogger.swift

...-launcher/android/src/main/java/expo/modules/devlauncher/network/DevLauncherNetworkLogger.kt

Kudo · 2023-04-03T18:37:20Z

...-launcher/android/src/main/java/expo/modules/devlauncher/network/DevLauncherNetworkLogger.kt

  }

  /**
   * Emits CDP `Network.responseReceived` and `Network.loadingFinished` events
   */
  fun emitNetworkResponse(request: Request, requestId: String, response: Response) {
    val now = BigDecimal(System.currentTimeMillis() / 1000.0).setScale(3, RoundingMode.CEILING)
-    var params = mapOf(
+    val responseReceivedParams = mapOf(
      "requestId" to requestId,
      "loaderId" to "",
      "hasExtraInfo" to false,


do you think we should set the hasExtraInfo as true when we send the Network.requestWillBeSentExtraInfo?

and also redirectHasExtraInfo maybe

I think you are right about the redirectHasExtraInfo:

But I don't think we have to enable the hasExtraInfo on Network.responseReceived, because that refers to the Network.responseReceivedExtraInfo.

So far, this change is only required because of the visual "⚠️ Provisional headers are shown" issue. Which seems to be an oversight in the spec itself, there is no option to say "we are not implementing the Network.requestWillBeSentExtraInfo event". Luckily, we do have an option to disable it for responses (this hasExtraInfo).

makes sense and nice explanation! thanks

…fo` event

Co-authored-by: Kudo Chien <kudo@expo.dev>

…ve a redirect response

…event (#21965) # Why This fixes the last two remaining "weird debug information" in chrome devtools. 1. The request headers are marked as "⚠️ Provisional headers shown" 2. The `Timing` column is stuck on "Pending" instead of showing the actual request time. Page | Screenshot --- | --- Overview | <img src="https://user-images.githubusercontent.com/1203991/229575879-201d2b47-ff02-4512-adc3-72716fad71f8.png"> Headers tab | <img src="https://user-images.githubusercontent.com/1203991/229575999-b57c6428-a754-4e69-910e-a1cd76c9b761.png"> Timing tab | <img width="1030" alt="image" src="https://user-images.githubusercontent.com/1203991/229576107-d08d1f65-9203-4e7e-bf4d-73b42bbbbc8f.png"> # How The two missing pieces of information are sent in the [`Network.requestWillBeSentExtraInfo`](https://chromedevtools.github.io/devtools-protocol/tot/Network/#event-requestWillBeSentExtraInfo) event, under `headers` and `connectTiming.requestTime`. > I don't think we can actually get the `connectionStart`/`connectionEnd` timing, [as described here](https://developer.apple.com/documentation/foundation/nsurlsessiontasktransactionmetrics#3162615). Instead of fetching that value, we just skip it and let chrome devtools only show the actual response time. # Test Plan - Make a request `fetch('https://httpbin.org/anything')` - Check the network tab - Should NOT show `⚠️ Provisional headers are shown` - Should NOT show `Pending` under `Timing` # Checklist  - [ ] 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: Kudo Chien <kudo@expo.dev>

…event (#21965) # Why This fixes the last two remaining "weird debug information" in chrome devtools. 1. The request headers are marked as "⚠️ Provisional headers shown" 2. The `Timing` column is stuck on "Pending" instead of showing the actual request time. Page | Screenshot --- | --- Overview | <img src="https://user-images.githubusercontent.com/1203991/229575879-201d2b47-ff02-4512-adc3-72716fad71f8.png"> Headers tab | <img src="https://user-images.githubusercontent.com/1203991/229575999-b57c6428-a754-4e69-910e-a1cd76c9b761.png"> Timing tab | <img width="1030" alt="image" src="https://user-images.githubusercontent.com/1203991/229576107-d08d1f65-9203-4e7e-bf4d-73b42bbbbc8f.png"> # How The two missing pieces of information are sent in the [`Network.requestWillBeSentExtraInfo`](https://chromedevtools.github.io/devtools-protocol/tot/Network/#event-requestWillBeSentExtraInfo) event, under `headers` and `connectTiming.requestTime`. > I don't think we can actually get the `connectionStart`/`connectionEnd` timing, [as described here](https://developer.apple.com/documentation/foundation/nsurlsessiontasktransactionmetrics#3162615). Instead of fetching that value, we just skip it and let chrome devtools only show the actual response time. # Test Plan - Make a request `fetch('https://httpbin.org/anything')` - Check the network tab - Should NOT show `⚠️ Provisional headers are shown` - Should NOT show `Pending` under `Timing` # Checklist  - [ ] 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: Kudo Chien <kudo@expo.dev> (cherry picked from commit 0cf2d5a)

byCedric requested review from lukmccall and douglowder as code owners April 3, 2023 16:50

byCedric requested a review from Kudo April 3, 2023 16:50

expo-bot added the bot: suggestions ExpoBot has some suggestions label Apr 3, 2023

lukmccall approved these changes Apr 3, 2023

View reviewed changes

byCedric commented Apr 3, 2023

View reviewed changes

packages/expo-dev-launcher/ios/Network/EXDevLauncherNetworkLogger.swift Outdated Show resolved Hide resolved

expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Apr 3, 2023

Kudo reviewed Apr 3, 2023

View reviewed changes

byCedric force-pushed the @bycedric/inspector/add-extra-request-info branch from fc4e209 to 4e7fa54 Compare April 3, 2023 21:31

Kudo approved these changes Apr 4, 2023

View reviewed changes

byCedric and others added 6 commits April 4, 2023 13:20

fix(expo-dev-launcher): add missing `Network.requestWillBeSentExtraIn…

b2656b2

…fo` event

[expo-dev-client] Add missing changelog entry

9b68c7c

[expo-dev-client] Resolve swift lint violation

84b30fd

[expo-dev-client] Convert var to val in Android network logger

1c08865

[expo-dev-client] Fix typo in var name

1dea098

Co-authored-by: Kudo Chien <kudo@expo.dev>

[expo-dev-launcher] Set redirectHasExtraInfo to true whenever we ha…

fc4597b

…ve a redirect response

byCedric force-pushed the @bycedric/inspector/add-extra-request-info branch from 4e7fa54 to fc4597b Compare April 4, 2023 11:20

byCedric merged commit 0cf2d5a into main Apr 4, 2023

byCedric deleted the @bycedric/inspector/add-extra-request-info branch April 4, 2023 12:43

Kudo added the published Changes from the PR have been published to npm label Apr 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[expo-dev-launcher] Add missing `Network.requestWillBeSentExtraInfo` event #21965

[expo-dev-launcher] Add missing `Network.requestWillBeSentExtraInfo` event #21965

byCedric commented Apr 3, 2023 •

edited

lukmccall Apr 3, 2023

byCedric Apr 3, 2023

lukmccall Apr 3, 2023

byCedric Apr 3, 2023

Kudo Apr 3, 2023

byCedric Apr 3, 2023 •

edited

Kudo Apr 4, 2023

	params = mapOf(
	"requestId" to requestId,
	"timestamp" to now,
	"encodedDataLength" to (response.body()?.contentLength() ?: 0),
	)
	data = JSONObject(mapOf(

[expo-dev-launcher] Add missing Network.requestWillBeSentExtraInfo event #21965

[expo-dev-launcher] Add missing Network.requestWillBeSentExtraInfo event #21965

Conversation

byCedric commented Apr 3, 2023 • edited

Why

How

Test Plan

Checklist

lukmccall Apr 3, 2023

Choose a reason for hiding this comment

byCedric Apr 3, 2023

Choose a reason for hiding this comment

lukmccall Apr 3, 2023

Choose a reason for hiding this comment

byCedric Apr 3, 2023

Choose a reason for hiding this comment

Kudo Apr 3, 2023

Choose a reason for hiding this comment

byCedric Apr 3, 2023 • edited

Choose a reason for hiding this comment

Kudo Apr 4, 2023

Choose a reason for hiding this comment

[expo-dev-launcher] Add missing `Network.requestWillBeSentExtraInfo` event #21965

[expo-dev-launcher] Add missing `Network.requestWillBeSentExtraInfo` event #21965

byCedric commented Apr 3, 2023 •

edited

byCedric Apr 3, 2023 •

edited