chore: expose HTTP accept primitives#193
Conversation
Adds the minimal API needed by nim-libp2p to avoid head-of-line blocking on slow or malformed WebSocket handshakes.
richard-ramos
force-pushed
the
expose-api
branch
from
1c12189 to
16cd25d
Compare
richard-ramos
changed the title
There was a problem hiding this comment.
Pull request overview
This PR exposes lower-level HTTP accept primitives in websock/http/server.nim so downstream consumers (e.g., nim-libp2p) can accept a connection/stream and control how/when the HTTP request is read, helping avoid head-of-line blocking from slow or malformed WebSocket handshakes.
Changes:
- Exported
readHttpRequestso it can be used externally. - Added
acceptStream*to accept a connection and return anAsyncStreamwithout reading the HTTP request. - Refactored
accept*to callacceptStream*and thenreadHttpRequest.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| trace "Got new request", isTls = server.secure | ||
| stream |
There was a problem hiding this comment.
acceptStream logs "Got new request", but at this point no HTTP request has been read yet (only the transport has been accepted and wrapped in an AsyncStream). This can be misleading in production diagnostics; consider changing it to "Got new connection/stream" or moving the log to accept after readHttpRequest succeeds.
| proc acceptStream*(server: HttpServer): Future[AsyncStream] {.async.} = | ||
| if not isNil(server.handler): |
There was a problem hiding this comment.
New public APIs acceptStream* and readHttpRequest* are introduced/refactored here, but there are no tests covering the accept-style flow (and specifically acceptStream). Adding an async test that accepts a connection via acceptStream, parses it via readHttpRequest, and completes a basic WebSocket handshake would help prevent regressions and validate the intended head-of-line blocking mitigation.
Adds the minimal API needed by nim-libp2p to avoid head-of-line blocking on slow or malformed WebSocket handshakes.