Visitar URL original
fix: close the tab opened by a failed new_page by serhiizghama · Pull Request #2928 · ChromeDevTools/chrome-devtools-mcp · GitHub
Skip to content

fix: close the tab opened by a failed new_page - #2928

Merged
OrKoN merged 3 commits into
ChromeDevTools:mainfrom
serhiizghama:fix/new-page-failed-navigation
Oct 20, 2026
Merged

OrKoN merged 3 commits into
ChromeDevTools:mainfrom
serhiizghama:fix/new-page-failed-navigation

Conversation

@serhiizghama

Copy link
Copy Markdown
Contributor

Fixes #2925.

new_page creates and selects the tab before it navigates, so when goto throws (blocked by the allow/blocklist, connection error, timeout) the tool returns only the error while the new tab stays open and selected. With --no-page-id-routing the next page-scoped call then runs on that leftover tab.

I went with the first option from the issue: on a failed navigation the handler closes the tab it just opened and puts the previous selection back, then rethrows the original error. The one exception is a load blocked by a dialog (alert() during parse). On main that tab stays open with the dialog pending, so the agent can still answer it with handle_dialog, and I kept it that way.

To get page.id in the handler, Context.newPage now returns McpPage, which is what McpContext.newPage already returns. Tests cover a failed navigation (port 1, refused by Chrome, so it fails offline) and the dialog case.

If you'd rather keep the tab and report its pageId in the error instead, I can switch to that.

@yulunz yulunz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for picking this up! Closing the tab and restoring the previous selection on a failed new_page navigation (while keeping the tab when page.getDialog() is present) makes sense, and narrowing Context.newPage to return Promise<McpPage> is clean since McpContext.newPage already returns it.

Could you make two small adjustments before we land this:

  1. Make the cleanup best-effort and guard previousPage: If context.closePage(page.id) throws inside catch (for example, CLOSE_PAGE_ERROR if the previous tab was closed while goto was running, No page found if the new target already went away, or a protocol error on disconnect), that error will replace the original navigation error and skip context.selectPage(previousPage). Also, previousPage could have been closed while goto was running. Please:
    • restore the selection first with if (previousPage && !previousPage.isClosed()) { context.selectPage(previousPage); }, and
    • wrap both the re-selection and await context.closePage(page.id) in their own try/catch so the original navigation error is always rethrown.
  2. Include the page list when keeping a dialog-blocked tab: When page.getDialog() is set and the new tab is kept open, new_page throws before response.setIncludePages(true), so the caller doesn't learn the new tab's pageId (needed to call handle_dialog). Calling response.setIncludePages(true) when page.getDialog() is present before rethrowing will include ## Pages in the error response.

@chiragjangiir

Copy link
Copy Markdown

I closed my duplicate (#3019) so this lands in one place, and I'm not going to touch your branch — this is just an offer, take it or leave it.

One case the current shape still leaves unreported: if context.closePage(page.id) throws inside the catch (the CLOSE_PAGE_ERROR / No page found / disconnect cases yulunz listed), the tab does stay open, but the rethrown error is only the navigation error, so the caller never learns that a tab survived and which pageId it has. That's the defect #2925 describes, one level down. Guarding previousPage and the try/catch fixes the selection, but the surviving tab still goes unnamed.

My version handled that by catching the close failure separately and raising an error that names the remaining page, states the selection that was kept, and includes the page list — while isError stays true and the original navigation text is preserved. Roughly:

if (previousPage && !previousPage.isClosed()) {
  context.selectPage(previousPage);
}
try {
  await context.closePage(page.id);
} catch (closeError) {
  // the navigation error is what the caller must see; name what survived
  response.setIncludePages(true);
  throw new Error(
    `Failed to close the page opened by new_page: ${closeError.message}. ` +
      `The tab this call opened is still open as page ${page.id}, ` +
      `and ${previousPage?.id} is selected again.`,
  );
}
throw error;

Happy to send that as a small follow-up PR on top of yours once this lands, or just paste the patch here — your call, and no objection if you'd rather keep it as it is. Thanks for picking this up.

@serhiizghama

Copy link
Copy Markdown
Contributor Author

Done, both points. The selection is restored first (only if previousPage is still open), and the re-select and closePage each sit in their own try/catch, so the navigation error is always the one rethrown. A dialog-blocked tab now sets setIncludePages(true) before rethrowing.

@chiragjangiir I folded in the part about a tab that fails to close: that catch also sets setIncludePages(true), so the surviving tab shows up in ## Pages with its id while the error text stays the original navigation error. Kept it at that rather than a new error message, but happy to go further if the maintainers prefer it spelled out.

Merged main too, and added tests for a failing closePage and for the page list on the dialog case.

@OrKoN
OrKoN requested a review from yulunz October 14, 2026 08:17
@OrKoN

OrKoN commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator

@yulunz, friendly reminder: this PR has new commits since your last review and has been waiting for 3 day(s) (automated reminder)

@yulunz yulunz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for addressing all the review feedback! The new_page cleanup and setIncludePages(true) handling look good to me.

One quick note before we merge: #3182 recently landed on main and migrated the surrounding new_page tests in tests/tools/pages.test.ts from withMcpContext to createHandlerMocks(), which created a merge conflict in tests/tools/pages.test.ts. Could you rebase onto main (and optionally switch the 3 new new_page tests to createHandlerMocks() and sinon.assert to match AGENTS.md)?

@serhiizghama

Copy link
Copy Markdown
Contributor Author

Rebased onto main and squashed it down to two commits, the fix itself is unchanged. Also moved the three new tests to createHandlerMocks() with sinon.assert, so they don't need a browser anymore. Checked that all three fail with the fix reverted.

@serhiizghama
serhiizghama force-pushed the fix/new-page-failed-navigation branch from b4ea902 to 4216083 Compare October 17, 2026 11:28
Comment thread src/tools/ToolDefinition.ts Resolved
@serhiizghama

Copy link
Copy Markdown
Contributor Author

Makes sense. newPage is back to returning ContextPage, and I added id and isClosed() to the ContextPage interface instead (McpPage already implements both), so pages.ts no longer imports McpPage. Happy to go another way if you'd rather not grow the interface for this.

@OrKoN
OrKoN self-requested a review October 20, 2026 06:11
@OrKoN
OrKoN enabled auto-merge October 20, 2026 07:32
@OrKoN
OrKoN disabled auto-merge October 20, 2026 14:33
@OrKoN
OrKoN added this pull request to the merge queue Oct 20, 2026
Merged via the queue into ChromeDevTools:main with commit c174677 Oct 20, 2026
14 checks passed
github-merge-queue Bot pushed a commit that referenced this pull request Oct 22, 2026
🤖 I have created a release *beep* *boop*
---


##
[1.12.0](chrome-devtools-mcp-v1.11.0...chrome-devtools-mcp-v1.12.0)
(2026-10-22)


### 🎉 Features

* add --maxNavigationSaved flag to configure retained navigation history
([#3018](#3018))
([a580d13](a580d13))
* add Kimi Code plugin manifest
([#3029](#3029))
([cd00b64](cd00b64))
* Analyze heap snapshots without a browser connection
([#3156](#3156))
([78c8320](78c8320))
* **config:** add --watchConfig to reload config file changes
([#3248](#3248))
([b48561f](b48561f))
* **config:** introduce layered ServerConfig for live reload
([#3203](#3203))
([37f1fda](37f1fda))
* **config:** share ReadonlyServerConfig with McpContext and McpPage
([#3204](#3204))
([0813bf3](0813bf3))
* **config:** support reloading blockedUrlPattern and allowedUrlPattern
([#3268](#3268))
([cc850f5](cc850f5))
* **context:** release disabled feature state on config reload
([#3049](#3049))
([c0140d1](c0140d1))
* Paginate by default in heap snapshot tools
([#3201](#3201))
([fb8375b](fb8375b))
* Paginate detailed diff in compare_heapsnapshots
([#3195](#3195))
([4a9dd53](4a9dd53))
* run independent tool calls in parallel with a TaskRunner
([#3243](#3243))
([8a8d877](8a8d877))
* run page-scoped tools on different pages in parallel
([#3252](#3252))
([f8f18ac](f8f18ac))
* **server:** add McpServer.applyConfig() to reload configuration
([#3051](#3051))
([f874aeb](f874aeb))
* **slim:** support filepath in slim mode screenshot
([#3296](#3296))
([d399c4a](d399c4a))
* Sort objects for class by self size in compare_heapsnapshots
([#3205](#3205))
([f8a18c6](f8a18c6))
* **tools:** support updating ToolHandler definitions in place
([#3050](#3050))
([8064138](8064138))
* **wait_for:** support opt-in visible text matching
([#3212](#3212))
([eef92da](eef92da))


### 🛠️ Fixes

* allow paths below a filesystem-root workspace root
([#3277](#3277))
([296ccef](296ccef))
* block evaluate_script while a dialog is open
([#3196](#3196))
([545bcbb](545bcbb))
* bound take_snapshot wait and clear the geolocation override
([#3265](#3265))
([6282f54](6282f54))
* **browser:** redact WebSocket headers from connection logs
([#2901](#2901))
([3a2f91e](3a2f91e))
* capture thrown transform errors as Zod validation issues
([#3242](#3242))
([0f95c21](0f95c21))
* clear handled dialog in McpPage.getDialog()
([#3241](#3241))
([2ab93a3](2ab93a3)),
closes
[#2958](#2958)
* **cli:** disallow --slim in CLI start options
([#2987](#2987))
([#3113](#3113))
([ae3f61c](ae3f61c))
* **cli:** do not forward tool args to start
([#3114](#3114))
([fef1598](fef1598))
* **cli:** serialize object daemon arguments
([#2797](#2797))
([f73a440](f73a440))
* close the tab opened by a failed new_page
([#2928](#2928))
([c174677](c174677))
* **config:** accept native JSON objects for wsHeaders in config files
([#3262](#3262))
([371e6c2](371e6c2))
* **console:** describe Map, Set, function, DOM node, symbol and cyclic
arguments
([#3199](#3199))
([968e4e5](968e4e5))
* **console:** include the reason of unhandled promise rejections
([#3148](#3148))
([f7cdf73](f7cdf73))
* defer focus emulation until a page is explicitly targeted
([#3230](#3230))
([35abe35](35abe35))
* disable Puppeteer signal handlers for launched browsers
([#3238](#3238))
([3afec73](3afec73)),
closes
[#3154](#3154)
* discover third-party developer tools before tool lookup
([#3208](#3208))
([2777fa0](2777fa0)),
closes
[#2973](#2973)
* discover third-party developer tools in page tools
([#3207](#3207))
([cf33529](cf33529)),
closes
[#2980](#2980)
* drop InlineTextBox nodes from text snapshots
([#3202](#3202))
([f8c775a](f8c775a))
* **emulate:** keep previous viewport when setViewport fails
([#3150](#3150))
([19d883c](19d883c))
* exclude source maps from the npm package
([#3295](#3295))
([d575da4](d575da4))
* keep surrogate pairs and emoji whole when truncating titles, names and
bodies
([#3215](#3215))
([058adcf](058adcf))
* let resourceTypes match CORS preflights and prefetches
([#3217](#3217))
([e342c6e](e342c6e))
* **lighthouse:** explain unsupported URL schemes in navigation mode
([#3144](#3144))
([c6ad84a](c6ad84a))
* **lighthouse:** fail the audit when Lighthouse reports a runtime error
([#3188](#3188))
([53ea78d](53ea78d))
* **lighthouse:** restore the emulated viewport after a navigation audit
([#3189](#3189))
([c777ece](c777ece))
* **network:** list only earlier hops in a request's redirect chain
([#3198](#3198))
([4675940](4675940))
* **network:** remove unsupported websocket from list_network_requests
filter
([#2961](#2961))
([#3282](#3282))
([8b119d9](8b119d9))
* **network:** reset page network emulation before applying URL
restrictions
([#3312](#3312))
([f25f3df](f25f3df))
* **pages:** reject devtools:// URLs in navigate_page and new_page
([#2935](#2935))
([#3128](#3128))
([c94c03d](c94c03d))
* **pages:** report the actual page size after resize_page
([#2998](#2998))
([#3132](#3132))
([f003056](f003056))
* **pages:** revert reporting the page size after resize_page
([#3308](#3308))
([186ea66](186ea66)),
closes
[#3306](#3306)
* **pagination:** return all items when pageSize is omitted and pageIdx
is 0
([#2996](#2996))
([#3134](#3134))
([3ad9115](3ad9115))
* **performance:** populate structuredContent for traceInsight in
performance_analyze_insight
([#2981](#2981))
([#3115](#3115))
([c2c6cc4](c2c6cc4))
* preserve unread page fallback across snapshots and deduplicate page
disposal
([#3161](#3161))
([8604365](8604365))
* print the no-roots temp-directory warning under default flags
([#2927](#2927))
([0dd8ed9](0dd8ed9))
* reject empty, extra, and unknown parts in geolocation and viewport
transforms
([#3097](#3097))
([#3120](#3120))
([3ea31fb](3ea31fb))
* resolve error causes on the session that reported the exception
([#3220](#3220))
([b2d37e8](b2d37e8))
* resolve service worker IDs for console messages
([#2881](#2881))
([3804a9a](3804a9a))
* return message when page exposes no 3p developer tools
([#3197](#3197))
([663f3c3](663f3c3)),
closes
[#3004](#3004)
* **script:** surface non-Error thrown and rejected values in
evaluate_script
([#2951](#2951))
([#3127](#3127))
([b1e6e68](b1e6e68))
* **security:** validate filesystem paths that parse as http/ws URLs
([#3186](#3186))
([91e112d](91e112d))
* **snapshot:** omit uid for accessibility nodes without backendNodeId
([#3187](#3187))
([9054351](9054351))
* stop active screencast on McpPage.dispose()
([#3160](#3160))
([558af81](558af81))
* stop get_network_request waiting forever for an unread no-store body
([#3216](#3216))
([c556be1](c556be1))
* surface nested error causes and autoConnect hint on connection failure
([#3111](#3111))
([d7f1e78](d7f1e78))
* treat 0.0.0.0 and [::] browser endpoints as local
([#3321](#3321))
([7d5f7de](7d5f7de))
* treat IPv4-mapped IPv6 loopback as localhost
([#3255](#3255))
([6cfb34c](6cfb34c))
* truncate data: URLs in accessibility snapshots
([#3292](#3292))
([11e7d47](11e7d47))
* validate type_text submitKey before typing
([#3185](#3185))
([c8615d7](c8615d7))
* **wait_for:** find text in dynamically attached iframes
([#3210](#3210))
([5f84888](5f84888))
* warn when screenshot dimensions exceed 16384px limit
([#2952](#2952))
([#3291](#3291))
([10455b7](10455b7))


### 📄 Documentation

* add a pre-submit checklist to AGENTS.md
([#3236](#3236))
([e96ee6c](e96ee6c))
* clarify URL pattern port and hostname matching
([#3287](#3287))
([4723335](4723335))
* **config:** note that hot-reloaded URL restrictions keep open targets
attached
([#3314](#3314))
([47db2b8](47db2b8))
* correct CLI availability of extension tools
([#2852](#2852))
([a7280ee](a7280ee))
* explain how to install skills, including the CLI skill
([#3274](#3274))
([46eb546](46eb546))
* remove experimental status from CLI and promote it in README
([#3273](#3273))
([0ced16b](0ced16b))
* require a rebase only when the branch has merge conflicts
([#3239](#3239))
([d1143ac](d1143ac))
* **skills:** clarify Lighthouse score scale and contrast workflow
([#3006](#3006))
([#3218](#3218))
([bf8c6d9](bf8c6d9))


### ⚡ Performance

* **cli:** load a small third-party bundle in the chrome-devtools CLI
([#3200](#3200))
([552e1f8](552e1f8))
* load the Lighthouse bundle on first use
([#3169](#3169))
([798f03a](798f03a))


### 🏗️ Refactor

* **config:** isolate scripts and unit tests from home config discovery
([#3256](#3256))
([65e6b97](65e6b97))
* **config:** lock slim and pageIdRouting options when viaCli is set
([#3257](#3257))
([384d9aa](384d9aa))
* **config:** pass ReadonlyServerConfig to BrowserManager and add
OptionGate
([#3267](#3267))
([98302f2](98302f2))
* **config:** simplify watchConfig and start watcher inside connect
([#3259](#3259))
([878c5f6](878c5f6))
* extract errorResult() and routedPageId() in ToolHandler
([#3251](#3251))
([9a6b03a](9a6b03a))
* import types and values from a module in one statement
([#3303](#3303))
([f4eaf1f](f4eaf1f))
* **lighthouse:** move Lighthouse runner methods to McpPage and add
bundle types
([#3283](#3283))
([e1823a2](e1823a2))
* list optional properties last in inline object types
([#3302](#3302))
([dc929c0](dc929c0))
* register all tools upfront across modes
([#3163](#3163))
([58f8d1f](58f8d1f))
* **response:** pass live ReadonlyServerConfig to McpResponse
([#3258](#3258))
([6042490](6042490))
* **tools:** add explicit factory types to tool definitions
([#3284](#3284))
([4d1d7b2](4d1d7b2))
* use #private names instead of the private modifier
([#3301](#3301))
([2e13311](2e13311))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Failed new_page leaves an unreported tab open and selected; close it or report its pageId

4 participants