Skip to content

fix: Improve navigate_page_history error messages #321

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
+24 −6
Open

fix: Improve navigate_page_history error messages #321

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
05a434a
Refactor: Remove onRelease functionality from Mutex
mithun50 Oct 2, 2025
51a1761
Fix Mutex formatting and update docs
mithun50 Oct 2, 2025
57df66f
Improve navigate_page_history error messages
mithun50 Oct 9, 2025
b102fee
Format navigate_page_history code
mithun50 Oct 9, 2025
978540f
Merge branch 'main' into main
mithun50 Oct 9, 2025
44a7126
Merge branch 'main' into main
mithun50 Oct 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 24 additions & 6 deletions src/tools/pages.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,16 +142,34 @@ export const navigatePageHistory = defineTool({
const options = {
timeout: request.params.timeout,
};

try {
let result;
if (request.params.navigate === 'back') {
await page.goBack(options);
result = await page.goBack(options);
} else {
result = await page.goForward(options);
}

// If result is null, navigation wasn't possible (no history)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is not correct. Successful navigations could result in a null result if the navigation has no network request. We fixed Puppeteer in puppeteer/puppeteer#14160 to throw an error when there is no history. So the null result, should be treated as a successful navigation.

if (result === null) {
const direction =
request.params.navigate === 'back' ? 'previous' : 'next';
response.appendResponseLine(
`Cannot navigate ${request.params.navigate}, no ${direction} page in history.`,
);
}
} catch (error) {
// Provide more specific error messages based on the error type
if (error.message && error.message.includes('timeout')) {
response.appendResponseLine(
`Navigation ${request.params.navigate} timed out after ${request.params.timeout || 30000}ms.`,
);
} else {
await page.goForward(options);
response.appendResponseLine(
`Unable to navigate ${request.params.navigate}: ${error.message || 'Unknown error occurred'}`,
);
}
} catch {
response.appendResponseLine(
`Unable to navigate ${request.params.navigate} in currently selected page.`,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be sufficient to add the error.message to this message and revert other changes

);
}

response.setIncludePages(true);
Expand Down