ulixee/secret-agent

3xx status code that represent successful redirects should be invisible to client code

andynuss opened this issue · 1 comments

I discovered, at least with the 301 status code, that secret-agent returns a goto response's status code of 301
for (some/all) pages that redirect successfully via 301, rather than the expected 200 as seen with playwright.

However, if we wait for DomContentLoaded and then use the plugin to evaluate the dom, we do get the expected
behavior, so that it appears that with one or more 3xx status codes, we have to ignore them as if they were 200s.

The example url that always shows this behavior is: https://httptoolkit.tech/blog/unblocking-node-with-unref


Here is a test snippet:

import { Agent, ConnectionFactory, ConnectionToCore, LocationStatus } from 'secret-agent';
import Path from 'path';
import Resource from '@secret-agent/client/lib/Resource';
import ResourceResponse from '@secret-agent/client/lib/ResourceResponse';

let sharedConnection: ConnectionToCore = null;

function getConnection (maxAgents: number): ConnectionToCore {
  if (sharedConnection !== null) return sharedConnection;
  sharedConnection = ConnectionFactory.createConnection({ maxConcurrency: maxAgents });
  return sharedConnection;
}

async function test(): Promise<void> {
  const agent: Agent = new Agent({
    connectionToCore: getConnection(1),
  });

  try {
    let pageResource: Resource;

    agent.use(Path.resolve(__dirname, 'simple-plugin'));
    pageResource = await agent.goto('https://httptoolkit.tech/blog/unblocking-node-with-unref');
    const pageResponse: ResourceResponse = pageResource.response;
    const status: number = await pageResponse.statusCode;
    console.log(`BUG: goto response status: ${status}, but expecting 200 as with playwright`);

    await agent.activeTab.waitForLoad(LocationStatus.DomContentLoaded);

    // @ts-ignore
    const html: string = await agent.evaluate(0, () => {
      // @ts-ignore
      const win: any = window;
      return win.document.documentElement.outerHTML;
    });

    // the idea is that if all the snippets are found in the eval result
    // it appears that secret-agent did what it supposed to, but IMO
    // it should return a status of 200, not 301 when the document body
    // is an actual 3xx document.
    //
    const snippets: string[] = [
      `It holds a queue of tasks to run`,
      `Occasionally you want to break out of that model`,
      `but it's very annoying to have to remember to carefully shut`,
      `This functions like a flag you can set on your timers`,
      `Although this can let you skip complicated cleanup processes`,
      `This can come with a small performance cost,`,
      `discover that your app is unexpectedly exiting half way through`,
    ];

    // ... and yes, all the flags are true, so this bug is an inconvenience
    // in comparison to playwright, not a showstopper
    const flags: boolean[] = snippets.map(snippet => html.indexOf(snippet) >= 0);
    console.log(`found flags for dom's html`, flags);

  } catch (e) {
    console.log('failure', e);
  } finally {
    await agent.close();
  }
}

(async () => {
  await test();
})();

and here is the simple-plugin.ts code:

import ClientPlugin from '@secret-agent/plugin-utils/lib/ClientPlugin';
import CorePlugin from '@secret-agent/plugin-utils/lib/CorePlugin';
import { ISendToCoreFn } from '@secret-agent/interfaces/IClientPlugin';
import { IOnClientCommandMeta } from '@secret-agent/interfaces/ICorePlugin';
import { IPuppetPage } from '@secret-agent/interfaces/IPuppetPage';
import { IPuppetFrame, ILifecycleEvents } from '@secret-agent/interfaces/IPuppetFrame';

export class MyClientPlugin extends ClientPlugin {

  static readonly id = 'my-evaluate-plugin';
  static coreDependencyIds = [MyClientPlugin.id];

  public onAgent(agent, sendToCore: ISendToCoreFn): void {

    agent.numFrames = (): Promise<number> => {
      return this.execFunc(sendToCore, 'numFrames');
    };

    agent.evaluate = (frameIndex: number, callback: Function, ...args: any[]): Promise<any> => {
      return this.execFunc(sendToCore, 'evaluate', frameIndex, callback, args);
    };
  }

  public onTab(tab, sendToCore: ISendToCoreFn): void {

    tab.numFrames = (): Promise<number> => {
      return this.execFunc(sendToCore, 'numFrames');
    };

    tab.evaluate = (frameIndex: number, callback: Function, ...args: any[]): Promise<any> => {
      return this.execFunc(sendToCore, 'evaluate', frameIndex, callback, args);
    };
  }

  private async execFunc(
    sendToCore: ISendToCoreFn,
    funcName: string,
    frameIndex?: number,
    callback?: Function,
    args?: any[]
  ): Promise<any> {
    const callbackStr: string = callback ? callback.toString() : undefined;
    return await sendToCore(MyClientPlugin.id, funcName, frameIndex, callbackStr, args);
  }
}

export class MyCorePlugin extends CorePlugin {

  static readonly id = 'my-evaluate-plugin';

  private hasWaited: boolean[] = [];

  private async getAwaitedFrame(page: IPuppetPage, frameIndex: number): Promise<IPuppetFrame | null> {
    if (frameIndex < 0) throw new Error('frameIndex is negative');
    const frames: IPuppetFrame[] = page.frames;
    if (frameIndex >= frames.length) return null;
    const frame: IPuppetFrame = frames[frameIndex];
    if (!frame) throw new Error('frameIndex in bounds but frame is ' + frame);
    if (!this.hasWaited[frameIndex]) {
      const key: keyof ILifecycleEvents = 'load';
      try {
        await frame.waitForLoad(key);
      } catch (e) {
        console.log('getAwaitedFrame error', e);
        throw e;
      }
      this.hasWaited[frameIndex] = true;
    }
    return frame;
  }

  public async onClientCommand(
    { puppetPage }: IOnClientCommandMeta,
    funcName: string,
    frameIndex: number,
    callback: string,         // this is the stringified value of the callback given to execFunc above
    args: any[]
  ): Promise<any> {
    const page: IPuppetPage = puppetPage;

    if (funcName === 'numFrames') {
      return page.frames.length;
    }

    if (funcName === 'evaluate') {
      const frame: IPuppetFrame = await this.getAwaitedFrame(page, frameIndex);
      if (!frame) return null;
      const expression: string = `((args) => {
        const fn = ${callback};
        const result = fn.apply(null, args);
        return result;
      })(${JSON.stringify(args)})`;
      try {
        return await frame.evaluate(expression, true);
      } catch (e) {
        console.log(`evaluate error: ${e.toString()}, frameIndex: ${frameIndex}`)
        throw e;
      }
    }

    throw new Error('invalid funcName: ' + funcName);
  }
}

Thanks for reporting this.