fix(puppet): stabilize chained nav

mitm/handlers/HttpRequestHandler.ts

@@ -30,6 +30,10 @@ export default class HttpRequestHandler extends BaseHttpHandler {
     const { clientToProxyRequest, proxyToClientResponse } = request;
     clientToProxyRequest.on('error', this.onError.bind(this, 'ClientToProxy.RequestError'));
     if (clientToProxyRequest instanceof Http2ServerRequest) {
+      clientToProxyRequest.stream.on(
+        'error',
+        this.onError.bind(this, 'ClientToProxy.Http2StreamError'),
+      );
       const http2Session = clientToProxyRequest.stream.session;
       if (!http2Session.listenerCount('error')) {
         http2Session.on('error', this.onError.bind(this, 'ClientToProxy.Http2SessionError'));

puppet-chrome/lib/CDPSession.ts

@@ -82,7 +82,7 @@ export class CDPSession extends EventEmitter {
   onMessage(object: ICDPSendResponseMessage & ICDPEventMessage): void {
     this.messageEvents.emit('receive', { ...object });
     if (!object.id) {
-      this.emit(object.method, object.params);
+      setImmediate(() => this.emit(object.method, object.params));
       return;
     }
 
@@ -92,9 +92,9 @@ export class CDPSession extends EventEmitter {
     this.pendingMessages.delete(object.id);
     if (object.error) {
       const protocolError = new ProtocolError(callback.error.stack, callback.method, object.error);
-      callback.reject(protocolError);
+      setImmediate(() => callback.reject(protocolError));
     } else {
-      callback.resolve(object.result);
+      setImmediate(() => callback.resolve(object.result));
     }
   }
 

puppet-chrome/lib/Frame.ts

@@ -52,7 +52,7 @@ export default class Frame extends TypedEventEmitter<IFrameEvents> implements IP
 
   protected readonly logger: IBoundLog;
 
-  private loaderIdResolvers = new Map<string, IResolvablePromise<void>>();
+  private loaderIdResolvers = new Map<string, IResolvablePromise<Error | null>>();
   private activeLoaderId: string;
   private readonly activeContexts: Set<number>;
   private readonly cdpSession: CDPSession;
@@ -134,7 +134,7 @@ export default class Frame extends TypedEventEmitter<IFrameEvents> implements IP
       loader = this.loaderIdResolvers.get(frame.loaderId) ?? this.activeLoader;
     }
     if (frame.unreachableUrl) {
-      loader.reject(new Error(`Unreachable url for navigation "${frame.unreachableUrl}"`));
+      loader.resolve(new Error(`Unreachable url for navigation "${frame.unreachableUrl}"`));
     } else {
       loader.resolve();
     }
@@ -163,10 +163,16 @@ export default class Frame extends TypedEventEmitter<IFrameEvents> implements IP
   }
 
   public async waitForLoader(loaderId?: string) {
-    await this.loaderIdResolvers.get(loaderId ?? this.activeLoaderId)?.promise;
+    const hasLoaderError = await this.loaderIdResolvers.get(loaderId ?? this.activeLoaderId)
+      ?.promise;
+    if (hasLoaderError) return hasLoaderError;
+
+    if (!this.getActiveContextId(false)) {
+      await this.waitForDefaultContext();
+    }
   }
 
-  public async onLifecycleEvent(name: string, pageLoaderId?: string) {
+  public onLifecycleEvent(name: string, pageLoaderId?: string) {
     const loaderId = pageLoaderId ?? this.activeLoaderId;
     if (name === 'init') {
       if (!this.loaderIdResolvers.has(loaderId)) {
@@ -177,7 +183,11 @@ export default class Frame extends TypedEventEmitter<IFrameEvents> implements IP
         this.setLoader(loaderId);
       }
     }
-    if (name === 'commit') {
+
+    if (
+      (name === 'commit' || name === 'DOMContentLoaded' || name === 'load') &&
+      !this.loaderIdResolvers.get(loaderId)?.isResolved
+    ) {
       this.logger.info('Resolving loader', {
         loaderId,
         frameId: this.id,
@@ -220,9 +230,6 @@ export default class Frame extends TypedEventEmitter<IFrameEvents> implements IP
 
   public async waitForActiveContextId(isolatedContext = true): Promise<number> {
     if (!this.isAttached()) throw new Error('Execution Context is not available in detached frame');
-    if (this.activeLoaderId) {
-      await this.loaderIdResolvers.get(this.activeLoaderId).promise;
-    }
 
     const existing = this.getActiveContextId(isolatedContext);
     if (existing) return existing;
@@ -235,8 +242,7 @@ export default class Frame extends TypedEventEmitter<IFrameEvents> implements IP
     }
 
     await this.waitForDefaultContext();
-
-    return this.waitForActiveContextId(isolatedContext);
+    return this.getActiveContextId(isolatedContext);
   }
 
   public getActiveContextId(isolatedContext: boolean) {
@@ -266,18 +272,21 @@ export default class Frame extends TypedEventEmitter<IFrameEvents> implements IP
   private setLoader(loaderId: string) {
     if (!loaderId) return;
     if (loaderId === this.activeLoaderId) return;
-    this.loaderLifecycles.set(loaderId, {});
-    this.activeLoaderId = loaderId;
-    const newResolver = createPromise();
-    if (loaderId !== 'inpage') {
-      const promise = newResolver.promise;
-      newResolver.promise = this.createIsolatedWorld().then(promise as any);
-      if (!this.getActiveContextId(false)) {
-        newResolver.promise = this.waitForDefaultContext().then(newResolver.promise as any);
+    if (loaderId === 'inpage' || !this.loaderIdResolvers.has(loaderId)) {
+      this.loaderLifecycles.set(loaderId, {});
+      const newResolver = createPromise();
+      if (loaderId !== 'inpage') {
+        const chain = newResolver.promise;
+        newResolver.promise = this.createIsolatedWorld().then(() => chain);
+      }
+
+      if (this.activeLoader && !this.activeLoader.isResolved) {
+        this.activeLoader.resolve(newResolver.promise);
       }
-    }
 
-    this.loaderIdResolvers.set(loaderId, newResolver);
+      this.loaderIdResolvers.set(loaderId, newResolver);
+    }
+    this.activeLoaderId = loaderId;
   }
 
   private async createIsolatedWorld() {

puppet-chrome/lib/FramesManager.ts

@@ -137,7 +137,8 @@ export default class FramesManager extends TypedEventEmitter<IPuppetFrameEvents>
     if (isInitiatingNavigation) {
       frame.initiateNavigation(url, loaderId);
     }
-    await frame.waitForLoader(loaderId);
+    const loaderError = await frame.waitForLoader(loaderId);
+    if (loaderError) throw loaderError;
   }
 
   public getFrameIdForExecutionContext(executionContextId: number) {

puppet-chrome/lib/NetworkManager.ts

@@ -103,7 +103,7 @@ export class NetworkManager extends TypedEventEmitter<IPuppetNetworkEvents> {
       })
       .catch(error => {
         if (error instanceof CanceledPromiseError) return;
-        this.logger.warn('NetworkManager.continueWithAuthError', {
+        this.logger.info('NetworkManager.continueWithAuthError', {
           error,
           requestId: event.requestId,
           url: event.request.url,
@@ -118,7 +118,7 @@ export class NetworkManager extends TypedEventEmitter<IPuppetNetworkEvents> {
       })
       .catch(error => {
         if (error instanceof CanceledPromiseError) return;
-        this.logger.warn('NetworkManager.continueRequestError', {
+        this.logger.info('NetworkManager.continueRequestError', {
           error,
           requestId: networkRequest.requestId,
           url: networkRequest.request.url,

puppet/interfaces/IPuppetFrame.ts

@@ -8,7 +8,7 @@ export interface IPuppetFrame {
   navigationReason?: string;
   disposition?: string;
   securityOrigin: string;
-  waitForLoader(loaderId?: string): Promise<void>;
+  waitForLoader(loaderId?: string): Promise<Error | undefined>;
   evaluate<T>(expression: string, isolateFromWebPageEnvironment?: boolean): Promise<T>;
 }
 

puppet/test/Frames.test.ts

@@ -430,7 +430,7 @@ describe.each([
       expect(frame.url).toBe(server.emptyPage);
     });
 
-    it('should work with goto following click', async () => {
+    it('should be able to navigate directly following click', async () => {
       server.setRoute('/login.html', async (req, res) => {
         res.setHeader('Content-Type', 'text/html');
         res.end(`You are logged in`);
@@ -445,7 +445,8 @@ describe.each([
       await page.click('input[type=text]');
       await page.keyboard.type('admin');
       await page.click('input[type=submit]');
-      await expect(page.goto(server.emptyPage)).resolves.toBe(undefined);
+
+      await expect(page.navigate(server.emptyPage)).resolves.toBe(undefined);
     });
   });
 });

puppet/test/TestPage.ts

@@ -53,8 +53,9 @@ export async function detachFrame(page: IPuppetPage, frameId: string) {
 }
 
 export async function goto(page: IPuppetPage, url: string, waitOnLifecycle = 'load') {
-  const nav = page.waitOn('frame-lifecycle', event => event.name === waitOnLifecycle);
-  await Promise.all([page.navigate(url), nav]);
+  const nav = page.navigate(url);
+  const lifecycle = page.waitOn('frame-lifecycle', event => event.name === waitOnLifecycle);
+  await Promise.all([lifecycle, nav]);
 }
 
 export async function setContent(page: IPuppetPage, content: string) {