[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

POSIX lets a nonblocking send() or recv() report either, and does not require the two constants to be equal. Where they differ, recv() reporting EAGAIN closed a healthy connection and send() reporting it was never re-armed for writability.

Both are now classified by Socket::IsWouldBlock():

#ifdef _WIN32
  return error == WSAEWOULDBLOCK;
#elif EAGAIN == EWOULDBLOCK
  return error == EWOULDBLOCK;
#else
  return error == EAGAIN || error == EWOULDBLOCK;
#endif

The preprocessor picks the branch, which is what makes this workable: an earlier revision wrote both comparisons unconditionally and clang-tidy reported misc-redundant-expression on every platform where the constants are equal, which is all of the ones CI builds. Measured on all-options-abiv1-preview, socket_tools.h carries the same five warnings as it does on main and no misc-redundant-expression.

ErrorWouldBlock stays: it is part of the public surface of an installed header. The test uses the helper rather than repeating the errno policy, and SocketToolsTests.RecognizesWouldBlockErrors pins that EPIPE is not classified as would-block, which a return true would otherwise satisfy.

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 80.89%. Comparing base (2beddad) to head (c2db907).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4283      +/-   ##
==========================================
+ Coverage   80.84%   80.89%   +0.05%     
==========================================
  Files         448      448              
  Lines       19161    19168       +7     
==========================================
+ Hits        15489    15504      +15     
+ Misses       3672     3664       -8     

Files with missing lines	Coverage Δ
...nclude/opentelemetry/ext/http/server/http_server.h	67.81% <100.00%> (+1.14%)	⬆️
...clude/opentelemetry/ext/http/server/socket_tools.h	97.33% <100.00%> (+1.66%)	⬆️

... and 1 file 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.

[thc1006]

Closing following @dbarker's guidance on #4287: http_server.h and socket_tools.h are internal, supporting the tests and one example, and the bar for them is stable and correct enough for that rather than production hardening. This is portability work beyond that bar, so it should not sit in the review queue. Happy to reopen if the bar ever moves.