swap onBeforeRequest for onSendHeaders observer

Author: charlesvien

apps/code/src/main/utils/network-webrequest-logger.test.ts

@@ -21,7 +21,7 @@ import type { ObservableWebRequest } from "./network-webrequest-logger";
 import { contentLengthFromHeaders } from "./network-webrequest-logger";
 
 type Listeners = {
-  onBeforeRequest: Parameters<ObservableWebRequest["onBeforeRequest"]>[1];
+  onSendHeaders: Parameters<ObservableWebRequest["onSendHeaders"]>[1];
   onCompleted: Parameters<ObservableWebRequest["onCompleted"]>[1];
   onErrorOccurred: Parameters<ObservableWebRequest["onErrorOccurred"]>[1];
 };
@@ -33,8 +33,8 @@ function fakeWebRequest(): { webRequest: ObservableWebRequest } & {
   return {
     listeners,
     webRequest: {
-      onBeforeRequest: (_filter, listener) => {
-        listeners.onBeforeRequest = listener;
+      onSendHeaders: (_filter, listener) => {
+        listeners.onSendHeaders = listener;
       },
       onCompleted: (_filter, listener) => {
         listeners.onCompleted = listener;
@@ -84,29 +84,20 @@ describe("contentLengthFromHeaders", () => {
 });
 
 describe("installRendererNetworkLogging", () => {
-  it("always invokes the onBeforeRequest callback", async () => {
-    const { listeners } = await installFresh();
-    const callback = vi.fn();
-
-    listeners.onBeforeRequest(
-      { id: 1, method: "GET", url: "https://example.com/" },
-      callback,
-    );
-
-    expect(callback).toHaveBeenCalledWith({});
-  });
-
   it("records a completed request and mirrors it to the dev toolbar", async () => {
     const { listeners, devNetwork, record } = await installFresh();
 
-    listeners.onBeforeRequest(
-      { id: 7, method: "GET", url: "https://us.posthog.com/api/" },
-      vi.fn(),
-    );
+    listeners.onSendHeaders({
+      id: 7,
+      method: "GET",
+      url: "https://us.posthog.com/api/",
+      timestamp: 1_000,
+    });
     listeners.onCompleted({
       id: 7,
       method: "GET",
       url: "https://us.posthog.com/api/",
+      timestamp: 1_290,
       statusCode: 200,
       responseHeaders: { "Content-Length": ["1834"] },
     });
@@ -116,14 +107,16 @@ describe("installRendererNetworkLogging", () => {
       method: "GET",
       url: "https://us.posthog.com/api/",
       status: 200,
-      durationMs: expect.any(Number),
+      durationMs: 290,
       bytes: 1834,
     });
     expect(devNetwork.recordExternal).toHaveBeenCalledWith(
       expect.objectContaining({
         origin: "renderer",
         status: 200,
         ok: true,
+        durationMs: 290,
+        startedAt: 1_000,
         bytes: 1834,
       }),
     );
@@ -132,14 +125,17 @@ describe("installRendererNetworkLogging", () => {
   it("records a failed request with the chromium error string", async () => {
     const { listeners, devNetwork, record } = await installFresh();
 
-    listeners.onBeforeRequest(
-      { id: 9, method: "POST", url: "https://example.com/x" },
-      vi.fn(),
-    );
+    listeners.onSendHeaders({
+      id: 9,
+      method: "POST",
+      url: "https://example.com/x",
+      timestamp: 5_000,
+    });
     listeners.onErrorOccurred({
       id: 9,
       method: "POST",
       url: "https://example.com/x",
+      timestamp: 5_120,
       error: "net::ERR_CONNECTION_RESET",
     });
 
@@ -148,13 +144,15 @@ describe("installRendererNetworkLogging", () => {
       method: "POST",
       url: "https://example.com/x",
       status: null,
-      durationMs: expect.any(Number),
+      durationMs: 120,
       bytes: null,
       error: "net::ERR_CONNECTION_RESET",
     });
     expect(devNetwork.recordExternal).toHaveBeenCalledWith(
       expect.objectContaining({
         ok: false,
+        durationMs: 120,
+        startedAt: 5_000,
         error: "net::ERR_CONNECTION_RESET",
       }),
     );
@@ -167,6 +165,7 @@ describe("installRendererNetworkLogging", () => {
       id: 3,
       method: "GET",
       url: "https://example.com/missing",
+      timestamp: 2_000,
       statusCode: 404,
     });
 
@@ -175,13 +174,14 @@ describe("installRendererNetworkLogging", () => {
     );
   });
 
-  it("still records completions whose start entry was evicted", async () => {
+  it("still records completions with no start entry (evicted or cache hit that skipped onSendHeaders)", async () => {
     const { listeners, record } = await installFresh();
 
     listeners.onCompleted({
       id: 999,
       method: "GET",
       url: "https://example.com/",
+      timestamp: 3_000,
       statusCode: 200,
     });
 

apps/code/src/main/utils/network-webrequest-logger.ts

@@ -9,6 +9,7 @@ interface RequestDetails {
   id: number;
   method: string;
   url: string;
+  timestamp: number;
 }
 
 interface CompletedDetails extends RequestDetails {
@@ -21,12 +22,9 @@ interface FailedDetails extends RequestDetails {
 }
 
 export interface ObservableWebRequest {
-  onBeforeRequest(
+  onSendHeaders(
     filter: WebRequestFilter,
-    listener: (
-      details: RequestDetails,
-      callback: (response: { cancel?: boolean }) => void,
-    ) => void,
+    listener: (details: RequestDetails) => void,
   ): void;
   onCompleted(
     filter: WebRequestFilter,
@@ -45,18 +43,18 @@ interface PendingRequest {
 const MAX_PENDING_REQUESTS = 2000;
 const pending = new Map<number, PendingRequest>();
 
-function trackPending(id: number): void {
+function trackPending(id: number, startedAt: number): void {
   if (pending.size >= MAX_PENDING_REQUESTS) {
     const oldest = pending.keys().next().value;
     if (oldest !== undefined) pending.delete(oldest);
   }
-  pending.set(id, { startedAt: Date.now() });
+  pending.set(id, { startedAt });
 }
 
-function takeDuration(id: number): number {
+function takeDuration(id: number, endedAt: number): number {
   const start = pending.get(id);
   pending.delete(id);
-  return start ? Date.now() - start.startedAt : 0;
+  return start ? endedAt - start.startedAt : 0;
 }
 
 export function contentLengthFromHeaders(
@@ -80,17 +78,12 @@ export function installRendererNetworkLogging(
 
   const filter: WebRequestFilter = { urls: ["http://*/*", "https://*/*"] };
 
-  webRequest.onBeforeRequest(filter, (details, callback) => {
-    // Chromium stalls the request forever if the callback never runs.
-    try {
-      trackPending(details.id);
-    } finally {
-      callback({});
-    }
+  webRequest.onSendHeaders(filter, (details) => {
+    trackPending(details.id, details.timestamp);
   });
 
   webRequest.onCompleted(filter, (details) => {
-    const durationMs = takeDuration(details.id);
+    const durationMs = takeDuration(details.id, details.timestamp);
     const bytes = contentLengthFromHeaders(details.responseHeaders);
     recordNetworkRequest({
       origin: "renderer",
@@ -107,13 +100,13 @@ export function installRendererNetworkLogging(
       status: details.statusCode,
       ok: details.statusCode >= 200 && details.statusCode < 300,
       durationMs,
-      startedAt: Date.now() - durationMs,
+      startedAt: details.timestamp - durationMs,
       bytes,
     });
   });
 
   webRequest.onErrorOccurred(filter, (details) => {
-    const durationMs = takeDuration(details.id);
+    const durationMs = takeDuration(details.id, details.timestamp);
     recordNetworkRequest({
       origin: "renderer",
       method: details.method,
@@ -130,7 +123,7 @@ export function installRendererNetworkLogging(
       status: null,
       ok: false,
       durationMs,
-      startedAt: Date.now() - durationMs,
+      startedAt: details.timestamp - durationMs,
       bytes: null,
       error: details.error,
     });