Don't spend an in-flight credit when nothing was sent

[MitchBradley]

Problem

With ASYNCWEBSERVER_USE_CHUNK_INFLIGHT enabled (the default), a chunked or unknown-length response can stall permanently if its source isn't ready on a couple of consecutive calls.

AsyncAbstractResponse::write_send_buffs() takes an in-flight credit at the end of the content stage on every call:

    _in_flight += payloadlen;
    --_in_flight_credit;  // take a credit

It does this even when _fillBufferAndProcessTemplates() returned RESPONSE_TRY_AGAIN and payloadlen is 0. Credits come back only with acks of sent data (len > 0); polls never return one. So:

1. The response starts with 2 credits.
2. Each call that finds the source empty spends one, and nothing is sent, so no ack will ever return it.
3. After two such calls, _in_flight_credit is 0. Every later poll hits the !_in_flight_credit early return, the user callback is never called again, and the response hangs until the connection times out.

This hits any response whose producer can be momentarily slower than the connection drains: a chunked response fed by another task, for example.

How it showed up

In FluidNC, HTTP commands run on another task and stream their output into a small buffer that a chunked response drains, returning RESPONSE_TRY_AGAIN while the buffer is empty. When the network side drained faster than the command produced output, the response stalled after its first couple of empty fills. The command task then gave up on the full, undrained buffer, and clients received truncated replies (the first 2 KB of a 9 KB JSON settings listing).

It's easiest to reproduce on a host build with a loopback-fast transport where acks are immediate. On ESP32 the same sequence needs a producer that falls behind the TCP drain, so it's intermittent there.

Fix

Take a credit, and count _in_flight, only when payloadlen is nonzero. An empty call puts nothing in flight, so it shouldn't consume flight credit. Retries from polls then keep working until the source has data, and normal ack pacing resumes from there.

Testing

On a host (Emscripten) build of FluidNC, using a custom AsyncTCP transport with immediate acks and ASYNCWEBSERVER_USE_CHUNK_INFLIGHT=1, fetching the ~9 KB settings listing over a chunked response:

• before: 8 of 9 replies truncated (2047 or 3000 bytes)
• after: 15 of 15 replies complete and valid, across five fresh starts; FluidNC's other HTTP and WebSocket tests also pass

Not yet tested on ESP32 hardware with lwIP. The change only affects calls that write zero bytes, so it can't increase data in flight.

🤖 Generated with Claude Code

[Copilot]

🟢 Approval recommended

The focused change correctly preserves retry capability without altering accounting for transmitted data.

0 open findings

What changed in this PR

Prevents stalled chunked or unknown-length responses when their source temporarily has no data.

Changes:

• Consume in-flight credit only when bytes are actually sent.
• Document why zero-byte retries must retain credit.

File	Description
src/​WebResponses.cpp	Guards in-flight accounting against zero-byte sends.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟢 Approval recommended

The focused change correctly preserves credits for zero-byte retries without altering normal send accounting.

0 open findings

🧠 Review effort: Balanced

[mathieucarbou]

Thank you for this bug fix - I agree with the analysis we might have missed this use case!

I will try to test this weekend.

[mathieucarbou]

@willmmiles : pinging you to approve this small fix also. thanks!

[willmmiles]

I'd largely ignored this feature as somewhat misguided -- maybe it made sense at some point, but it definitely doesn't today.

In the case of a client returning data in small chunks, the current implementation of AsyncAbstractResponse::write_send_buffs will loop over _fillBuffer until it fills the outgoing packets completely (or the client requests a backoff with RESPONSE_TRY_AGAIN). Further, each connection is limited by the send buffer size to only 4 outstanding packets. Artificially throttling any one connection won't improve the transfer rates of others unless there's sufficient memory pressure to exhaust the underlying packet buffer pool -- transfers will pretty much always proceed at the rate and in the order that acks are received from their client partners. (It's worth noting that a single dropped ack can tank a connection for a very long retransmission timeout, but no amount of playing games with outbound packets will affect the inbound packet drop rate.)

Anyways, this fix is a valid correction, for all that IMO the "feature" is most likely counterproductive.

[mathieucarbou]

I'd largely ignored this feature as somewhat misguided -- maybe it made sense at some point, but it definitely doesn't today.

Yes we had some bug reports when dealing with some long connection problems like when reading files from sdcard or processing a long request. There are some examples in the project about these corner cases that were not possible before and were made possible by introducing the if light flag.

[willmmiles]

I'd largely ignored this feature as somewhat misguided -- maybe it made sense at some point, but it definitely doesn't today.

Yes we had some bug reports when dealing with some long connection problems like when reading files from sdcard or processing a long request. There are some examples in the project about these corner cases that were not possible before and were made possible by introducing the if light flag.

Thanks - would you mind giving me a pointer to a specific example? I don't see any mention of the inflight flag in any of the examples, but maybe I'm not looking in the right place?

It really looks to me like it's now unnecessary. The better solution - looping over the available send buffer, instead of mapping one incoming callback to exactly one user code fetch - was introduced here in #316. I recall being happy to see that approach PR'd, as it's pretty much exactly what I'd done to mitigate this problem in the WLED fork. (In fact all the inflight logic was part of what held me back from upstreaming that code easily since the functions were so divergent -- I didn't really understand what it was for (and arguably still don't!), and I didn't have the time to do a manual merge.)

[mathieucarbou]

I'd largely ignored this feature as somewhat misguided -- maybe it made sense at some point, but it definitely doesn't today.

Yes we had some bug reports when dealing with some long connection problems like when reading files from sdcard or processing a long request. There are some examples in the project about these corner cases that were not possible before and were made possible by introducing the if light flag.

Thanks - would you mind giving me a pointer to a specific example? I don't see any mention of the inflight flag in any of the examples, but maybe I'm not looking in the right place?

It really looks to me like it's now unnecessary. The better solution - looping over the available send buffer, instead of mapping one incoming callback to exactly one user code fetch - was introduced here in #316. I recall being happy to see that approach PR'd, as it's pretty much exactly what I'd done to mitigate this problem in the WLED fork. (In fact all the inflight logic was part of what held me back from upstreaming that code easily since the functions were so divergent -- I didn't really understand what it was for (and arguably still don't!), and I didn't have the time to do a manual merge.)

It was SlowChunkResponse: the idea for example was to serve a response (chunk encoding) with sdcard i/o (slow). File system access and directory listing is slow so the chunk callback was called and was executing slowly, slowing down the async_tcp task (we simulated that with a delay).

edit:

Maybe ChunkRetryResponse is also worth looking, and definitely also LargeResponse => there were issues serving large responses (it was impossible before)