Skip to content

Force-close lingering connections in request test teardown#314657

Merged
chrmarti merged 1 commit into
mainfrom
chrmarti/fix-request-test-teardown
May 6, 2026
Merged

Force-close lingering connections in request test teardown#314657
chrmarti merged 1 commit into
mainfrom
chrmarti/fix-request-test-teardown

Conversation

@chrmarti
Copy link
Copy Markdown
Collaborator

@chrmarti chrmarti commented May 6, 2026

Summary

Fixes a flaky Timeout of 30000ms exceeded failure in the "after each" hook for "cancel" of vs/base/parts/request/test/electron-main/request.test, observed on Linux CI.

The cancel and timeout tests open a request to a /noreply endpoint that the test server intentionally never responds to. After the client aborts/times out, the server-side socket can remain open when teardown runs, causing server.close() to wait until all connections drain — which never happens, and the hook hits the 30s mocha timeout.

Calling server.closeAllConnections() (Node 18.2+) before server.close() destroys those lingering sockets so close() resolves promptly.

Session Context
  • Diagnosis: The failure is in the after each hook for cancel, not the test body. server.close() keeps existing connections alive by design, so any socket from /noreply that hasn't fully torn down on the server side blocks teardown until the mocha timeout.
  • Fix scope: Limited to the test teardown. No production code changes; the request implementation is unaffected.
  • Why closeAllConnections(): Available since Node 18.2 (well within VS Code's supported range) and is the documented way to force in-flight server-side sockets closed.

Copilot AI review requested due to automatic review settings May 6, 2026 07:29
@chrmarti chrmarti marked this pull request as ready for review May 6, 2026 07:29
@chrmarti chrmarti enabled auto-merge (rebase) May 6, 2026 07:29
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Addresses a Linux CI flake in the Request electron-main tests where the /noreply endpoint can leave server-side sockets open, causing server.close() in teardown to hang until the Mocha hook timeout.

Changes:

  • Force-close any lingering HTTP connections in test teardown via server.closeAllConnections() before awaiting server.close().
Show a summary per file
File Description
src/vs/base/parts/request/test/electron-main/request.test.ts Ensures the test server teardown can’t hang on lingering sockets created by /noreply requests.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 0

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

Screenshot Changes

Base: 6fcc6cfe Current: 1f752500

Changed (12)

chat/input/chatInput/Default/Light
Before After
before after
chat/input/chatInput/WithArtifacts/Dark
Before After
before after
chat/input/chatInput/WithArtifacts/Light
Before After
before after
chat/input/chatInput/WithFileChanges/Dark
Before After
before after
chat/input/chatInput/WithFileChanges/Light
Before After
before after
chat/input/chatInput/WithTodos/Dark
Before After
before after
chat/input/chatInput/WithTodos/Light
Before After
before after
chat/input/chatInput/WithTodosAndFileChanges/Light
Before After
before after
chat/input/chatInput/WithArtifactsAndFileChanges/Dark
Before After
before after
chat/input/chatInput/WithArtifactsAndFileChanges/Light
Before After
before after
chat/input/chatInput/Full/Light
Before After
before after
chat/widget/chatWidget/PendingToolApproval/Light
Before After
before after

@chrmarti chrmarti merged commit d566389 into main May 6, 2026
30 checks passed
@chrmarti chrmarti deleted the chrmarti/fix-request-test-teardown branch May 6, 2026 07:46
@vs-code-engineering vs-code-engineering Bot added this to the 1.120.0 milestone May 6, 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.

3 participants