fix(network): extra info can arrive before will send request (#8569)

src/client/browserContext.ts

@@ -86,9 +86,7 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel,
     });
     this._channel.on('request', ({ request, page }) => this._onRequest(network.Request.from(request), Page.fromNullable(page)));
     this._channel.on('requestFailed', ({ request, failureText, responseEndTiming, page }) => this._onRequestFailed(network.Request.from(request), responseEndTiming, failureText, Page.fromNullable(page)));
-    this._channel.on('requestFinished', ({ request, response, responseEndTiming, responseHeaders, page, requestSizes }) =>
-      this._onRequestFinished(network.Request.from(request), network.Response.fromNullable(response), responseEndTiming, responseHeaders, requestSizes, Page.fromNullable(page))
-    );
+    this._channel.on('requestFinished', params => this._onRequestFinished(params));
     this._channel.on('response', ({ response, page }) => this._onResponse(network.Response.from(response), Page.fromNullable(page)));
     this._closedPromise = new Promise(f => this.once(Events.BrowserContext.Close, f));
   }
@@ -126,7 +124,11 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel,
       page.emit(Events.Page.RequestFailed, request);
   }
 
-  private _onRequestFinished(request: network.Request, response: network.Response | null, responseEndTiming: number, responseHeaders: channels.NameValue[] | undefined, requestSizes: channels.RequestSizes, page: Page | null) {
+  private _onRequestFinished(params: channels.BrowserContextRequestFinishedEvent) {
+    const { requestSizes, responseEndTiming, responseHeaders } = params;
+    const request = network.Request.from(params.request);
+    const response = network.Response.fromNullable(params.response);
+    const page = Page.fromNullable(params.page);
     if (request._timing)
       request._timing.responseEnd = responseEndTiming;
     request._sizes = requestSizes;
@@ -135,6 +137,8 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel,
     this.emit(Events.BrowserContext.RequestFinished, request);
     if (page)
       page.emit(Events.Page.RequestFinished, request);
+    if (response)
+      response._finishedPromise.resolve();
   }
 
   _onRoute(route: network.Route, request: network.Request) {

src/client/network.ts

@@ -21,7 +21,7 @@ import { Frame } from './frame';
 import { Headers, RemoteAddr, SecurityDetails, WaitForEventOptions } from './types';
 import fs from 'fs';
 import * as mime from 'mime';
-import { isString, headersObjectToArray, headersArrayToObject } from '../utils/utils';
+import { isString, headersObjectToArray, headersArrayToObject, ManualPromise } from '../utils/utils';
 import { Events } from './events';
 import { Page } from './page';
 import { Waiter } from './waiter';
@@ -384,6 +384,7 @@ export type RequestSizes = {
 export class Response extends ChannelOwner<channels.ResponseChannel, channels.ResponseInitializer> implements api.Response {
   _headers: Headers;
   private _request: Request;
+  readonly _finishedPromise = new ManualPromise<void>();
 
   static from(response: channels.ResponseChannel): Response {
     return (response as any)._object;
@@ -421,13 +422,8 @@ export class Response extends ChannelOwner<channels.ResponseChannel, channels.Re
     return { ...this._headers };
   }
 
-  async finished(): Promise<Error | null> {
-    return this._wrapApiCall(async (channel: channels.ResponseChannel) => {
-      const result = await channel.finished();
-      if (result.error)
-        return new Error(result.error);
-      return null;
-    });
+  async finished(): Promise<null> {
+    return this._finishedPromise.then(() => null);
   }
 
   async body(): Promise<Buffer> {

src/dispatchers/networkDispatchers.ts

@@ -73,10 +73,6 @@ export class ResponseDispatcher extends Dispatcher<Response, channels.ResponseIn
     });
   }
 
-  async finished(): Promise<channels.ResponseFinishedResult> {
-    return await this._object._finishedPromise;
-  }
-
   async body(): Promise<channels.ResponseBodyResult> {
     return { binary: (await this._object.body()).toString('base64') };
   }

src/protocol/channels.ts

@@ -2673,7 +2673,6 @@ export type ResponseInitializer = {
 };
 export interface ResponseChannel extends Channel {
   body(params?: ResponseBodyParams, metadata?: Metadata): Promise<ResponseBodyResult>;
-  finished(params?: ResponseFinishedParams, metadata?: Metadata): Promise<ResponseFinishedResult>;
   securityDetails(params?: ResponseSecurityDetailsParams, metadata?: Metadata): Promise<ResponseSecurityDetailsResult>;
   serverAddr(params?: ResponseServerAddrParams, metadata?: Metadata): Promise<ResponseServerAddrResult>;
 }
@@ -2682,11 +2681,6 @@ export type ResponseBodyOptions = {};
 export type ResponseBodyResult = {
   binary: Binary,
 };
-export type ResponseFinishedParams = {};
-export type ResponseFinishedOptions = {};
-export type ResponseFinishedResult = {
-  error?: string,
-};
 export type ResponseSecurityDetailsParams = {};
 export type ResponseSecurityDetailsOptions = {};
 export type ResponseSecurityDetailsResult = {

src/protocol/protocol.yml

@@ -2199,10 +2199,6 @@ Response:
       returns:
         binary: binary
 
-    finished:
-      returns:
-        error: string?
-
     securityDetails:
       returns:
         value: SecurityDetails?

src/protocol/validator.ts

@@ -1042,7 +1042,6 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme {
     responseStart: tNumber,
   });
   scheme.ResponseBodyParams = tOptional(tObject({}));
-  scheme.ResponseFinishedParams = tOptional(tObject({}));
   scheme.ResponseSecurityDetailsParams = tOptional(tObject({}));
   scheme.ResponseServerAddrParams = tOptional(tObject({}));
   scheme.SecurityDetails = tObject({

src/server/chromium/crNetworkManager.ts

@@ -337,12 +337,12 @@ export class CRNetworkManager {
 
   _handleRequestRedirect(request: InterceptableRequest, responsePayload: Protocol.Network.Response, timestamp: number) {
     const response = this._createResponse(request, responsePayload);
-    response._requestFinished((timestamp - request._timestamp) * 1000, 'Response body is unavailable for redirect responses');
+    response._requestFinished((timestamp - request._timestamp) * 1000);
     this._requestIdToRequest.delete(request._requestId);
     if (request._interceptionId)
       this._attemptedAuthentications.delete(request._interceptionId);
     this._page._frameManager.requestReceivedResponse(response);
-    this._page._frameManager.requestFinished(request.request, response);
+    this._page._frameManager.reportRequestFinished(request.request, response);
   }
 
   _onResponseReceivedExtraInfo(event: Protocol.Network.responseReceivedExtraInfoPayload) {
@@ -377,12 +377,12 @@ export class CRNetworkManager {
     if (response) {
       request.request._sizes.transferSize = event.encodedDataLength;
       request.request._sizes.responseBodySize = event.encodedDataLength - response?.headersSize();
-      response._requestFinished(helper.secondsToRoundishMillis(event.timestamp - request._timestamp), undefined);
+      response._requestFinished(helper.secondsToRoundishMillis(event.timestamp - request._timestamp));
     }
     this._requestIdToRequest.delete(request._requestId);
     if (request._interceptionId)
       this._attemptedAuthentications.delete(request._interceptionId);
-    this._page._frameManager.requestFinished(request.request, response);
+    this._page._frameManager.reportRequestFinished(request.request, response);
   }
 
   _onLoadingFailed(event: Protocol.Network.loadingFailedPayload) {
@@ -596,13 +596,22 @@ class ResponseExtraInfoTracker {
       // This is redirect.
       this._innerResponseReceived(info, event.redirectResponse);
     } else {
-      this._requests.set(event.requestId, {
-        requestId: event.requestId,
+      this._getOrCreateEntry(event.requestId);
+    }
+  }
+
+  _getOrCreateEntry(requestId: string): RequestInfo {
+    let info = this._requests.get(requestId);
+    if (!info) {
+      info = {
+        requestId: requestId,
         responseReceivedExtraInfo: [],
         responses: [],
         sawResponseWithoutConnectionId: false
-      });
+      };
+      this._requests.set(requestId, info);
     }
+    return info;
   }
 
   responseReceived(event: Protocol.Network.responseReceivedPayload) {
@@ -620,9 +629,7 @@ class ResponseExtraInfoTracker {
   }
 
   responseReceivedExtraInfo(event: Protocol.Network.responseReceivedExtraInfoPayload) {
-    const info = this._requests.get(event.requestId);
-    if (!info)
-      return;
+    const info = this._getOrCreateEntry(event.requestId);
     info.responseReceivedExtraInfo.push(event);
     this._patchResponseHeaders(info, info.responseReceivedExtraInfo.length - 1);
     this._checkFinished(info);

src/server/firefox/ffNetworkManager.ts

@@ -123,12 +123,12 @@ export class FFNetworkManager {
     // Keep redirected requests in the map for future reference as redirectedFrom.
     const isRedirected = response.status() >= 300 && response.status() <= 399;
     if (isRedirected) {
-      response._requestFinished(this._relativeTiming(event.responseEndTime), 'Response body is unavailable for redirect responses');
+      response._requestFinished(this._relativeTiming(event.responseEndTime));
     } else {
       this._requests.delete(request._id);
-      response._requestFinished(this._relativeTiming(event.responseEndTime), undefined);
+      response._requestFinished(this._relativeTiming(event.responseEndTime));
     }
-    this._page._frameManager.requestFinished(request.request, response);
+    this._page._frameManager.reportRequestFinished(request.request, response);
   }
 
   _onRequestFailed(event: Protocol.Network.requestFailedPayload) {

src/server/frames.ts

@@ -275,7 +275,7 @@ export class FrameManager {
     this._page._browserContext.emit(BrowserContext.Events.Response, response);
   }
 
-  requestFinished(request: network.Request, response: network.Response | null) {
+  reportRequestFinished(request: network.Request, response: network.Response | null) {
     this._inflightRequestFinished(request);
     if (request._isFavicon)
       return;

src/server/network.ts

@@ -86,7 +86,7 @@ type RequestSizes = {
 export class Request extends SdkObject {
   private _response: Response | null = null;
   private _redirectedFrom: Request | null;
-  private _redirectedTo: Request | null = null;
+  _redirectedTo: Request | null = null;
   readonly _documentId?: string;
   readonly _isFavicon: boolean;
   _failureText: string | null = null;
@@ -319,7 +319,7 @@ export type SecurityDetails = {
 export class Response extends SdkObject {
   private _request: Request;
   private _contentPromise: Promise<Buffer> | null = null;
-  _finishedPromise = new ManualPromise<{ error?: string }>();
+  _finishedPromise = new ManualPromise<void>();
   private _status: number;
   private _statusText: string;
   private _url: string;
@@ -355,9 +355,9 @@ export class Response extends SdkObject {
     this._securityDetailsPromise.resolve(securityDetails);
   }
 
-  _requestFinished(responseEndTiming: number, error?: string) {
+  _requestFinished(responseEndTiming: number) {
     this._request._responseEndTiming = Math.max(responseEndTiming, this._timing.responseStart);
-    this._finishedPromise.resolve({ error });
+    this._finishedPromise.resolve();
   }
 
   _setHttpVersion(httpVersion: string) {
@@ -403,10 +403,6 @@ export class Response extends SdkObject {
     return this._headersMap.get(name);
   }
 
-  finished(): Promise<Error | null> {
-    return this._finishedPromise.then(({ error }) => error ? new Error(error) : null);
-  }
-
   timing(): ResourceTiming {
     return this._timing;
   }
@@ -421,9 +417,9 @@ export class Response extends SdkObject {
 
   body(): Promise<Buffer> {
     if (!this._contentPromise) {
-      this._contentPromise = this._finishedPromise.then(async ({ error }) => {
-        if (error)
-          throw new Error(error);
+      this._contentPromise = this._finishedPromise.then(async () => {
+        if (this._request._redirectedTo)
+          throw new Error('Response body is unavailable for redirect responses');
         return this._getResponseBodyCallback();
       });
     }

src/server/webkit/wkPage.ts

@@ -978,10 +978,10 @@ export class WKPage implements PageDelegate {
     const response = request.createResponse(responsePayload);
     response._securityDetailsFinished();
     response._serverAddrFinished();
-    response._requestFinished(responsePayload.timing ? helper.secondsToRoundishMillis(timestamp - request._timestamp) : -1, 'Response body is unavailable for redirect responses');
+    response._requestFinished(responsePayload.timing ? helper.secondsToRoundishMillis(timestamp - request._timestamp) : -1);
     this._requestIdToRequest.delete(request._requestId);
     this._page._frameManager.requestReceivedResponse(response);
-    this._page._frameManager.requestFinished(request.request, response);
+    this._page._frameManager.reportRequestFinished(request.request, response);
   }
 
   _onRequestIntercepted(session: WKSession, event: Protocol.Network.requestInterceptedPayload) {
@@ -1051,12 +1051,12 @@ export class WKPage implements PageDelegate {
       request.request._sizes.transferSize += response.headersSize();
       if (event.metrics?.protocol)
         response._setHttpVersion(event.metrics.protocol);
-      response._requestFinished(helper.secondsToRoundishMillis(event.timestamp - request._timestamp), undefined);
+      response._requestFinished(helper.secondsToRoundishMillis(event.timestamp - request._timestamp));
     }
 
     this._requestIdToResponseReceivedPayloadEvent.delete(request._requestId);
     this._requestIdToRequest.delete(request._requestId);
-    this._page._frameManager.requestFinished(request.request, response);
+    this._page._frameManager.reportRequestFinished(request.request, response);
   }
 
   _onLoadingFailed(event: Protocol.Network.loadingFailedPayload) {