Skip to content

[miniflare] Force-close connections in InspectorProxyController dispose#13523

Closed
petebacondarwin wants to merge 1 commit intomainfrom
fix/inspector-proxy-controller-close-connections
Closed

[miniflare] Force-close connections in InspectorProxyController dispose#13523
petebacondarwin wants to merge 1 commit intomainfrom
fix/inspector-proxy-controller-close-connections

Conversation

@petebacondarwin
Copy link
Copy Markdown
Contributor

@petebacondarwin petebacondarwin commented Apr 15, 2026

InspectorProxyController.dispose() was calling server.close() without first calling server.closeAllConnections(). Active HTTP keep-alive or WebSocket connections would prevent the close callback from firing, hanging the dispose indefinitely and contributing to wrangler dev not exiting on Ctrl-C.

The same file's #closeServer() method (used during port changes via setOptions()) already uses closeAllConnections() — this brings dispose() in line with that pattern.


  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows:
    • Additional testing not necessary because: consistent with existing #closeServer() pattern in the same file; 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
Copy link
Copy Markdown

changeset-bot Bot commented Apr 15, 2026

🦋 Changeset detected

Latest commit: 0bd94a8

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 edmundhung and removed request for a team April 15, 2026 15:32
@workers-devprod
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-controller.ts: [@cloudflare/wrangler]

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Apr 15, 2026

The change is straightforward and correct. The dispose() method at line 323 was missing a server.closeAllConnections() call before server.close(), which is exactly what the existing #closeServer() method at line 96 already does. The fix adds the missing call, making dispose() consistent with #closeServer(). The changeset is properly formatted as a patch for miniflare.

LGTM

github run

@github-actions
Copy link
Copy Markdown
Contributor

✅ All changesets look good

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

Open in Devin Review

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Apr 15, 2026

create-cloudflare

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

@cloudflare/kv-asset-handler

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

miniflare

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

@cloudflare/pages-shared

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

@cloudflare/unenv-preset

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

@cloudflare/vite-plugin

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

@cloudflare/vitest-pool-workers

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

@cloudflare/workers-editor-shared

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

wrangler

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

commit: 0bd94a8

petebacondarwin added a commit that referenced this pull request Apr 16, 2026
…ispose

Fold in fix from PR #13523: call server.closeAllConnections() before
server.close() so active HTTP keep-alive or WebSocket connections
don't prevent the close callback from firing.
@petebacondarwin
Copy link
Copy Markdown
Contributor Author

Folded into #13515 which now includes this fix (server.closeAllConnections() before server.close() in InspectorProxyController.dispose()).

@github-project-automation github-project-automation Bot moved this from Untriaged to Done in workers-sdk Apr 16, 2026
petebacondarwin added a commit that referenced this pull request Apr 16, 2026
…ispose

Fold in fix from PR #13523: call server.closeAllConnections() before
server.close() so active HTTP keep-alive or WebSocket connections
don't prevent the close callback from firing.
petebacondarwin added a commit that referenced this pull request Apr 16, 2026
…ispose

Fold in fix from PR #13523: call server.closeAllConnections() before
server.close() so active HTTP keep-alive or WebSocket connections
don't prevent the close callback from firing.
petebacondarwin added a commit that referenced this pull request Apr 17, 2026
…ispose

Fold in fix from PR #13523: call server.closeAllConnections() before
server.close() so active HTTP keep-alive or WebSocket connections
don't prevent the close callback from firing.
petebacondarwin added a commit that referenced this pull request Apr 17, 2026
…ispose

Fold in fix from PR #13523: call server.closeAllConnections() before
server.close() so active HTTP keep-alive or WebSocket connections
don't prevent the close callback from firing.
petebacondarwin added a commit that referenced this pull request Apr 17, 2026
…ispose

Fold in fix from PR #13523: call server.closeAllConnections() before
server.close() so active HTTP keep-alive or WebSocket connections
don't prevent the close callback from firing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants