From 05a434aff716cdb01b7636508056ca54caedfaf6 Mon Sep 17 00:00:00 2001 From: mithun50 Date: Thu, 2 Oct 2025 15:54:29 +0530 Subject: [PATCH 1/6] Refactor: Remove onRelease functionality from Mutex --- src/Mutex.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/Mutex.ts b/src/Mutex.ts index 78c01633d..c5de4bca2 100644 --- a/src/Mutex.ts +++ b/src/Mutex.ts @@ -7,13 +7,10 @@ export class Mutex { static Guard = class Guard { #mutex: Mutex; - #onRelease?: () => void; - constructor(mutex: Mutex, onRelease?: () => void) { + constructor(mutex: Mutex) { this.#mutex = mutex; - this.#onRelease = onRelease; } dispose(): void { - this.#onRelease?.(); return this.#mutex.release(); } }; @@ -22,9 +19,7 @@ export class Mutex { #acquirers: Array<() => void> = []; // This is FIFO. - async acquire( - onRelease?: () => void, - ): Promise> { + async acquire(): Promise> { if (!this.#locked) { this.#locked = true; return new Mutex.Guard(this); @@ -32,7 +27,7 @@ export class Mutex { const {resolve, promise} = Promise.withResolvers(); this.#acquirers.push(resolve); await promise; - return new Mutex.Guard(this, onRelease); + return new Mutex.Guard(this); } release(): void { @@ -44,3 +39,4 @@ export class Mutex { resolve(); } } + From 51a1761b5fabc9153e86bd0ee3489a42e6f5f344 Mon Sep 17 00:00:00 2001 From: mithun50 Date: Thu, 2 Oct 2025 16:58:30 +0530 Subject: [PATCH 2/6] Fix Mutex formatting and update docs --- src/Mutex.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Mutex.ts b/src/Mutex.ts index c5de4bca2..b66e0cd26 100644 --- a/src/Mutex.ts +++ b/src/Mutex.ts @@ -39,4 +39,3 @@ export class Mutex { resolve(); } } - From 57df66f69ce82c2e294407188dbd0cd8f7fc9da2 Mon Sep 17 00:00:00 2001 From: mithun50 Date: Thu, 9 Oct 2025 15:47:34 +0530 Subject: [PATCH 3/6] Improve navigate_page_history error messages Check navigation result and provide specific feedback: - Detect when no previous/next page exists in history - Show timeout errors with duration details - Include actual error messages instead of generic text --- src/tools/pages.ts | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/src/tools/pages.ts b/src/tools/pages.ts index 65d2b093b..9e348464a 100644 --- a/src/tools/pages.ts +++ b/src/tools/pages.ts @@ -142,16 +142,33 @@ 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) + 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.`, - ); } response.setIncludePages(true); From b102fee77f3e298f6647a186e794c7d6047137df Mon Sep 17 00:00:00 2001 From: mithun50 Date: Thu, 9 Oct 2025 15:55:56 +0530 Subject: [PATCH 4/6] Format navigate_page_history code --- src/tools/pages.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tools/pages.ts b/src/tools/pages.ts index 9e348464a..a86f04297 100644 --- a/src/tools/pages.ts +++ b/src/tools/pages.ts @@ -153,7 +153,8 @@ export const navigatePageHistory = defineTool({ // If result is null, navigation wasn't possible (no history) if (result === null) { - const direction = request.params.navigate === 'back' ? 'previous' : 'next'; + const direction = + request.params.navigate === 'back' ? 'previous' : 'next'; response.appendResponseLine( `Cannot navigate ${request.params.navigate}, no ${direction} page in history.`, ); From 845c8d3dd7b4df9781bd401e4b1e8a5aa8c45423 Mon Sep 17 00:00:00 2001 From: mithun50 Date: Mon, 13 Oct 2025 13:07:17 +0530 Subject: [PATCH 5/6] fix: correct navigate_page_history error handling Per puppeteer/puppeteer#14160, null return from goBack/goForward indicates successful navigation without network request, not failure. Puppeteer throws error when no history exists. Simplified to append error.message as suggested by @OrKoN. --- src/tools/pages.ts | 27 +++++---------------------- tests/tools/pages.test.ts | 14 ++++++++------ 2 files changed, 13 insertions(+), 28 deletions(-) diff --git a/src/tools/pages.ts b/src/tools/pages.ts index a86f04297..7643881d0 100644 --- a/src/tools/pages.ts +++ b/src/tools/pages.ts @@ -144,32 +144,15 @@ export const navigatePageHistory = defineTool({ }; try { - let result; if (request.params.navigate === 'back') { - result = await page.goBack(options); + await page.goBack(options); } else { - result = await page.goForward(options); - } - - // If result is null, navigation wasn't possible (no history) - if (result === null) { - const direction = - request.params.navigate === 'back' ? 'previous' : 'next'; - response.appendResponseLine( - `Cannot navigate ${request.params.navigate}, no ${direction} page in history.`, - ); + await page.goForward(options); } } 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 { - response.appendResponseLine( - `Unable to navigate ${request.params.navigate}: ${error.message || 'Unknown error occurred'}`, - ); - } + response.appendResponseLine( + `Unable to navigate ${request.params.navigate} in currently selected page. ${error.message}`, + ); } response.setIncludePages(true); diff --git a/tests/tools/pages.test.ts b/tests/tools/pages.test.ts index 38a23ad8b..9d4b847f8 100644 --- a/tests/tools/pages.test.ts +++ b/tests/tools/pages.test.ts @@ -163,9 +163,10 @@ describe('pages', () => { context, ); - assert.equal( - response.responseLines.at(0), - 'Unable to navigate forward in currently selected page.', + assert.ok( + response.responseLines + .at(0) + ?.startsWith('Unable to navigate forward in currently selected page.'), ); assert.ok(response.includePages); }); @@ -178,9 +179,10 @@ describe('pages', () => { context, ); - assert.equal( - response.responseLines.at(0), - 'Unable to navigate back in currently selected page.', + assert.ok( + response.responseLines + .at(0) + ?.startsWith('Unable to navigate back in currently selected page.'), ); assert.ok(response.includePages); }); From 280a895071aac6a7e70731e62d1d4d60af01964a Mon Sep 17 00:00:00 2001 From: mithun50 Date: Mon, 13 Oct 2025 13:10:27 +0530 Subject: [PATCH 6/6] chore: fix formatting --- tests/tools/pages.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/tools/pages.test.ts b/tests/tools/pages.test.ts index 9d4b847f8..43dfd918e 100644 --- a/tests/tools/pages.test.ts +++ b/tests/tools/pages.test.ts @@ -166,7 +166,9 @@ describe('pages', () => { assert.ok( response.responseLines .at(0) - ?.startsWith('Unable to navigate forward in currently selected page.'), + ?.startsWith( + 'Unable to navigate forward in currently selected page.', + ), ); assert.ok(response.includePages); });