Skip to content

fix: node crash after not handled aborted connections#8613

Open
Varixo wants to merge 1 commit intobuild/v2from
v2-fix-node-crash
Open

fix: node crash after not handled aborted connections#8613
Varixo wants to merge 1 commit intobuild/v2from
v2-fix-node-crash

Conversation

@Varixo
Copy link
Copy Markdown
Member

@Varixo Varixo commented May 6, 2026

There were three problems:

  • EPIPE / ECONNRESET were not treated as normal client disconnects.
    The old code only handled messages like write after end and The stream has been destroyed.
  • ServerResponse / request socket error events were not listened to.
    In Node, an emitted error event without a listener can crash the process.
  • The Node adapter dropped the getWritableStream(..., resolve) callback.
    Other adapters resolve the response promise when the response stream is created. Node was not doing that, so handled.response could hang.

@Varixo Varixo self-assigned this May 6, 2026
@Varixo Varixo requested a review from a team as a code owner May 6, 2026 17:34
@Varixo Varixo added the V2 label May 6, 2026
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 6, 2026

🦋 Changeset detected

Latest commit: 77a86e2

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

@maiieul maiieul moved this from Backlog to Waiting For Review in Qwik Development May 6, 2026
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 6, 2026

Open in StackBlitz

@qwik.dev/core

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/core@8613

@qwik.dev/router

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/router@8613

eslint-plugin-qwik

npm i https://pkg.pr.new/QwikDev/qwik/eslint-plugin-qwik@8613

create-qwik

npm i https://pkg.pr.new/QwikDev/qwik/create-qwik@8613

@qwik.dev/optimizer

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/optimizer@8613

commit: 77a86e2

@Varixo Varixo force-pushed the v2-fix-node-crash branch from 555b10e to 77a86e2 Compare May 6, 2026 17:40
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
qwik-docs ✅ Ready (View Log) Visit Preview 77a86e2

Copy link
Copy Markdown
Member

@wmertens wmertens left a comment

Choose a reason for hiding this comment

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

LGTM

Some nitpicks, see if you want to change those or not, LGTM

});
});
let responseError: unknown;
let resolveClosed = () => {};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you can just do let resolveClosed: () => void, it will always be initialized

});
let responseError: unknown;
let resolveClosed = () => {};
const closedPromise = new Promise<void>((resolve) => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you can make this = closed ? Promise.resolve() : ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Waiting For Review

Development

Successfully merging this pull request may close these issues.

3 participants