Skip to content

test: cover synchronous connector throws - #76

Merged
ronag merged 2 commits into
nxtedition:masterfrom
ronag:codex/test-connector-throw-20260830
Aug 30, 2026
Merged

test: cover synchronous connector throws#76
ronag merged 2 commits into
nxtedition:masterfrom
ronag:codex/test-connector-throw-20260830

Conversation

@ronag

@ronag ronag commented Aug 30, 2026

Copy link
Copy Markdown
Member

Ports the regressions from nodejs/undici PR nodejs#4834.

The lower fork already handles a synchronously throwing connector through its Promise-based connect path. These tests verify that repeated requests settle instead of hanging and that the client recovers once the connector stops throwing.

Validation so far:

  • focused regression: 2/2 passing
  • ESLint and git diff check passing

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds Node.js-style regression coverage ensuring Client requests settle (don’t hang) when a custom connect implementation throws synchronously, and that the client can recover once the connector stops throwing.

Changes:

  • Introduces a new node-test covering repeated request settlement when connect throws synchronously.
  • Adds a recovery regression asserting subsequent requests succeed once connect stops throwing.
Suppressed comments (4)

test/node-test/client-connector-throw.js:17

  • Use t.after(...) for server cleanup to match the established node-test pattern in this repo and to ensure the hook is registered on the current test instance.
  after(closeServerAsPromise(server))

test/node-test/client-connector-throw.js:25

  • Use t.after(...) for client cleanup to match the established node-test pattern in this repo and to ensure the hook is registered on the current test instance.
    after(() => client.destroy())

test/node-test/client-connector-throw.js:48

  • Use t.after(...) for server cleanup to match the established node-test pattern in this repo and to ensure the hook is registered on the current test instance.
  after(closeServerAsPromise(server))

test/node-test/client-connector-throw.js:64

  • Use t.after(...) for client cleanup to match the established node-test pattern in this repo and to ensure the hook is registered on the current test instance.
    after(() => client.destroy())

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread test/node-test/client-connector-throw.js Outdated
Comment thread test/node-test/client-connector-throw.js

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@ronag
ronag marked this pull request as ready for review August 30, 2026 06:46
@ronag
ronag merged commit 2355f63 into nxtedition:master Aug 30, 2026
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.

2 participants