fix(core): flaky tests, fix interact with string

Author: blakebyrnes

client/lib/Interactor.ts

@@ -49,7 +49,7 @@ export default class Interactor {
 }
 
 function convertToInteractionGroups(interactions: IInteractions): IInteractionGroups {
-  const lastPosition: IMousePosition = [0, 0];
+  let lastPosition: ICoreMousePosition = [0, 0];
   const interactionGroups: IInteractionGroups = [];
   interactions.forEach(interaction => {
     if (typeof interaction === 'string') {
@@ -58,6 +58,7 @@ function convertToInteractionGroups(interactions: IInteractions): IInteractionGr
       interactionGroups.push([interactionStep]);
     } else {
       const interactionGroup = convertInteractionToInteractionGroup(interaction);
+      lastPosition = interactionGroup[interactionGroup.length - 1].mousePosition;
       interactionGroups.push(interactionGroup);
     }
   });

core/lib/Session.ts

@@ -63,6 +63,14 @@ export default class Session extends TypedEventEmitter<{
     return this._isClosing;
   }
 
+  public mitmErrorsByUrl = new Map<
+    string,
+    {
+      resourceId: number;
+      event: IRequestSessionHttpErrorEvent;
+    }[]
+  >();
+
   private isolatedMitmProxy?: MitmProxy;
   private _isClosing = false;
 
@@ -292,14 +300,41 @@ export default class Session extends TypedEventEmitter<{
   }
 
   private onMitmError(event: IRequestSessionHttpErrorEvent) {
-    const tabId = this.mitmRequestSession.browserRequestMatcher.requestIdToTabId.get(
-      event.request.browserRequestId,
+    const { request } = event;
+    let tabId = this.mitmRequestSession.browserRequestMatcher.requestIdToTabId.get(
+      request.browserRequestId,
     );
+    const url = request.request?.url;
+    const isDocument = request?.resourceType === 'Document';
+    if (isDocument && !tabId) {
+      for (const tab of this.tabs) {
+        const isMatch = tab.findFrameWithUnresolvedNavigation(
+          request.browserRequestId,
+          request.request?.method,
+          url,
+          request.response?.url,
+        );
+        if (isMatch) {
+          tabId = tab.id;
+          break;
+        }
+      }
+    }
+
+    // record errors
+    const resource = this.sessionState.captureResourceError(tabId, request, event.error);
+    if (!request.browserRequestId && url) {
+      const existing = this.mitmErrorsByUrl.get(url) ?? [];
+      existing.push({
+        resourceId: resource.id,
+        event,
+      });
+      this.mitmErrorsByUrl.set(url, existing);
+    }
 
-    const resource = this.sessionState.captureResourceError(tabId, event.request, event.error);
-    if (event.request?.resourceType === 'Document') {
+    if (tabId && isDocument) {
       const tab = this.tabs.find(x => x.id === tabId);
-      tab?.checkForResolvedNavigation(event.request.browserRequestId, resource, event.error);
+      tab?.checkForResolvedNavigation(request.browserRequestId, resource, event.error);
     }
   }
 

core/lib/SessionState.ts

@@ -42,7 +42,7 @@ export default class SessionState {
   private readonly scriptInstanceMeta: IScriptInstanceMeta;
   private readonly createDate = new Date();
   private readonly frames: { [frameId: number]: IFrameRecord } = {};
-  private readonly resources: IResourceMeta[] = [];
+  private readonly resourcesById = new Map<number, IResourceMeta>();
   private readonly websocketMessages: IWebsocketResourceMessage[] = [];
   private websocketListeners: {
     [resourceId: string]: ((msg: IWebsocketResourceMessage) => any)[];
@@ -209,7 +209,7 @@ export default class SessionState {
     let resourceMeta = this.getResourceMeta(resourceId);
     if (!resourceMeta) {
       resourceMeta = convertedMeta;
-      this.resources.push(resourceMeta);
+      this.resourcesById.set(resourceMeta.id, resourceMeta);
     }
     if (convertedMeta.response) {
       resourceMeta.response ??= convertedMeta.response;
@@ -229,8 +229,28 @@ export default class SessionState {
     const resource = this.resourceEventToMeta(tabId, resourceEvent);
     this.db.resources.insert(tabId, resource, null, resourceEvent, error);
 
-    if (!this.getResourceMeta(resource.id)) {
-      this.resources.push(resource);
+    if (!this.resourcesById.has(resource.id)) {
+      this.resourcesById.set(resource.id, resource);
+    }
+    return resource;
+  }
+
+  public captureResourceRequestId(
+    resourceId: number,
+    browserRequestId: string,
+    tabId: number,
+  ): IResourceMeta {
+    const resource = this.resourcesById.get(resourceId);
+    if (resource) {
+      resource.tabId = tabId;
+
+      // NOTE: browserRequestId can be shared amongst redirects
+      this.browserRequestIdToResources[browserRequestId] ??= [];
+      this.browserRequestIdToResources[browserRequestId].push({
+        resourceId,
+        url: resource.url,
+      });
+      this.db.resources.updateResource(resourceId, { browserRequestId, tabId });
     }
     return resource;
   }
@@ -246,7 +266,7 @@ export default class SessionState {
     this.db.resources.insert(tabId, resource, resourceResponseEvent.body, resourceEvent);
 
     if (isResponse) {
-      this.resources.push(resource);
+      this.resourcesById.set(resource.id, resource);
     }
     return resource;
   }
@@ -271,9 +291,7 @@ export default class SessionState {
 
     if (browserRequestId) {
       // NOTE: browserRequestId can be shared amongst redirects
-      if (!this.browserRequestIdToResources[browserRequestId]) {
-        this.browserRequestIdToResources[browserRequestId] = [];
-      }
+      this.browserRequestIdToResources[browserRequestId] ??= [];
       this.browserRequestIdToResources[browserRequestId].push({
         resourceId: resourceEvent.id,
         url: request.url,
@@ -304,7 +322,7 @@ export default class SessionState {
 
   public getResourceLookupMap(tabId: number): { [method_url: string]: IResourceMeta[] } {
     const result: { [method_url: string]: IResourceMeta[] } = {};
-    for (const resource of this.resources) {
+    for (const resource of this.resourcesById.values()) {
       if (resource.tabId === tabId) {
         const key = `${resource.request.method}_${resource.request.url}`;
         result[key] ??= [];
@@ -315,15 +333,19 @@ export default class SessionState {
   }
 
   public getResources(tabId: number): IResourceMeta[] {
-    return this.resources.filter(x => x.tabId === tabId);
+    const resources: IResourceMeta[] = [];
+    for (const resource of this.resourcesById.values()) {
+      if (resource.tabId === tabId) resources.push(resource);
+    }
+    return resources;
   }
 
   public getResourceData(id: number, decompress: boolean): Promise<Buffer> {
     return this.db.resources.getResourceBodyById(id, decompress);
   }
 
   public getResourceMeta(id: number): IResourceMeta {
-    return this.resources.find(x => x.id === id);
+    return this.resourcesById.get(id);
   }
 
   ///////   FRAMES ///////

core/lib/Tab.ts

@@ -141,22 +141,38 @@ export default class Tab extends TypedEventEmitter<ITabEventParams> {
     browserRequestId: string,
     resource: IResourceMeta,
     error?: Error,
-  ) {
-    if (resource.request.method !== 'GET') return;
+  ): boolean {
+    const frame = this.findFrameWithUnresolvedNavigation(
+      browserRequestId,
+      resource.request?.method,
+      resource.request?.url,
+      resource.response?.url,
+    );
+    if (frame) {
+      frame.navigations.onResourceLoaded(resource.id, resource.response?.statusCode, error);
+      return true;
+    }
+    return false;
+  }
 
-    for (const frame of this.frameEnvironmentsById.values()) {
-      if (!frame.isAttached) continue;
+  public findFrameWithUnresolvedNavigation(
+    browserRequestId: string,
+    method: string,
+    requestedUrl: string,
+    finalUrl: string,
+  ): FrameEnvironment {
+    if (method !== 'GET') return null;
 
+    for (const frame of this.frameEnvironmentsById.values()) {
       const top = frame.navigations.top;
       if (!top || top.resourceId.isResolved) continue;
 
       if (
-        resource.response?.url === top.finalUrl ||
-        resource.request.url === top.requestedUrl ||
+        finalUrl === top.finalUrl ||
+        requestedUrl === top.requestedUrl ||
         browserRequestId === top.browserRequestId
       ) {
-        frame.navigations.onResourceLoaded(resource.id, resource.response?.statusCode, error);
-        break;
+        return frame;
       }
     }
   }
@@ -664,8 +680,29 @@ export default class Tab extends TypedEventEmitter<ITabEventParams> {
       event.resource.browserRequestId,
     );
 
-    // if we get a cached response, it might never hit mitm, so record now
     if (!resourcesWithBrowserRequestId?.length) {
+      // first check if this is a mitm error
+      const errorsMatchingUrl = this.session.mitmErrorsByUrl.get(event.resource.url.href);
+
+      // if this resource error-ed out,
+      for (let i = 0; i < errorsMatchingUrl?.length ?? 0; i += 1) {
+        const error = errorsMatchingUrl[i];
+        const request = error.event?.request?.request;
+        if (!request) continue;
+        if (
+          request.method === event.resource.method &&
+          Math.abs(request.timestamp - event.resource.requestTime.getTime()) < 500
+        ) {
+          errorsMatchingUrl.splice(i, 1);
+          this.sessionState.captureResourceRequestId(
+            error.resourceId,
+            event.resource.browserRequestId,
+            this.id,
+          );
+          return;
+        }
+      }
+
       const ctx = MitmRequestContext.createFromPuppetResourceRequest(event.resource);
       const resourceDetails = MitmRequestContext.toEmittedResource(ctx);
       if (!event.resource.browserServedFromCache) {

core/models/ResourcesTable.ts

@@ -50,6 +50,15 @@ export default class ResourcesTable extends SqliteTable<IResourcesRecord> {
     );
   }
 
+  public updateResource(id: number, data: { tabId: number; browserRequestId: string }): void {
+    if (this.hasPending(x => x[0] === id)) {
+      this.flush();
+    }
+    this.db
+      .prepare(`update ${this.tableName} set tabId=?, devtoolsRequestId=? where id=?`)
+      .run(data.tabId, data.browserRequestId, id);
+  }
+
   public insert(
     tabId: number,
     meta: IResourceMeta,

core/test/resources.test.ts

@@ -1,9 +1,15 @@
 import { Helpers } from '@secret-agent/testing';
 import Core, { Session } from '@secret-agent/core';
+import Resolvable from '@secret-agent/commons/Resolvable';
+import ConnectionToClient from '../server/ConnectionToClient';
 
+let connection: ConnectionToClient;
+beforeAll(() => {
+  connection = Core.addConnection();
+  Helpers.onClose(() => connection.disconnect(), true);
+});
 afterAll(Helpers.afterAll);
 afterEach(Helpers.afterEach);
-process.env.MITM_ALLOW_INSECURE = 'true';
 
 test('loads http2 resources', async () => {
   const server = await Helpers.runHttp2Server((req, res) => {
@@ -21,14 +27,46 @@ test('loads http2 resources', async () => {
     res.end(`<html><body><img src="/img.png"/></body></html>`);
   });
 
-  const connection = Core.addConnection();
-  Helpers.onClose(() => connection.disconnect());
   const meta = await connection.createSession();
   const tab = Session.getTab(meta);
+  const session = Session.get(meta.sessionId);
+  Helpers.needsClosing.push(session);
   await tab.goto(server.url);
   await tab.waitForLoad('DomContentLoaded');
 
   const resources = await tab.waitForResource({ url: /.*\/img.png/ });
   expect(resources).toHaveLength(1);
   expect(resources[0].type).toBe('Image');
+  await session.close();
+});
+
+test('records a single resource for failed mitm requests', async () => {
+  const meta = await connection.createSession();
+  const session = Session.get(meta.sessionId);
+  Helpers.needsClosing.push(session);
+  const tab = Session.getTab(meta);
+
+  const resolvable = new Resolvable<void>();
+  const originalEmit = tab.puppetPage.emit.bind(tab.puppetPage);
+  // @ts-ignore
+  jest.spyOn(tab.puppetPage.networkManager, 'emit').mockImplementation((evt, args) => {
+    // eslint-disable-next-line promise/always-return,promise/catch-or-return
+    resolvable.promise.then(() => {
+      originalEmit(evt, args);
+    });
+    return true;
+  });
+  const goToPromise = tab.goto(`http://localhost:2344/not-there`);
+
+  await expect(goToPromise).rejects.toThrowError();
+  expect(session.mitmErrorsByUrl.get(`http://localhost:2344/not-there`)).toHaveLength(1);
+  expect(session.sessionState.getResources(meta.tabId)).toHaveLength(1);
+  // @ts-ignore
+  expect(Object.keys(session.sessionState.browserRequestIdToResources)).toHaveLength(0);
+  resolvable.resolve();
+  await new Promise(resolve => setTimeout(resolve, 100));
+  expect(session.sessionState.getResources(meta.tabId)).toHaveLength(1);
+  // @ts-ignore
+  expect(Object.keys(session.sessionState.browserRequestIdToResources)).toHaveLength(1);
+  await session.close();
 });

full-client/test/interact.test.ts

@@ -169,4 +169,36 @@ describe('basic Interact tests', () => {
     await expect(agent.interact({ click: logo })).resolves.toBeUndefined();
     await agent.close();
   });
+
+  it('can accept empty commands', async () => {
+    koaServer.get('/empty-click', ctx => {
+      ctx.body = `
+        <body>
+           <div style="margin-top: 200px">
+             <a href="#none" onclick="clickit()">Empty clicker</a>
+           </div>
+          <script>
+            let lastClicked = '';
+            function clickit() {
+              lastClicked = 'clickedit';
+            }
+          </script>
+        </body>
+      `;
+    });
+    const agent = await handler.createAgent({
+      humanEmulatorId: 'basic',
+    });
+    Helpers.needsClosing.push(agent);
+    await agent.goto(`${koaServer.baseUrl}/empty-click`);
+    await agent.activeTab.waitForLoad(LocationStatus.PaintingStable);
+    await agent.interact(
+      {
+        [Command.move]: agent.document.querySelector('a'),
+      },
+      'click',
+    );
+
+    expect(await agent.activeTab.getJsValue('lastClicked')).toBe('clickedit');
+  });
 });

mitm/lib/DnsOverTlsSocket.ts

@@ -85,7 +85,9 @@ export default class DnsOverTlsSocket {
     this.mitmSocket.on('eof', async () => {
       removeEventListeners([registration]);
       this.mitmSocket.close();
-      await this.connect();
+      this.isConnected = this.connect();
+
+      await this.isConnected;
       // re-run pending queries
       for (const [id, entry] of this.pending) {
         this.pending.delete(id);