[PR #157] [CLOSED] fix: treat screenshot failure as non-critical in browser tool #200

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/157
Author: @atian8179
Created: 3/1/2026
Status: ❌ Closed

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

📝 Commits (1)

8e0c1da fix: treat screenshot failure as non-critical in browser tool

📊 Changes

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

View changed files

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

📄 Description

Problem

In backend/pkg/tools/browser.go, the ContentMD, ContentHTML, and Links methods run two concurrent operations: fetching page content and capturing a screenshot. If the screenshot fails (scraper unavailable, timeout, image too small), the function returns an error discarding successfully-fetched page content.

The AI agent receives an error instead of the content it needs. Screenshot is a non-critical side-effect — PutScreenshot already ignores errors at the call-site.

Reported in #149.

Solution

Log a warning on screenshot failure instead of returning an error. Content is preserved.

if errScreenshot != nil {
    log.Printf("Warning: screenshot failed for %s (content still available): %v", url, errScreenshot)
    screenshotName = ""
}

Changes

backend/pkg/tools/browser.go: 3 methods changed — ContentMD(), ContentHTML(), Links() (+6/-3)

Testing

Content succeeds + screenshot fails → content returned ✅
Both succeed → unchanged ✅
Content fails → error returned ✅

Closes #149

_{🔄 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/157 **Author:** [@atian8179](https://github.com/atian8179) **Created:** 3/1/2026 **Status:** ❌ Closed **Base:** `master` ← **Head:** `fix/browser-screenshot-non-blocking` --- ### 📝 Commits (1) - [`8e0c1da`](https://github.com/vxcontrol/pentagi/commit/8e0c1da11331524fc59408224cf91dd8d231ccd8) fix: treat screenshot failure as non-critical in browser tool ### 📊 Changes **1 file changed** (+6 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `backend/pkg/tools/browser.go` (+6 -3) </details> ### 📄 Description ## Problem In `backend/pkg/tools/browser.go`, the `ContentMD`, `ContentHTML`, and `Links` methods run two concurrent operations: fetching page content and capturing a screenshot. If the screenshot fails (scraper unavailable, timeout, image too small), the function returns an error **discarding successfully-fetched page content**. The AI agent receives an error instead of the content it needs. Screenshot is a non-critical side-effect — `PutScreenshot` already ignores errors at the call-site. Reported in #149. ## Solution Log a warning on screenshot failure instead of returning an error. Content is preserved. ```go if errScreenshot != nil { log.Printf("Warning: screenshot failed for %s (content still available): %v", url, errScreenshot) screenshotName = "" } ``` ## Changes - `backend/pkg/tools/browser.go`: 3 methods changed — `ContentMD()`, `ContentHTML()`, `Links()` (+6/-3) ## Testing - Content succeeds + screenshot fails → content returned ✅ - Both succeed → unchanged ✅ - Content fails → error returned ✅ Closes #149 --- <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 #200] [MERGED] test: add unit tests for pkg/terminal package #230

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.