fix: log connections that timeout, abort, or send unparsable data

[itslenny]

What kind of change does this PR introduce?

Bug fix

What is the current behavior?

There is no log entry created for connections that don't trigger a request.

• connections that timeout (at the node:http level)
• connections that are aborted before a full http request is received
• connections that send data that can't be parsed

What is the new behavior?

Create a log entry for all connections that don't result in a request event

Example logs

Disconnect before data sent

# close connection after 1 second
timeout 1 tail -f /dev/null | nc localhost 5000

Node http connection timeout

# leave connection open triggers node:http timeout
tail -f /dev/null | nc localhost 5000

Send unparsable data

printf "saldfijasdflijasdflisadfsldif" | nc localhost 5000

Send incomplete data

printf "POST /object/mybucket/file.txt HTTP/1.1\r\nHost: localhost:5000\r\nUser-Agent: MyClient/1.0\r\nx-forwarded-host: bjhaohmqunupljrqypxz.local.com\r\nAuthorization: Bearer xxx\r\n" | nc localhost 5000

Send complete headers with no payload

# Triggers ABORTED REQ (existing behavior)
printf "POST /object/mybucket/file.txt HTTP/1.1\r\nHost: localhost:5000\r\nUser-Agent: MyClient/1.0\r\nx-forwarded-host: bjhaohmqunupljrqypxz.local.com\r\nAuthorization: Bearer xxx\r\n\r\n" | nc localhost 5000

[coveralls]

Pull Request Test Coverage Report for Build 19147127865

Details

• 76 of 170 (44.71%) changed or added relevant lines in 3 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.2%) to 77.584%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/http/plugins/log-request.ts	52	89	58.43%
src/internal/http/partial-http-parser.ts	23	80	28.75%

Totals
Change from base Build 19105992955:	-0.2%
Covered Lines:	24663
Relevant Lines:	31468

---

💛 - Coveralls

[fenos]

we would need to remove these event handlers at the end of the request otherwise we will create memory leaks

[fenos]

Also we need to consider that we use keep-alive so connections might be reused hence the listeners would be removed in the current implementation

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• New Features
  • Added comprehensive logging for aborted socket connections, capturing partial request data to enhance diagnostics and monitoring when connections close before requests are fully received.

Walkthrough

The changes introduce socket-level tracking for logging aborted connections that close before a complete HTTP request is parsed. A new partial HTTP parser module extracts method, URL, headers, and tenant information from buffered socket data. The log-request plugin registers an onConnection handler to buffer incoming data and a cleanup mechanism to log "ABORTED CONN" events when sockets close prematurely. A helper function constructs minimal request objects from partial data for logging consistency. The LogRequestOptions interface is updated to include the new 'ABORTED CONN' status value alongside existing statuses.

Sequence Diagram

sequenceDiagram
    participant Client as Client/Socket
    participant Plugin as Log-Request Plugin
    participant Parser as Partial HTTP Parser
    participant Logger as Logger
    
    Client->>Plugin: Connects (onConnection)
    Plugin->>Plugin: Register socket lifecycle cleanup
    Client->>Plugin: Send partial request data (≤2048 bytes)
    Plugin->>Plugin: Buffer incoming data chunks
    Client->>Plugin: Close before full request parsed
    Plugin->>Parser: parsePartialHttp(bufferedData)
    Parser->>Parser: Parse method, URL, headers, tenantId
    Parser-->>Plugin: Return PartialHttpData
    Plugin->>Plugin: createPartialLogRequest(partial data)
    Plugin->>Logger: Log event with status 'ABORTED CONN'
    Logger-->>Plugin: Event logged
    Plugin->>Plugin: Cleanup socket reference

Loading

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

src/http/plugins/log-request.ts (1)

43-48: ⚠️ Potential issue | 🔴 Critical

The per-socket cleanup becomes unusable after the first request.

socketCleanupMap.set() happens once per socket, but cleanupSocketListeners() deletes that entry the first time a request finishes. After that, later requests on the same connection never clear waitingForRequest/currentRequestData, so a reused socket can false-positive ABORTED CONN or double-log after ABORTED REQ/RES.

Also applies to: 61-67, 131-141, 188-189

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/http/plugins/log-request.ts` around lines 43 - 48, The socket-cleanup map
entry is removed by cleanupSocketListeners(), making per-socket state
(socketCleanupMap, waitingForRequest, currentRequestData) unavailable for
subsequent requests on the same connection; instead of deleting the socket's map
entry inside cleanupSocketListeners, only remove the listeners/timeouts it
registered and preserve the map entry (or reinitialize/reset
waitingForRequest/currentRequestData per new request). Update
cleanupSocketListeners to avoid socketCleanupMap.delete(socket) and ensure code
that begins a new request (where waitingForRequest/currentRequestData are set)
either reassigns/clears those fields per-request or relies on a persistent
per-socket entry; reference functions/variables to change:
cleanupSocketListeners, socketCleanupMap, waitingForRequest, currentRequestData
(and the places that currently call socketCleanupMap.set so they either set
per-request or keep the persistent entry).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/http/plugins/log-request.ts`:
- Around line 53-59: The zero-byte connection timeout is missed because
waitingForRequest is only set in onData; update onConnection/onData handling so
that waitingForRequest is true immediately when a socket connects (in
onConnection) and ensure the socket close/timeout handler checks for zero-length
requests (currentRequestDataSize === 0) and !pendingRequestLogged to emit the
timeout/no-request log; adjust resetting logic for pendingRequestLogged and
currentRequestStart inside onData and after emitting the log so the zero-byte
path is covered even if onData never ran.

---

Duplicate comments:
In `@src/http/plugins/log-request.ts`:
- Around line 43-48: The socket-cleanup map entry is removed by
cleanupSocketListeners(), making per-socket state (socketCleanupMap,
waitingForRequest, currentRequestData) unavailable for subsequent requests on
the same connection; instead of deleting the socket's map entry inside
cleanupSocketListeners, only remove the listeners/timeouts it registered and
preserve the map entry (or reinitialize/reset
waitingForRequest/currentRequestData per new request). Update
cleanupSocketListeners to avoid socketCleanupMap.delete(socket) and ensure code
that begins a new request (where waitingForRequest/currentRequestData are set)
either reassigns/clears those fields per-request or relies on a persistent
per-socket entry; reference functions/variables to change:
cleanupSocketListeners, socketCleanupMap, waitingForRequest, currentRequestData
(and the places that currently call socketCleanupMap.set so they either set
per-request or keep the persistent entry).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: ba8cb604-ec80-4e7b-b1cf-3d6228a2a370

📥 Commits

Reviewing files that changed from the base of the PR and between bb602aa and 14342a7.

📒 Files selected for processing (3)

• src/http/plugins/log-request.ts
• src/internal/http/index.ts
• src/internal/http/partial-http-parser.ts

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Zero-byte connection timeouts are still skipped.

If a client connects and idles until node:http closes the socket, onData never runs, so waitingForRequest stays false and this guard exits without emitting a log. That misses the exact timeout/no-request path this PR is meant to cover.

Also applies to: 98-100

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/http/plugins/log-request.ts` around lines 53 - 59, The zero-byte
connection timeout is missed because waitingForRequest is only set in onData;
update onConnection/onData handling so that waitingForRequest is true
immediately when a socket connects (in onConnection) and ensure the socket
close/timeout handler checks for zero-length requests (currentRequestDataSize
=== 0) and !pendingRequestLogged to emit the timeout/no-request log; adjust
resetting logic for pendingRequestLogged and currentRequestStart inside onData
and after emitting the log so the zero-byte path is covered even if onData never
ran.