[BUG] Fix transient would-block handling in the ext/http embedded server

[thc1006]

Fixes #4281
Fixes #4282
Refs #4287

Two robustness bugs in the ext/http embedded server's would-block handling, both found while reviewing #4280, plus a third that review of this PR turned up in the fix itself.

sendMore() drops the response on write backpressure (#4281)

When send() returns -1 with EWOULDBLOCK, the old code skipped the error early-return and fell through to sendBuffer.erase(0, sent); with sent == -1 the count converts to SIZE_MAX and wipes the whole unsent response. It now never erases on sent < 0, and re-registers the socket for writability on a would-block so the buffer is kept and retried.

onSocketReadable() closes on transient recv errors (#4282)

The old code closed the connection on any recv() <= 0. For a nonblocking socket, recv() == -1/EWOULDBLOCK is transient, not a close. It now closes only on an orderly shutdown (recv() == 0) or a real error, and ignores a transient would-block.

Restoring read interest after an asynchronous 100 Continue

This one came out of review of this PR, and it was introduced by the first fix above.

Reactor::addSocket() replaces the interest set rather than adding to it, so the new would-block path leaves a socket registered for writability only. Every other sending state either keeps sending or re-registers Readable once it finishes, but Sending100Continue moved to ReceivingBody without doing so and then returned to wait for a request body it could no longer be woken for. The old code avoided this by accident: it wiped the buffer on would-block, so it never reached the addSocket() call at all.

I checked the Linux half with an epoll probe registering only EPOLLOUT on a socketpair that already had readable bytes waiting:

  registered EPOLLOUT only; peer has 17 readable bytes waiting
    wait 0: n=1 events=0x4  EPOLLIN=no EPOLLOUT=yes
    wait 1: n=1 events=0x4  EPOLLIN=no EPOLLOUT=yes
    wait 2: n=1 events=0x4  EPOLLIN=no EPOLLOUT=yes

Every wait returns immediately with EPOLLOUT and never EPOLLIN, so the connection would spin without ever reading the body. On Windows there would be no further FD_WRITE and the request would simply stall.

The interest set is now restored explicitly on the Sending100Continue -> ReceivingBody transition rather than left to a later event, and a deterministic test covers it (see Test coverage).

Error code capture ordering

Both paths read the socket error immediately after send()/recv(), before LOG_TRACE can clobber errno or the Winsock last error.

On EAGAIN vs EWOULDBLOCK

An earlier revision also checked EAGAIN, but EAGAIN and EWOULDBLOCK have the same value on every platform this repo builds on (Linux, macOS; Winsock has only WSAEWOULDBLOCK), so checking both is redundant and trips clang-tidy misc-redundant-expression. A single ErrorWouldBlock check is therefore used. POSIX does permit the two to differ, so this is a statement about the platforms here rather than a general one.

What this PR does not fix

All tracked in #4287, and I have narrowed the title so this PR does not read as a complete fix of the nonblocking write state machine:

• Windows short writes. sendMore() still performs a single send() per readiness event. Winsock permits a nonblocking send() to succeed having transferred only part of the buffer, and only guarantees a further FD_WRITE after a send has failed with WSAEWOULDBLOCK. Re-registering does not help, since addSocket() is a no-op when the flag set is unchanged. A positive short write on a large response can therefore stall. The fix is a drain loop and it deserves its own PR.
• Fatal send() errors still return true without closing the connection, removing the reactor registration or changing state.
• EINTR is not classified as retryable.
• static_cast<int>(conn.sendBuffer.size()) narrows for a handler-produced body near INT_MAX.
• SIGPIPE is [BUG] Embedded HTTP server writes can raise SIGPIPE and terminate the process #4293, with PR [BUG] Stop embedded HTTP server writes from raising SIGPIPE #4294 up. That one is worth landing early, since otherwise the process can die before any of the fatal-error handling above is reached.

Test coverage

All three fixed branches have deterministic regression tests in ext/test/http/http_server_test.cc, each verified fail-before/pass-after against the production header:

• WouldBlockPreservesTheSendBuffer fills a nonblocking socket's send buffer, calls the protected sendMore(), and asserts the response is kept rather than wiped by erase(0, -1) and that the socket is re-armed Writable | Closed. On the normal single-threaded Linux level-triggered path this branch is not expected from the writable callback (EPOLLOUT fires once the kernel has space, so that send() returns a positive count); it is reached from the readable path on a backlogged keep-alive connection, which the test constructs directly.
• WouldBlockKeepsTheConnection drives the private onSocketReadable() override through the reactor's public SocketCallback reference on a nonblocking socket with no data, and asserts the transient EWOULDBLOCK leaves the connection registered and the socket open.
• CompletedInterimResponseRestoresReadInterest stages a completed Sending100Continue and calls the protected handleConnection(), asserting the transition to ReceivingBody restores Readable | Closed.

The two reactor-callback tests reach private and protected HttpServer state through a test subclass and the existing public Reactor::SocketCallback interface, so no production header seam was added.

Verification

• ext/test/http/http_server_test.cc passes under ASan and UBSan; each of the three regression tests was verified fail-before/pass-after against the relevant pre-fix revision (the send- and recv-side tests fail against the original base, and the 100-Continue transition test fails without the explicit read-interest restoration).
• clang-format 18 clean.
• clang-tidy 18 with the repo .clang-tidy on http_server.h: the same pre-existing modernize-use-emplace warnings before and after, and the fix adds none. This PR does not change warning_limit.
• The epoll behavior quoted above was measured rather than assumed.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.27%. Comparing base (29d7a75) to head (640ae45).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4283      +/-   ##
==========================================
+ Coverage   81.25%   81.27%   +0.03%     
==========================================
  Files         446      446              
  Lines       18872    18877       +5     
==========================================
+ Hits        15332    15341       +9     
+ Misses       3540     3536       -4     

Files with missing lines	Coverage Δ
...nclude/opentelemetry/ext/http/server/http_server.h	67.08% <100.00%> (+1.17%)	⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[thc1006]

Update (cc75666): the two lines this note originally flagged are now covered. HttpServerSendMoreTest.WouldBlockPreservesTheSendBuffer drives the sendMore() would-block branch directly (the addSocket() re-arm and the early return true), and the recv-would-block and 100-Continue branches have their own deterministic tests, so Codecov now reports 100% patch coverage — no socket_tools.h test seam was needed.

Separately, reading the rest of the file for this fix turned up several further problems, including what looks like a use-after-free when a handler returns -1. They are collected with line references in #4287, and the memory-safety one has its own issue in #4288. None of them are in scope here, and this PR does not depend on any of them.

[thc1006]

The macOS job here is red on config.ProgrammaticConfigTest.EnabledConfigProducesProviders, and I do not think it is related to this change.

It fails as a CTest timeout rather than an assertion, and sdk/test/configuration/programmatic_configuration_test.cc contains no reference to http_server.h or anything else under ext/http, so the header this PR touches is not in that test binary. The two failing runs took 33 minutes against 9 to 12 for the ones that passed, which fits the timeout itself being what consumed those minutes.

Four of my other open PRs passed the same job today, two of them in a window that overlaps a failing run, so a bad runner period does not look like the explanation either. I have also seen this test time out on the pre-rebase revision of this PR and on the tsan job.

I cannot re-run jobs on this repository. Happy to rebase if a clean run would be more useful than my word for it, although the branch is only behind by dependabot commits.