Skip to content

fix(miniflare): Skip creating hyperdrive proxy server for local database#13390

Merged
petebacondarwin merged 2 commits into
cloudflare:mainfrom
Ltadrian:agracia/SQC-812-troubleshoot-windows
Apr 29, 2026
Merged

fix(miniflare): Skip creating hyperdrive proxy server for local database#13390
petebacondarwin merged 2 commits into
cloudflare:mainfrom
Ltadrian:agracia/SQC-812-troubleshoot-windows

Conversation

@Ltadrian

@Ltadrian Ltadrian commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

Skip creating a local TCP proxy server for Hyperdrive bindings when SSL is not needed (sslmode disabled or not specified), connecting directly to the database instead. This avoids connection refused errors caused by firewall rules or proxy port binding issues on Windows/macOS.

Also fixes two bugs in HyperdriveProxyController:

  • Add missing error handler on the proxy net.Server to prevent silent crashes
  • Fix missing return in dispose() that caused servers not to be awaited on cleanup

Fixes: #11556


  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows:

On Windows 10 Machine

  1. pnpm run build --filter wrangler
  2. Run custom build of wrangler in worker with Hyperdrive pointing at local postgres
  3. node ..\workers-sdk\packages\wrangler\wrangler-dist\cli.js dev
  4. Confirm that connecting to localhost works as expected
  • Additional testing not necessary because:
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: This is expected behavior, and attempting to fix customers that run into this issue.

A picture of a cute animal (not mandatory, but encouraged)

@changeset-bot

changeset-bot Bot commented Apr 9, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: b29b302

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

@pkg-pr-new

pkg-pr-new Bot commented Apr 9, 2026

Copy link
Copy Markdown
create-cloudflare

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

@cloudflare/kv-asset-handler

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

miniflare

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

@cloudflare/pages-shared

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

@cloudflare/unenv-preset

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

@cloudflare/vite-plugin

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

@cloudflare/vitest-pool-workers

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

@cloudflare/workers-editor-shared

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

@cloudflare/workers-utils

npm i https://pkg.pr.new/@cloudflare/workers-utils@13390

wrangler

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

commit: b29b302

@Ltadrian Ltadrian changed the title fix(miniflare): skip hyperdrive proxy server when SSL is disabled and… fix(miniflare): Skip creating hyperdrive proxy server for local database Apr 17, 2026
@Ltadrian Ltadrian force-pushed the agracia/SQC-812-troubleshoot-windows branch 3 times, most recently from ffa97e3 to c73dad3 Compare April 19, 2026 17:17
@Ltadrian Ltadrian marked this pull request as ready for review April 19, 2026 17:18
@workers-devprod workers-devprod requested review from a team and petebacondarwin and removed request for a team April 19, 2026 17:18
@workers-devprod

Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • @cloudflare/wrangler
Show detailed file reviewers
  • packages/miniflare/src/index.ts: [@cloudflare/wrangler]
  • packages/miniflare/src/plugins/hyperdrive/hyperdrive-proxy.ts: [@cloudflare/wrangler]
  • packages/miniflare/src/plugins/hyperdrive/index.ts: [@cloudflare/wrangler]
  • packages/miniflare/test/plugins/hyperdrive/index.spec.ts: [@cloudflare/wrangler]

devin-ai-integration[bot]

This comment was marked as resolved.

@Ltadrian Ltadrian force-pushed the agracia/SQC-812-troubleshoot-windows branch 3 times, most recently from d3a206c to f10a220 Compare April 19, 2026 21:32
devin-ai-integration[bot]

This comment was marked as resolved.

@petebacondarwin petebacondarwin force-pushed the agracia/SQC-812-troubleshoot-windows branch from f10a220 to f7a5c21 Compare April 20, 2026 11:51
devin-ai-integration[bot]

This comment was marked as resolved.

@petebacondarwin petebacondarwin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tidy PR. Thanks!
Please can you update the changeset and address the devin comment about re-assigning the log object during setOptions.

Comment thread .changeset/quiet-houses-attack.md Outdated
Comment thread packages/miniflare/test/plugins/hyperdrive/index.spec.ts Outdated
@github-project-automation github-project-automation Bot moved this from Untriaged to In Review in workers-sdk Apr 20, 2026
@Ltadrian Ltadrian force-pushed the agracia/SQC-812-troubleshoot-windows branch 2 times, most recently from e321cef to 2b26e2d Compare April 22, 2026 15:14
@Ltadrian

Copy link
Copy Markdown
Contributor Author

Tidy PR. Thanks! Please can you update the changeset and address the devin comment about re-assigning the log object during setOptions.

@petebacondarwin thank you! I have updated these, and the tests failing seem unrelated to my current PR

@workers-devprod

workers-devprod commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • ✅ @cloudflare/wrangler
Show detailed file reviewers

@Ltadrian Ltadrian force-pushed the agracia/SQC-812-troubleshoot-windows branch 3 times, most recently from 59c3dfd to 395563f Compare April 27, 2026 16:59
@Ltadrian Ltadrian force-pushed the agracia/SQC-812-troubleshoot-windows branch from 395563f to fe25fa5 Compare April 27, 2026 17:11
devin-ai-integration[bot]

This comment was marked as resolved.

@Ltadrian Ltadrian force-pushed the agracia/SQC-812-troubleshoot-windows branch from fe25fa5 to 7d7392a Compare April 27, 2026 17:24
devin-ai-integration[bot]

This comment was marked as resolved.

@Ltadrian Ltadrian force-pushed the agracia/SQC-812-troubleshoot-windows branch 3 times, most recently from 549446b to d116ea5 Compare April 27, 2026 19:33
fix proxy error handling

Skip creating a local TCP proxy server for Hyperdrive bindings when SSL
is
not needed (sslmode disabled or not specified), connecting directly to
the
database instead. This avoids connection refused errors caused by
firewall
rules or proxy port binding issues on Windows/macOS.

Also fixes two bugs in HyperdriveProxyController:
- Add missing error handler on the proxy net.Server to prevent silent
  crashes
- Fix missing return in dispose() that caused servers not to be awaited
  on cleanup
@petebacondarwin petebacondarwin force-pushed the agracia/SQC-812-troubleshoot-windows branch from d116ea5 to 1ef0256 Compare April 29, 2026 10:58

@workers-devprod workers-devprod left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Codeowners reviews satisfied

@github-project-automation github-project-automation Bot moved this from In Review to Approved in workers-sdk Apr 29, 2026
@petebacondarwin

Copy link
Copy Markdown
Contributor

Investigating the CI failures...

When wrangler dev is force-killed via tree-kill at e2e teardown, workerd's
outbound TCP connection to the test mock database server is reset rather
than gracefully closed. With the Hyperdrive proxy now skipped for
sslmode=disable (the default), that RST surfaces directly on the
per-connection socket of the test's nodeNet.Server. Without an error
listener Node escalates it to uncaughtException, which Vitest catches as
an unhandled error and fails the run even though the test itself passed.

Add a small ECONNRESET-only-tolerant error handler to the affected mock
sockets in dev.test.ts and get-platform-proxy.test.ts. This is the same
pattern already used at dev.test.ts:947, just generalised and applied
where it was missing.
@petebacondarwin

Copy link
Copy Markdown
Contributor

Hi @Ltadrian — pushed a follow-up commit (b29b302) to address the failing Wrangler E2E jobs (Linux shard 1/4, macOS shard 1/4, Windows shard 1/4, Windows shard 4/4).

What was failing

In every failing job the affected test itself passed (all assertions green), but Vitest then caught two unhandled ECONNRESET exceptions and failed the run.

Job Test
Linux/macOS/Windows shard 1 e2e/dev.test.ts:819hyperdrive dev tests > connects to a socket
Windows shard 4 e2e/get-platform-proxy.test.ts:472Hyperdrive > can connect to a TCP socket via the hyperdrive connect method

Why this PR triggered it

Before this PR: worker → miniflare hyperdrive proxy (net.Server) → test mock server. When wrangler dev was force-killed via treeKill (WranglerLongLivedCommand.stop()), the proxy's clientSocket.pipe(dbSocket) semantics caused dbSocket.end() (a graceful FIN) to be sent to the test mock. So the mock saw a clean close.

After this PR (when sslmode=disable, the default): worker → test mock server directly. treeKill of wrangler tears down workerd's outbound TCP connection with RST instead of FIN. The mock sockets had no error listener, so Node escalated to uncaughtException and Vitest reported it.

The proxy was effectively absorbing the abrupt-close edge case for these tests.

The fix

Attach an ECONNRESET-tolerant socket.on('error', …) listener to the mock server sockets in the two affected files. This mirrors the pattern already in use at dev.test.ts:947 for the immediately-following sibling test — it just wasn't applied to the spots that started failing under the new direct-connect path.

function ignoreEconnreset(err: NodeJS.ErrnoException): void {
  if (err.code !== 'ECONNRESET') {
    throw err;
  }
}

No production code changes — the miniflare behaviour change in this PR is intentional and good. The unit tests in LocalRuntimeController.test.ts were unaffected because they tear down via controller.teardown() (graceful) rather than treeKill.

Verification

Ran both affected tests locally (after rebuilding miniflare + wrangler from this branch) and they pass cleanly with no Unhandled Errors:

  • e2e/dev.test.ts -t \"connects to a socket\" — 2 passed (postgresql + mysql)
  • e2e/get-platform-proxy.test.ts -t \"can connect to a TCP socket via the hyperdrive connect method\" — 3 passed

Format / lint / type-check all clean. No changeset added — the change is test-only and the existing quiet-houses-attack.md covers the production change.

@petebacondarwin petebacondarwin merged commit 0bf64a7 into cloudflare:main Apr 29, 2026
51 checks passed
@github-project-automation github-project-automation Bot moved this from Approved to Done in workers-sdk Apr 29, 2026
ruifigueira pushed a commit that referenced this pull request Apr 29, 2026
…ase (#13390)

Co-authored-by: Adrian Gracia <agracia@cloudflare.com>
Co-authored-by: Pete Bacon Darwin <pbacondarwin@cloudflare.com>
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.

Issue with Windows Hyperdrive configs locally

3 participants