Skip to content

[miniflare] Close runtime WebSocket in InspectorProxy dispose#13522

Closed
petebacondarwin wants to merge 2 commits into
mainfrom
fix/inspector-proxy-close-runtime-ws
Closed

[miniflare] Close runtime WebSocket in InspectorProxy dispose#13522
petebacondarwin wants to merge 2 commits into
mainfrom
fix/inspector-proxy-close-runtime-ws

Conversation

@petebacondarwin

@petebacondarwin petebacondarwin commented Apr 15, 2026 •
edited by devin-ai-integration Bot
Loading

Copy link
Copy Markdown
Contributor

InspectorProxy.dispose() was closing the devtools WebSocket connection but not the runtime WebSocket connection to workerd's inspector server. The open runtime WebSocket (and its 10-second keep-alive interval, which was correctly cleared) kept the socket alive, preventing the Node.js event loop from draining and contributing to wrangler dev hanging on Ctrl-C.

  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows:
    • Additional testing not necessary because: one-line addition; existing inspector proxy tests confirm no regression
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: internal implementation fix with no user-facing API change
Open with Devin

@changeset-bot

changeset-bot Bot commented Apr 15, 2026 •
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 763b66c

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-project-automation github-project-automation Bot moved this to Untriaged in workers-sdk Apr 15, 2026
@workers-devprod workers-devprod requested review from a team and vicb and removed request for a team April 15, 2026 15:31
@workers-devprod

workers-devprod commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • @cloudflare/wrangler
Show detailed file reviewers
  • packages/miniflare/src/plugins/core/inspector-proxy/inspector-proxy.ts: [@cloudflare/wrangler]

@ask-bonk

ask-bonk Bot commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

The change is straightforward and correct:

  • #runtimeWs is typed as WebSocket (not optional), but ?. is used defensively which is fine.
  • The keep-alive interval was already being cleared, but the underlying WebSocket was not being closed — this was a clear omission.
  • close() on a ws WebSocket is safe to call regardless of its current state.
  • The changeset is properly formatted as a patch for miniflare.
  • No regressions, no API changes, no security concerns.

LGTM

github run

devin-ai-integration[bot]

This comment was marked as resolved.

Sign in to view

@pkg-pr-new

pkg-pr-new Bot commented Apr 15, 2026 •
edited
Loading

Copy link
Copy Markdown
create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@13522

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@13522

miniflare

npm i https://pkg.pr.new/miniflare@13522

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@13522

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@13522

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@13522

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@13522

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@13522

wrangler

npm i https://pkg.pr.new/wrangler@13522

commit: 763b66c

@github-actions

github-actions Bot commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

✅ All changesets look good

Summary of changesets reviewed:

  1. c3-frameworks-update-12714.md (create-cloudflare, patch) - Dependency update for create-cloudflare (exempt from validation per rule wrangler tail #5)

  2. nasty-years-taste.md (wrangler, patch) - Improve validation and error messaging for R2 CORS configuration files to catch AWS S3-style formatting mistake

  3. fix-ipv6-localhost-spin.md (miniflare, patch) - Fix internal loopback communication to use 127.0.0.1 when localhost is configured, preventing IPv6/IPv4 mismatch issues

  4. fix-inspector-proxy-close-runtime-ws.md (miniflare, patch) - Close runtime WebSocket in InspectorProxy dispose to fix wrangler dev hanging on Ctrl-C

All changesets:

  • Use appropriate version types (patch for bug fixes/improvements)
  • Have meaningful, descriptive changelog entries
  • Contain no h1/h2/h3 headers

@petebacondarwin

petebacondarwin commented Apr 16, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #13515 which includes this fix (closing #runtimeWs in InspectorProxy.dispose()).

@github-project-automation github-project-automation Bot moved this from Untriaged to Done in workers-sdk Apr 16, 2026
@petebacondarwin petebacondarwin mentioned this pull request Apr 16, 2026
Merged
3 tasks
@com6056 com6056 mentioned this pull request Jun 5, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@devin-ai-integration devin-ai-integration[bot] devin-ai-integration[bot] left review comments

@workers-devprod workers-devprod Awaiting requested review from workers-devprod workers-devprod is a code owner

@vicb vicb Awaiting requested review from vicb vicb was automatically assigned from cloudflare/wrangler

Assignees

No one assigned

Labels

None yet

Projects

workers-sdk
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@petebacondarwin @workers-devprod