chore: propagate 'Invalid header' errors (#7020)

Enable developers to handle 'Invalid header' errors instead of hiding them to make sure they can address them properly.
Co-authored-by: Jan Scheffler <janscheffler@chromium.org>

src/common/Connection.ts

@@ -22,6 +22,7 @@ import { Protocol } from 'devtools-protocol';
 import { ProtocolMapping } from 'devtools-protocol/types/protocol-mapping.js';
 import { ConnectionTransport } from './ConnectionTransport.js';
 import { EventEmitter } from './EventEmitter.js';
+import { ProtocolError } from './Errors.js';
 
 /**
  * @public
@@ -34,7 +35,7 @@ export { ConnectionTransport, ProtocolMapping };
 export interface ConnectionCallback {
   resolve: Function;
   reject: Function;
-  error: Error;
+  error: ProtocolError;
   method: string;
 }
 
@@ -99,7 +100,12 @@ export class Connection extends EventEmitter {
     const params = paramArgs.length ? paramArgs[0] : undefined;
     const id = this._rawSend({ method, params });
     return new Promise((resolve, reject) => {
-      this._callbacks.set(id, { resolve, reject, error: new Error(), method });
+      this._callbacks.set(id, {
+        resolve,
+        reject,
+        error: new ProtocolError(),
+        method,
+      });
     });
   }
 
@@ -206,7 +212,7 @@ export interface CDPSessionOnMessageObject {
   id?: number;
   method: string;
   params: Record<string, unknown>;
-  error: { message: string; data: any };
+  error: { message: string; data: any; code: number };
   result?: any;
 }
 
@@ -288,7 +294,12 @@ export class CDPSession extends EventEmitter {
     });
 
     return new Promise((resolve, reject) => {
-      this._callbacks.set(id, { resolve, reject, error: new Error(), method });
+      this._callbacks.set(id, {
+        resolve,
+        reject,
+        error: new ProtocolError(),
+        method,
+      });
     });
   }
 
@@ -348,21 +359,26 @@ export class CDPSession extends EventEmitter {
  * @returns {!Error}
  */
 function createProtocolError(
-  error: Error,
+  error: ProtocolError,
   method: string,
-  object: { error: { message: string; data: any } }
+  object: { error: { message: string; data: any; code: number } }
 ): Error {
   let message = `Protocol error (${method}): ${object.error.message}`;
   if ('data' in object.error) message += ` ${object.error.data}`;
-  return rewriteError(error, message);
+  return rewriteError(error, message, object.error.message);
 }
 
 /**
  * @param {!Error} error
  * @param {string} message
  * @returns {!Error}
  */
-function rewriteError(error: Error, message: string): Error {
+function rewriteError(
+  error: ProtocolError,
+  message: string,
+  originalMessage?: string
+): Error {
   error.message = message;
+  error.originalMessage = originalMessage;
   return error;
 }

src/common/Errors.ts

@@ -18,7 +18,7 @@
  * @public
  */
 export class CustomError extends Error {
-  constructor(message: string) {
+  constructor(message?: string) {
     super(message);
     this.name = this.constructor.name;
     Error.captureStackTrace(this, this.constructor);
@@ -36,6 +36,15 @@ export class CustomError extends Error {
  * @public
  */
 export class TimeoutError extends CustomError {}
+/**
+ * ProtocolError is emitted whenever there is an error from the protocol.
+ *
+ * @public
+ */
+export class ProtocolError extends CustomError {
+  public code?: number;
+  public originalMessage: string;
+}
 /**
  * @public
  */

src/common/HTTPRequest.ts

@@ -19,6 +19,7 @@ import { HTTPResponse } from './HTTPResponse.js';
 import { assert } from './assert.js';
 import { helper, debugError } from './helper.js';
 import { Protocol } from 'devtools-protocol';
+import { ProtocolError } from './Errors.js';
 
 /**
  * @public
@@ -229,11 +230,7 @@ export class HTTPRequest {
   async finalizeInterceptions(): Promise<void> {
     await this._interceptActions.reduce(
       (promiseChain, interceptAction) =>
-        promiseChain.then(interceptAction).catch((error) => {
-          // This is here so cooperative handlers that fail do not stop other handlers
-          // from running
-          debugError(error);
-        }),
+        promiseChain.then(interceptAction).catch(handleError),
       Promise.resolve()
     );
     const [resolution] = this.interceptResolution();
@@ -443,12 +440,7 @@ export class HTTPRequest {
         postData: postDataBinaryBase64,
         headers: headers ? headersArray(headers) : undefined,
       })
-      .catch((error) => {
-        // In certain cases, protocol will return error if the request was
-        // already canceled or the page was closed. We should tolerate these
-        // errors.
-        debugError(error);
-      });
+      .catch(handleError);
   }
 
   /**
@@ -540,12 +532,7 @@ export class HTTPRequest {
         responseHeaders: headersArray(responseHeaders),
         body: responseBody ? responseBody.toString('base64') : undefined,
       })
-      .catch((error) => {
-        // In certain cases, protocol will return error if the request was
-        // already canceled or the page was closed. We should tolerate these
-        // errors.
-        debugError(error);
-      });
+      .catch(handleError);
   }
 
   /**
@@ -594,12 +581,7 @@ export class HTTPRequest {
         requestId: this._interceptionId,
         errorReason,
       })
-      .catch((error) => {
-        // In certain cases, protocol will return error if the request was
-        // already canceled or the page was closed. We should tolerate these
-        // errors.
-        debugError(error);
-      });
+      .catch(handleError);
   }
 }
 
@@ -666,6 +648,16 @@ function headersArray(
   return result;
 }
 
+async function handleError(error: ProtocolError) {
+  if (['Invalid header'].includes(error.originalMessage)) {
+    throw error;
+  }
+  // In certain cases, protocol will return error if the request was
+  // already canceled or the page was closed. We should tolerate these
+  // errors.
+  debugError(error);
+}
+
 // List taken from
 // https://www.iana.org/assignments/http-status-codes/http-status-codes.xhtml
 // with extra 306 and 418 codes.

src/common/NetworkManager.ts

@@ -388,10 +388,7 @@ export class NetworkManager extends EventEmitter {
     );
     this._requestIdToRequest.set(event.requestId, request);
     this.emit(NetworkManagerEmittedEvents.Request, request);
-    request.finalizeInterceptions().catch((error) => {
-      // This should never happen, but catch just in case.
-      debugError(error);
-    });
+    request.finalizeInterceptions();
   }
 
   _onRequestServedFromCache(

test/network.spec.ts

@@ -68,6 +68,30 @@ describe('network', function () {
     });
   });
 
+  describeFailsFirefox('Request.continue', () => {
+    it('should split a request header at new line characters and add the header multiple times instead', async () => {
+      const { page, server } = getTestState();
+
+      let resolve;
+      const errorPromise = new Promise((r) => {
+        resolve = r;
+      });
+
+      await page.setRequestInterception(true);
+      page.on('request', async (request) => {
+        await request
+          .continue({
+            headers: {
+              'X-Test-Header': 'a\nb',
+            },
+          })
+          .catch(resolve);
+      });
+      page.goto(server.PREFIX + '/empty.html');
+      const error = await errorPromise;
+      expect(error).toBeTruthy();
+    });
+  });
   describe('Request.frame', function () {
     it('should work for main frame navigation request', async () => {
       const { page, server } = getTestState();