Skip to content

chore: expose HTTP accept primitives #193

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

Merged
richard-ramos merged 1 commit into from
May 6, 2026
Merged

chore: expose HTTP accept primitives #193

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
13 changes: 9 additions & 4 deletions websock/http/server.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ template used(x: typed) =
# silence unused warning
discard

proc readHttpRequest(
proc readHttpRequest*(
stream: AsyncStream, headersTimeout: Duration
): Future[HttpRequest] {.
async: (raises: [CancelledError, AsyncStreamError, HttpError])
Expand Down Expand Up @@ -129,9 +129,7 @@ proc handleConnCb(
finally:
await stream.closeWait()

# TODO async raises not implemented for accept because it breaks libp2p (1.13.0
# at the time of writing)
proc accept*(server: HttpServer): Future[HttpRequest] {.async.} =
proc acceptStream*(server: HttpServer): Future[AsyncStream] {.async.} =
if not isNil(server.handler):
Comment on lines +132 to 133
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
raise newException(
HttpError, "Callback already registered - cannot mix callback and accepts styles!"
Expand All @@ -145,6 +143,13 @@ proc accept*(server: HttpServer): Future[HttpRequest] {.async.} =
raise (ref HttpError)(msg: error)

trace "Got new request", isTls = server.secure
stream
Comment on lines 145 to +146
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.

# TODO async raises not implemented for accept because it breaks libp2p (1.13.0
# at the time of writing)
proc accept*(server: HttpServer): Future[HttpRequest] {.async.} =
let stream = await server.acceptStream()

try:
await stream.readHttpRequest(server.headersTimeout)
except CancelledError as exc:
Expand Down
Loading