fix(client): resource timeout + blank new tab

Author: blakebyrnes

client/connections/ConnectionToCore.ts

@@ -294,8 +294,8 @@ export default abstract class ConnectionToCore extends TypedEventEmitter<{
 
   protected cancelPendingRequests(): void {
     const host = String(this.resolvedHost);
-    this.coreSessions.close(new DisconnectedFromCoreError(host));
-    this.commandQueue.clearPending(new DisconnectedFromCoreError(host));
+    this.coreSessions.stop(new DisconnectedFromCoreError(host));
+    this.commandQueue.stop(new DisconnectedFromCoreError(host));
     for (const entry of this.pendingRequestsById.values()) {
       const id = entry.id;
       if (this.connectRequestId === id || this.disconnectRequestId === id) {

client/lib/Agent.ts

@@ -37,6 +37,7 @@ import CoreSession from './CoreSession';
 import IAgentConfigureOptions from '../interfaces/IAgentConfigureOptions';
 import ConnectionFactory from '../connections/ConnectionFactory';
 import ConnectionToCore from '../connections/ConnectionToCore';
+import DisconnectedFromCoreError from '../connections/DisconnectedFromCoreError';
 
 export const DefaultOptions = {
   defaultBlockedResourceTypes: [BlockedResourceType.None],
@@ -148,12 +149,17 @@ export default class Agent extends AwaitedEventTarget<{ close: void }> {
 
   // METHODS
 
-  public close(): Promise<void> {
+  public async close(): Promise<void> {
     const { isClosing, connection } = getState(this);
     if (isClosing) return;
     setState(this, { isClosing: true });
 
-    return connection.close();
+    try {
+      return await connection.close();
+    } catch (error) {
+      if (error instanceof DisconnectedFromCoreError) return;
+      throw error;
+    }
   }
 
   public async closeTab(tab: Tab): Promise<void> {

client/lib/CoreCommandQueue.ts

@@ -33,7 +33,7 @@ export default class CoreCommandQueue {
     });
   }
 
-  public clearPending(cancelError: CanceledPromiseError): void {
+  public stop(cancelError: CanceledPromiseError): void {
     this.internalQueue.stop(cancelError);
   }
 

client/lib/CoreSessions.ts

@@ -28,7 +28,7 @@ export default class CoreSessions {
     return this.sessionsById.get(sessionId);
   }
 
-  public close(closeError: Error): boolean {
+  public stop(closeError: Error): boolean {
     const hasSessions = this.sessionsById.size > 0;
     this.queue.stop(closeError);
     this.sessionsById.clear();

client/lib/Resource.ts

@@ -84,25 +84,44 @@ export default class Resource {
     const timer = new Timer(options?.timeoutMs ?? 30e3);
 
     const resourceFilter = { url: filter.url, type: filter.type };
-    const resourceOptions = {
-      ...(options ?? {}),
+    const resourceOptions: IWaitForResourceOptions = {
+      sinceCommandId: options?.sinceCommandId,
       timeoutMs: 2e3,
       throwIfTimeout: false,
-    } as IWaitForResourceOptions;
+    };
 
     let isComplete = false;
     const done = (): boolean => (isComplete = true);
 
     do {
-      let foundResources: IResourceMeta[] = [];
-
       try {
         const waitForResourcePromise = coreTab.waitForResource(resourceFilter, resourceOptions);
-        foundResources = await timer.waitForPromise(
+        const foundResources = await timer.waitForPromise(
           waitForResourcePromise,
           'Timeout waiting for Resource(s)',
         );
         resourceOptions.sinceCommandId = coreTab.commandQueue.lastCommandId;
+
+        for (const resourceMeta of foundResources) {
+          if (idsSeen.has(resourceMeta.id)) continue;
+          idsSeen.add(resourceMeta.id);
+
+          const resource = createResource(resourceMeta, Promise.resolve(coreTab));
+
+          let shouldInclude = true;
+
+          if (filter.filterFn) {
+            // resources can trigger commandQueue functions, so time them out
+            shouldInclude = await timer.waitForPromise(
+              Promise.resolve(filter.filterFn(resource, done)),
+              'Timeout waiting for waitResource.filterFn',
+            );
+          }
+
+          if (shouldInclude) resources.push(resource);
+
+          if (isComplete) break;
+        }
       } catch (err) {
         if (err instanceof TimeoutError) {
           if (options?.throwIfTimeout === false) {
@@ -112,22 +131,6 @@ export default class Resource {
         throw err;
       }
 
-      for (const resourceMeta of foundResources) {
-        if (idsSeen.has(resourceMeta.id)) continue;
-        idsSeen.add(resourceMeta.id);
-
-        const resource = createResource(resourceMeta, Promise.resolve(coreTab));
-
-        if (filter.filterFn) {
-          const shouldIncludeResource = await filter.filterFn(resource, done);
-          if (shouldIncludeResource) resources.push(resource);
-        } else {
-          resources.push(resource);
-        }
-
-        if (isComplete) break;
-      }
-
       // if no filter callback provided, break after 1 found
       if (!filter.filterFn && resources.length) {
         done();

commons/Timer.ts

@@ -50,14 +50,12 @@ export default class Timer {
     return time[0] * 1000 + time[1] / 1000000;
   }
 
-  public waitForPromise<Z>(promise: Promise<Z>, message: string): Promise<Z> {
+  public async waitForPromise<Z>(promise: Promise<Z>, message: string): Promise<Z> {
     this.timeoutMessage = message;
-    return Promise.race([
-      promise,
-      this.expirePromise.then(() => {
-        throw new TimeoutError(this.timeoutMessage);
-      }),
-    ]) as Promise<Z>;
+    const timeout = new TimeoutError(this.timeoutMessage);
+    const result = await Promise.race([promise, this.expirePromise.then(() => timeout)]);
+    if (result instanceof TimeoutError) throw timeout;
+    return result;
   }
 
   public waitForTimeout(): Promise<void> {

core/lib/Session.ts

@@ -69,7 +69,6 @@ export default class Session extends TypedEventEmitter<{
   }
 
   private _isClosing = false;
-  private pendingNavigationMitmResponses: IRequestSessionResponseEvent[] = [];
 
   constructor(readonly options: ICreateTabOptions) {
     super();
@@ -210,8 +209,6 @@ export default class Session extends TypedEventEmitter<{
       sessionId: this.id,
     });
 
-    this.pendingNavigationMitmResponses.forEach(x => this.onMitmResponse(x));
-
     await this.mitmRequestSession.close();
     await Promise.all(Object.values(this.tabs).map(x => x.close()));
     try {
@@ -279,12 +276,10 @@ export default class Session extends TypedEventEmitter<{
     const tabId = this.mitmRequestSession.browserRequestMatcher.requestIdToTabId.get(
       event.browserRequestId,
     );
-    let tab = this.tabs.find(x => x.id === tabId);
-    if (!tab && event.browserRequestId === 'fallback-navigation') {
-      tab = this.tabs.find(x => x.url === event.request.url || x.url === event.redirectedToUrl);
-      if (!tab) {
-        return this.pendingNavigationMitmResponses.push(event);
-      }
+    const tab = this.tabs.find(x => x.id === tabId);
+    if (!tab && !tabId) {
+      this.logger.warn(`Mitm Response received without matching tab`, { event });
+      return;
     }
 
     const resource = this.sessionState.captureResource(tab?.id ?? tabId, event, true);
@@ -320,21 +315,7 @@ export default class Session extends TypedEventEmitter<{
     this.sessionState.captureTab(tab.id, page.id, page.devtoolsSessionId, parentTab.id);
     this.registerTab(tab, page);
     await tab.isReady;
-    await page.mainFrame.waitForLoader();
-    const startUrl = page.mainFrame.url;
     parentTab.emit('child-tab-created', tab);
-    // make sure we match browser requests that weren't associated with a tab to the new tab
-    if (this.pendingNavigationMitmResponses.length) {
-      const replayPending = [...this.pendingNavigationMitmResponses];
-      this.pendingNavigationMitmResponses.length = 0;
-      while (replayPending.length) {
-        const next = replayPending.pop();
-        if (next.redirectedToUrl === startUrl || next.request.url === startUrl) {
-          const resource = this.sessionState.captureResource(tab.id, next, true);
-          tab.emit('resource', resource);
-        }
-      }
-    }
     return tab;
   }
 

core/lib/Tab.ts

@@ -428,6 +428,7 @@ b) Use the UserProfile feature to set cookies for 1 or more domains before they'
     // last command is the one running right now
     const startCommandId = options?.sinceCommandId ?? this.lastCommandId - 1;
     let newTab: Tab;
+    const startTime = new Date();
     if (startCommandId >= 0) {
       for (const tab of this.session.tabs) {
         if (tab.parentTabId === this.id && tab.createdAtCommandId >= startCommandId) {
@@ -437,6 +438,15 @@ b) Use the UserProfile feature to set cookies for 1 or more domains before they'
       }
     }
     if (!newTab) newTab = await this.waitOn('child-tab-created', undefined, options?.timeoutMs);
+
+    // wait for a real url to be requested
+    if (newTab.url === 'about:blank' || !newTab.url) {
+      let timeoutMs = options?.timeoutMs ?? 10e3;
+      const millis = new Date().getTime() - startTime.getTime();
+      timeoutMs -= millis;
+      await newTab.navigations.waitOn('navigation-requested', null, timeoutMs).catch(() => null);
+    }
+
     await newTab.navigationsObserver.waitForNavigationResourceId();
     return newTab;
   }

core/models/DevtoolsMessagesTable.ts

@@ -80,7 +80,7 @@ export default class DevtoolsMessagesTable extends SqliteTable<IDevtoolsMessageR
 
       // clean out post data (we have these in resources table)
       if ((key === 'headers' || key === 'postData') && params.request) {
-        return 'OMITTED';
+        return 'SA_REMOVED_FOR_DB';
       }
       return value;
     }

full-client/test/resources.test.ts

@@ -63,4 +63,17 @@ describe('basic resource tests', () => {
     }
     await agent.close();
   });
+
+  it('cancels a pending resource on agent close', async () => {
+    const exampleUrl = `${koaServer.baseUrl}/test`;
+    const agent = await handler.createAgent();
+
+    await agent.goto(exampleUrl);
+
+    const waitForResource = agent.waitForResource({ type: 'Fetch' });
+    // eslint-disable-next-line jest/valid-expect
+    const waitError = expect(waitForResource).rejects.toThrowError('disconnected');
+    await agent.close();
+    await waitError;
+  });
 });