Repository navigation
Name the page behind a window a step fails on - #3444
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
| Layer / File(s) | Summary |
|---|---|
Resolve the opener page internal/runtime/builtin/browser/engine.go, internal/runtime/builtin/browser/stagehand.go |
The engine interface and Stagehand implementation resolve the opener page and expose its title and screenshot. |
Report opener details on failure internal/runtime/builtin/browser/ops.go, internal/runtime/builtin/browser/browser_test.go, internal/runtime/builtin/browser/fakes_test.go, conformance/spec072_browser/*, specs/072-browser.md |
Failure messages identify the opener page. When failure screenshots are enabled, the opener screenshot is saved and included when available. Tests and the conformance scenario cover failures on a window left open. |
Estimated code review effort: 3 (Moderate) | ~20 minutes
Possibly related PRs
- dagucloud/dagu#3442: Changes browser window selection and keeps operations on the opener while a temporary window is open.
Merge Risk: 
87155
-
No actionable merge-blocking risk was identified.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: naming the page behind a window when a browser step fails. Description check The description explains the change, motivation, implementation behavior, and tests. It does not use the template headings and omits the checklist and related-issues section, but it contains the main … Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description explains the change, motivation, implementation behavior, and tests. It does not use the template headings and omits the checklist and related-issues section, but it contains the main information needed to review the change.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 6 files. (2 skipped: 2 unsupported.)
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
87155b4 to
f180acd
Compare
A frames-based site's search opens a "searching" window and posts into a frame, whose answer closes the window. When the answer is an error, such as a search text over the site's length limit, the window stays open. Operations then work on the window, as on any window still open past its opener's load, so the step failed with "the page only shows a communicating dialog" and the error that told what went wrong stayed hidden behind it. A step that fails on a window another open page opened now names that page in its error and, when it takes screenshots on failure, saves it as NN-failure-opener.png beside the failure screenshot.
f180acd to
0fc496a
Compare
What
A browser step that fails while its operations work on a window another, still open, page opened now:
the page read is a window that the page "<title>" opened and that is still open in front of it, andNN-failure-opener.pngbesideNN-failure.png, and names the file in the error.Why
A frames-based site's search opens a "searching" window and posts into a frame, whose answer closes the window (the case #3442 holds). When the answer is an error, for example a search text over the site's length limit, the site never closes the window. Operations then work on the window, as they do on any window still open past its opener's load, so the step failed with an expect like "the page only shows a communicating dialog", and its screenshot showed only that window. The error that told what went wrong was on the page behind it, which nothing reported, so the cause looked like a timing problem rather than bad input.
How it was found
On a live procurement site, five of 26 items of a foreach failed on every attempt with "the page only shows the communicating dialog". The page behind the window read "input text is too long": the site accepts at most 25 characters in its search field.
Tests
TestFailureOnAWindowCapturesThePageBehindIt: the error names the opener and both screenshots are saved.TestBrowserNamesThePageBehindAWindowLeftOpen: the bids fixture answers a search with no region with an error that leaves the searching window open; the step fails naming the page behind it and its screenshot.TestBrowserSearchingWindowstill passes.Summary by cubic
A step that fails while working on a window another open page opened now names that page in its error, since the window hides it. When failure screenshots are enabled, it also saves that page as
NN-failure-opener.pngbeside the failure screenshot.Written for commit 0fc496a. Summary will update on new commits.
Summary by CodeRabbit