[PR #159] [CLOSED] fix(browser): treat screenshot failure as non-critical warning #201

New Issue

2026-06-06T22:09:40-04:00

yindo commented

2026-06-06 22:09:40 -04:00

📋 Pull Request Information

Original PR: https://github.com/vxcontrol/pentagi/pull/159
Author: @haosenwang1018
Created: 3/1/2026
Status: ❌ Closed

Base: master ← Head: fix/browser-screenshot-non-critical

📝 Commits (1)

ee8dcd1 fix(browser): treat screenshot failure as non-critical warning

📊 Changes

1 file changed (+12 additions, -3 deletions)

View changed files

📝 backend/pkg/tools/browser.go (+12 -3)

📄 Description

Summary

Fixes #149

The browser tool's ContentMD, ContentHTML, and Links methods run content fetching and screenshot capture concurrently. Currently, if the screenshot fails for any reason, the entire operation returns an error — discarding successfully-fetched page content.

This PR makes screenshot failure non-critical: when content is successfully fetched but the screenshot fails, a warning is logged and the content is returned with an empty screenshot name.

Problem

Screenshot failures can occur due to:

Scraper service being temporarily unavailable or restarting
Network timeout (65s hard timeout)
Target page producing a screenshot smaller than minImgContentSize (2048 bytes)
Disk write failure in writeScreenshotToFile

Since the caller (wrapCommandResult) already treats the screenshot as optional (_, _ = b.scp.PutScreenshot(...)), the screenshot result is already considered disposable at the call-site. Yet the producing function can abort the entire operation.

Changes

In ContentMD(), ContentHTML(), and Links():

Screenshot errors are now logged as warnings via logrus.WithError(...).Warnf()
screenshotName is set to empty string on failure
Valid content is returned instead of being discarded

Before / After

Before: Agent receives browser tool 'markdown' handled with error: failed to fetch screenshot... even though content was fetched successfully.

After: Agent receives the page content. A warning is logged for debugging.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/vxcontrol/pentagi/pull/159 **Author:** [@haosenwang1018](https://github.com/haosenwang1018) **Created:** 3/1/2026 **Status:** ❌ Closed **Base:** `master` ← **Head:** `fix/browser-screenshot-non-critical` --- ### 📝 Commits (1) - [`ee8dcd1`](https://github.com/vxcontrol/pentagi/commit/ee8dcd14567cc82615ad93f9e7721971591e59d1) fix(browser): treat screenshot failure as non-critical warning ### 📊 Changes **1 file changed** (+12 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `backend/pkg/tools/browser.go` (+12 -3) </details> ### 📄 Description ## Summary Fixes #149 The browser tool's `ContentMD`, `ContentHTML`, and `Links` methods run content fetching and screenshot capture concurrently. Currently, if the screenshot fails for any reason, the **entire operation returns an error** — discarding successfully-fetched page content. This PR makes screenshot failure non-critical: when content is successfully fetched but the screenshot fails, a warning is logged and the content is returned with an empty screenshot name. ## Problem Screenshot failures can occur due to: - Scraper service being temporarily unavailable or restarting - Network timeout (65s hard timeout) - Target page producing a screenshot smaller than `minImgContentSize` (2048 bytes) - Disk write failure in `writeScreenshotToFile` Since the caller (`wrapCommandResult`) already treats the screenshot as optional (`_, _ = b.scp.PutScreenshot(...)`), the screenshot result is already considered disposable at the call-site. Yet the producing function can abort the entire operation. ## Changes In `ContentMD()`, `ContentHTML()`, and `Links()`: - Screenshot errors are now logged as warnings via `logrus.WithError(...).Warnf()` - `screenshotName` is set to empty string on failure - Valid content is returned instead of being discarded ## Before / After **Before:** Agent receives `browser tool 'markdown' handled with error: failed to fetch screenshot...` even though content was fetched successfully. **After:** Agent receives the page content. A warning is logged for debugging. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

yindo added the pull-request label 2026-06-06 22:09:40 -04:00

yindo closed this issue

2026-06-06 22:09:40 -04:00

yindo referenced this issue

2026-06-06 22:09:48 -04:00

[PR #201] [MERGED] test: add unit tests for pkg/server/response package #231

yindo referenced this issue

2026-06-06 22:09:52 -04:00

[PR #222] [MERGED] Test Coverage & Performance Improvements #245

Sign in to join this conversation.