Merge pull request #1135 from asherdrake/fix/single-pass-screenshots

amhsirak · web-flow · commit 6d7b9521d2b9 · 2026-07-09T16:08:11.000+05:30
fix: capture crawl/search screenshots in single scraping pass reducing time from 2N pages just down to N (closes #1105)
diff --git a/maxun-core/src/interpret.ts b/maxun-core/src/interpret.ts
@@ -43,6 +43,12 @@ interface InterpreterOptions {
   binaryCallback: (output: any, mimeType: string) => (void | Promise<void>);
   debug: boolean;
   type?: 'extract' | 'scrape' | 'crawl' | 'search' | 'doc-extract' | 'doc-parse';
+  /**
+   * This outputs the user's pereference for screenshooting a website
+   * This let's the crawl/search capture the screenshot during the first pass instead
+   * of having to go and revisit every page afterwards (#1105). This reduces our 2N scan time.
+   */
+  formats?: string[];
   debugChannel: Partial<{
     activeId: (id: number) => void,
     debugMessage: (msg: string) => void,
@@ -385,14 +391,16 @@ export default class Interpreter extends EventEmitter {
    * Returns the optimal Playwright `waitUntil` navigation strategy based on
    * whether the current operation requires visual rendering.
    *
-   * - `'networkidle'`            — used when screenshots are requested; waits for all
-   *                         sub-resources so the page renders correctly.
+   * - `'networkidle'`            — used when `visualRenderRequired` (extract mode) is set;
+   *                         waits for all sub-resources so the page renders correctly.
+   *                         Screenshot capture during crawl/search handles its own
+   *                         networkidle wait separately, in captureScreenshotsForCurrentPage.
    * - `'domcontentloaded'` — used for all DOM-only operations (scraping, crawling,
    *                         extraction, search); skips stylesheet/image loading for
    *                         maximum speed.
    *
-   * @param blockOverride Pass `true` when the caller will take a screenshot
-   *                          or requires styled layout. Defaults to `false`.
+   * @param blockOverride Pass `true` to force a fully-rendered wait regardless of
+   *                          `visualRenderRequired`. Defaults to `false`.
    */
   private getNavigationWaitStrategy(
     blockOverride?: boolean
@@ -426,6 +434,64 @@ export default class Interpreter extends EventEmitter {
     });
   }
 
+  /**
+   * If user selected either screenshot-visible or screenshot-fullpage output format,
+   * this returns true.
+   */
+  private screenshotFormatsRequested(): boolean {
+    const formats = this.options.formats ?? [];
+    return formats.includes('screenshot-visible') || formats.includes('screenshot-fullpage');
+  }
+
+  private async captureScreenshotsForCurrentPage(
+    page: Page,
+    keyPrefix: 'crawl' | 'search',
+    pageNumber: number,
+  ): Promise<{ screenshotVisible?: string; screenshotFullpage?: string }> {
+    const keys: { screenshotVisible?: string; screenshotFullpage?: string } = {};
+    if (!this.screenshotFormatsRequested()) return keys;
+
+    const formats = this.options.formats ?? [];
+
+    // Bounded, non-fatal render-readiness before screenshotting.
+    // A capped networkidle race lets never-idle pages (chat/ads/websockets) settle
+    // without hanging navigation; timeouts here are swallowed, never fatal. Navigation
+    // stays on the fast domcontentloaded path (see the crawl/search goto calls).
+    await page.waitForLoadState('networkidle', { timeout: 10000 }).catch(() => {});
+    await this.waitForImagesLoaded(page);
+    await new Promise(resolve => setTimeout(resolve, 1000));
+
+    if (formats.includes('screenshot-visible')) {
+      try {
+        const buffer = await page.screenshot({ type: 'png' });
+        const key = `${keyPrefix}-${pageNumber}-screenshot-visible`;
+        await this.options.binaryCallback(
+          { name: key, data: buffer, mimeType: 'image/png' },
+          'image/png',
+        );
+        keys.screenshotVisible = key;
+      } catch (error: any) {
+        this.log(`Failed to capture visible screenshot of ${page.url()}: ${error.message}`, Level.WARN);
+      }
+    }
+
+    if (formats.includes('screenshot-fullpage')) {
+      try {
+        const buffer = await page.screenshot({ type: 'png', fullPage: true });
+        const key = `${keyPrefix}-${pageNumber}-screenshot-fullpage`;
+        await this.options.binaryCallback(
+          { name: key, data: buffer, mimeType: 'image/png' },
+          'image/png',
+        );
+        keys.screenshotFullpage = key;
+      } catch (error: any) {
+        this.log(`Failed to capture full-page screenshot of ${page.url()}: ${error.message}`, Level.WARN);
+      }
+    }
+
+    return keys;
+  }
+
   /**
    * Returns true if any of the remaining blocks in the workflow require a visual render
    * before the next page navigation.
@@ -1484,6 +1550,12 @@ export default class Interpreter extends EventEmitter {
               pageResult.metadata.depth = depth;
               crawlResults.push(pageResult);
 
+              // Screenshots are taken here instead of 2N after every crawl (#1105).
+              Object.assign(
+                pageResult,
+                await this.captureScreenshotsForCurrentPage(page, 'crawl', crawlResults.length),
+              );
+
               this.log(`✓ Scraped ${url} (${pageResult.wordCount} words, depth ${depth})`, Level.LOG);
 
               if (crawlConfig.followLinks && depth < crawlConfig.maxDepth) {
@@ -1830,8 +1902,8 @@ export default class Interpreter extends EventEmitter {
               await page.goto(result.url, {
                 waitUntil: this.getNavigationWaitStrategy(),
                 timeout: 30000
-              }).catch(() => {
-                this.log(`Failed to navigate to ${result.url}, skipping...`, Level.WARN);
+              }).catch((err) => {
+                throw new Error(`Navigation failed: ${err.message}`);
               });
 
               await this.waitForPageReady(page, currentWorkflow || []);
@@ -1883,7 +1955,7 @@ export default class Interpreter extends EventEmitter {
                 };
               });
 
-              scrapedResults.push({
+              const scrapedEntry = {
                 searchResult: {
                   query: searchConfig.query,
                   position: result.position,
@@ -1900,7 +1972,14 @@ export default class Interpreter extends EventEmitter {
                 links: pageData.links,
                 wordCount: pageData.wordCount,
                 scrapedAt: new Date().toISOString()
-              });
+              };
+              scrapedResults.push(scrapedEntry);
+
+              // Screenshots are taken here instead of 2N after every crawl (#1105).
+              Object.assign(
+                scrapedEntry,
+                await this.captureScreenshotsForCurrentPage(page, 'search', i + 1),
+              );
 
               this.log(`✓ Scraped ${result.url} (${pageData.wordCount} words)`, Level.LOG);
 
diff --git a/server/src/api/record.ts b/server/src/api/record.ts
@@ -1150,7 +1150,6 @@ async function executeRun(id: string, userId: string) {
                         crawl: categorizedOutput.crawl as Record<string, any>,
                         search: categorizedOutput.search as Record<string, any>,
                     },
-                    currentPage,
                     initialBinaryOutput: postBinaryOutput,
                     llmConfig,
                 });
diff --git a/server/src/routes/storage.ts b/server/src/routes/storage.ts
@@ -1051,6 +1051,19 @@ router.put('/runs/:id', requireSignIn, async (req: AuthenticatedRequest, res) =>
       return res.status(404).send({ error: 'Recording not found' });
     }
 
+    // Validate formats once before persisting; a malformed value (e.g. an object)
+    // would otherwise reach the interpreter and break formats.includes(...).
+    let interpreterFormats = (recording.recording_meta as any).formats;
+    if (req.body?.formats !== undefined) {
+      const { validFormats, invalidFormats } = parseOutputFormats(req.body.formats);
+      if (invalidFormats.length > 0) {
+        return res.status(400).json({
+          error: `Invalid formats: ${invalidFormats.map(String).join(', ')}`,
+        });
+      }
+      interpreterFormats = validFormats;
+    }
+
     // Generate runId first
     const runId = uuid();
     
@@ -1081,8 +1094,8 @@ router.put('/runs/:id', requireSignIn, async (req: AuthenticatedRequest, res) =>
           robotMetaId: recording.recording_meta.id,
           startedAt: new Date().toLocaleString(),
           finishedAt: '',
-          browserId: browserId, 
-          interpreterSettings: req.body,
+          browserId: browserId,
+          interpreterSettings: { ...req.body, formats: interpreterFormats },
           log: '',
           runId,
           runByUserId: req.user.id,
@@ -1147,7 +1160,7 @@ router.put('/runs/:id', requireSignIn, async (req: AuthenticatedRequest, res) =>
         startedAt: new Date().toLocaleString(),
         finishedAt: '',
         browserId,
-        interpreterSettings: req.body,
+        interpreterSettings: { ...req.body, formats: interpreterFormats },
         log: 'Run queued - waiting for available browser slot',
         runId,
         runByUserId: req.user.id,
diff --git a/server/src/storage/mino.ts b/server/src/storage/mino.ts
@@ -38,6 +38,26 @@ async function fixMinioBucketConfiguration(bucketName: string) {
   }
 }
 
+/**
+ * Bucket existence/policy is external, process-global state, so the check only
+ * needs to succeed once per bucket per process. The in-flight promise is cached
+ * so concurrent first uploads share a single check; a failed check is evicted
+ * so the next upload retries instead of caching the failure.
+ */
+const bucketConfigPromises = new Map<string, Promise<void>>();
+
+function ensureBucketConfigured(bucketName: string): Promise<void> {
+  let promise = bucketConfigPromises.get(bucketName);
+  if (!promise) {
+    promise = fixMinioBucketConfiguration(bucketName).catch((error) => {
+      bucketConfigPromises.delete(bucketName);
+      throw error;
+    });
+    bucketConfigPromises.set(bucketName, promise);
+  }
+  return promise;
+}
+
 minioClient.bucketExists('maxun-test')
   .then((exists) => {
     if (exists) {
@@ -90,8 +110,54 @@ class BinaryOutputService {
     this.bucketName = bucketName;
   }
 
+  /**
+   * Uploads a single binary item to MinIO and returns its public URL.
+   * Never throws: a null return lets callers fall back to keeping the
+   * base64 payload, which the end-of-run bulk upload will retry.
+   * @param runId - The run ID used to namespace the object key.
+   * @param key - The binary output key (also the object name within the run).
+   * @param data - The raw binary data to upload.
+   * @param mimeType - The MIME type of the data (defaults to image/png).
+   * @returns The public URL of the uploaded object, or null on failure.
+   */
+  async uploadBinaryOutputItem(runId: string, key: string, data: Buffer, mimeType?: string): Promise<string | null> {
+    try {
+      await ensureBucketConfigured(this.bucketName);
+
+      const minioKey = `${runId}/${encodeURIComponent(key.trim().replace(/\s+/g, '_'))}`;
+
+      console.log(`Uploading to bucket ${this.bucketName} with key ${minioKey}`);
+
+      await minioClient.putObject(
+        this.bucketName,
+        minioKey,
+        data,
+        data.length,
+        { 'Content-Type': mimeType || 'image/png' }
+      );
+
+      // Build the browser-facing object URL. Prefer a complete public base URL
+      // (MINIO_PUBLIC_URL) so deployments behind a reverse proxy or on a
+      // non-default public port are not forced onto the internal MinIO port —
+      // which produced unreachable "localhost:9000" links (#832). Falls back to
+      // the existing host:port composition for local/dev setups.
+      const publicBase = (
+        process.env.MINIO_PUBLIC_URL
+          ? process.env.MINIO_PUBLIC_URL
+          : `${process.env.MINIO_PUBLIC_HOST || 'http://localhost'}:${process.env.MINIO_PORT || '9000'}`
+      ).replace(/\/+$/, '');
+
+      return `${publicBase}/${this.bucketName}/${minioKey}`;
+    } catch (error) {
+      console.error(`❌ Error uploading key ${key} to MinIO:`, error);
+      return null;
+    }
+  }
+
   /**
    * Uploads binary data to Minio and stores references in PostgreSQL.
+   * Entries that are already public URLs (uploaded per-page during the run)
+   * are preserved as-is, making this call idempotent for them.
    * @param run - The run object representing the current process.
    * @param binaryOutput - The binary output object containing data to upload.
    * @returns A map of Minio URLs pointing to the uploaded binary data.
@@ -110,6 +176,18 @@ class BinaryOutputService {
 
       console.log(`Processing binary output key: ${key}`);
 
+      // Entries uploaded per-page during the run are already public URLs —
+      // pass them through untouched, so the run.update below (a full replace
+      // of run.binaryOutput) cannot wipe them.
+      if (typeof binaryData === 'string' && /^https?:\/\//.test(binaryData)) {
+        uploadedBinaryOutput[key] = binaryData;
+        continue;
+      }
+      if (binaryData && typeof binaryData.data === 'string' && /^https?:\/\//.test(binaryData.data)) {
+        uploadedBinaryOutput[key] = binaryData.data;
+        continue;
+      }
+
       // Convert binary data to Buffer (handles base64, data URI, and old Buffer format)
       let bufferData: Buffer | null = null;
 
@@ -151,38 +229,13 @@ class BinaryOutputService {
         continue;
       }
 
-      try {
-        await fixMinioBucketConfiguration(this.bucketName);
-
-        const minioKey = `${plainRun.runId}/${encodeURIComponent(key.trim().replace(/\s+/g, '_'))}`;
-
-        console.log(`Uploading to bucket ${this.bucketName} with key ${minioKey}`);
-
-        await minioClient.putObject(
-          this.bucketName,
-          minioKey,
-          bufferData,
-          bufferData.length,
-          { 'Content-Type': binaryData.mimeType || 'image/png' }
-        );
-
-        // Build the browser-facing object URL. Prefer a complete public base URL
-        // (MINIO_PUBLIC_URL) so deployments behind a reverse proxy or on a
-        // non-default public port are not forced onto the internal MinIO port —
-        // which produced unreachable "localhost:9000" links (#832). Falls back to
-        // the existing host:port composition for local/dev setups.
-        const publicBase = (
-          process.env.MINIO_PUBLIC_URL
-            ? process.env.MINIO_PUBLIC_URL
-            : `${process.env.MINIO_PUBLIC_HOST || 'http://localhost'}:${process.env.MINIO_PORT || '9000'}`
-        ).replace(/\/+$/, '');
-        const publicUrl = `${publicBase}/${this.bucketName}/${minioKey}`;
-
+      const publicUrl = await this.uploadBinaryOutputItem(plainRun.runId, key, bufferData, binaryData.mimeType);
+      if (publicUrl) {
         uploadedBinaryOutput[key] = publicUrl;
-
         console.log(`✅ Uploaded and stored: ${publicUrl}`);
-      } catch (error) {
-        console.error(`❌ Error uploading key ${key} to MinIO:`, error);
+      } else {
+        // Keep the base64 entry rather than dropping it, so run.update doesn't remove it
+        uploadedBinaryOutput[key] = binaryData;
       }
     }
 
diff --git a/server/src/task-runner.ts b/server/src/task-runner.ts
@@ -437,7 +437,6 @@ async function processRunExecution(data: ExecuteRunData): Promise<void> {
           robotType,
           outputFormats,
           categorizedOutput: { crawl: categorizedOutput.crawl as Record<string, any>, search: categorizedOutput.search as Record<string, any> },
-          currentPage,
           initialBinaryOutput: binaryOutput,
           llmConfig: {
             provider: ((recording.recording_meta as any).promptLlmProvider || 'ollama') as 'anthropic' | 'openai' | 'ollama',
diff --git a/server/src/types/index.ts b/server/src/types/index.ts
@@ -8,6 +8,7 @@ export interface InterpreterSettings {
     maxConcurrency: number;
     maxRepeats: number;
     debug: boolean;
+    formats?: string[];
     params?: any;
 }
 
diff --git a/server/src/utils/output-post-processor.ts b/server/src/utils/output-post-processor.ts
diff --git a/server/src/workflow-management/classes/Interpreter.ts b/server/src/workflow-management/classes/Interpreter.ts
diff --git a/server/src/workflow-management/scheduler/index.ts b/server/src/workflow-management/scheduler/index.ts

Original file line number	Diff line number	Diff line change
`@@ -8,6 +8,7 @@ export interface InterpreterSettings {`
`8`	`8`	`maxConcurrency: number;`
`9`	`9`	`maxRepeats: number;`
`10`	`10`	`debug: boolean;`
	`11`	`+ formats?: string[];`
`11`	`12`	`params?: any;`
`12`	`13`	`}`
`13`	`14`