Skip to content

fix(http): server write first #816

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mikicho wants to merge 1 commit into
base: main
Choose a base branch
from
+20 −0
Open

fix(http): server write first #816

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions test/modules/http/regressions/http-non-http-socket-passthrough.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,23 @@ it('passes a non-http socket through to the actual server', async () => {

expect(response).toBe('PONG')
})

it.only('server write first', async () => {

@coderabbitai coderabbitai Bot Jul 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="test/modules/http/regressions/http-non-http-socket-passthrough.test.ts"
if [ -f "$file" ]; then
  echo "== file exists =="
  wc -l "$file"
  echo "== relevant section =="
  sed -n '1,90p' "$file" | cat -n
else
  echo "file not found"
  fd -a 'http-non-http-socket-passthrough.test.ts' .
fi

echo "== other it.only/it.se* in regression test files =="
rg -n "\b(it\.only|it\.serial|it\.sequential|it\.concurrent|it\.todo|describe\.only)\b" test/modules/http/regressions || true

Repository: mswjs/interceptors

Length of output: 2411

Remove it.only before merging.

This focuses Vitest on the new test and skips the other regression tests, which can let failures elsewhere go unnoticed.

Proposed fix 
-it.only('server write first', async () => {
+it('server write first', async () => {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it.only('server write first', async () => {
it('server write first', async () => {
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/modules/http/regressions/http-non-http-socket-passthrough.test.ts` at
line 44, Remove the `.only` modifier from the `it` declaration for the “server
write first” test so the full regression test suite runs.

await using server = await createRawTestServer(() => {
return new net.Server((connection) => {
connection.write('PING')
})
})

const response = await new Promise<string>((resolve, reject) => {
const socket = net.connect(server.port, server.hostname)
socket.on('data', (chunk) => {
resolve(chunk.toString())
socket.destroy()
})
socket.on('error', reject)
})

expect(response).toBe('PING')
Comment on lines +51 to +60

@coderabbitai coderabbitai Bot Jul 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate and inspect the referenced test file and nearby code.
if [ -f "test/modules/http/regressions/http-non-http-socket-passthrough.test.ts" ]; then
  echo "== file exists =="
  wc -l test/modules/http/regressions/http-non-http-socket-passthrough.test.ts
  echo
  echo "== relevant test file =="
  cat -n test/modules/http/regressions/http-non-http-socket-passthrough.test.ts
else
  echo "Referenced file not found"
  fd -i 'http-non-http-socket-passthrough.test.ts' .
fi

echo
echo "== related server/passthrough definitions =="
git ls-files | xargs rg -n "non-http|PING|passthrough|http-non-http-socket|connect\\(" test src 2>/dev/null | head -200 || true

Repository: mswjs/interceptors

Length of output: 23818

🌐 Web query:

Node.js net.Socket data event chunk partial receive documentation

💡 Result:

In Node.js, the net.Socket 'data' event does not guarantee the receipt of complete messages [1][2]. Because TCP is a stream-oriented protocol rather than a message-oriented one, there is no inherent concept of message boundaries [2][3]. A single logical message sent by a client can be split into multiple 'data' event chunks, or multiple messages can be merged into a single 'data' event chunk [2][4]. Key characteristics regarding 'data' event chunks include: 1. Arbitrary Chunk Size: The size of the 'data' event chunk is non-deterministic [5]. It depends on factors such as network conditions, kernel buffer availability, and the underlying stream's buffering behavior [5][2]. 2. Streaming Nature: When a net.Socket is in flowing mode, it emits 'data' events as soon as data arrives and is read from the kernel [1][5]. You should not assume that a 'data' event payload represents a full logical unit of data [4]. 3. Manual Boundary Handling: Because the network layer does not enforce boundaries, application-level code must implement its own mechanisms to delineate messages [2][3]. Common strategies include: - Length-prefixing: Sending the size of the message as a header before the actual message body [4]. - Delimiters: Using specific characters (e.g., newline \n or other markers) to signal the end of a message [3]. - Buffering: Accumulating received chunks into a buffer and parsing that buffer for complete messages whenever new data arrives [4][3]. If you need better control over how data is read or processed, you can consider using the 'readable' event and the readable.read method, which allow you to manually consume data from the stream's internal buffer instead of relying on the automatic 'data' event flow [6][7].

Citations:

Accumulate TCP chunks before asserting.

net.Socket data events are stream chunks, so resolving on the first chunk can truncate multi-chunk writes. Accumulate until the expected payload (PONG or PING) is complete before resolving.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/modules/http/regressions/http-non-http-socket-passthrough.test.ts`
around lines 51 - 60, Update the Promise in the socket response test to
accumulate data chunks rather than resolving on the first data event. Resolve
and destroy the socket only once the complete expected payload (PING or PONG, as
applicable) has been received, while retaining error rejection and the final
response assertion.

})

Loading