-
Notifications
You must be signed in to change notification settings - Fork 595
Description
Environment Details
- Helidon Version: 4.3.4
- Helidon SE or Helidon MP: SE
- JDK version: 25.0.2
- OS: MacOs Tahoe
Another show-stopper for me:
Bug description
Http1ServerResponse.sink() creates a sink that writes directly to the low-level dataWriter, but never marks the response as having an entity. This causes Helidon's routing to think no response was produced.
The check (HttpRoutingImpl.java, lines 196-211)
After every handler returns, the router checks in order:
- response.shouldReroute() → false
- response.isNexted() → false
- response.hasEntity() → false ← BUG: should be true
- → throws "A route MUST call either send, reroute, or next"
Why hasEntity() returns false?
Http1ServerResponse.hasEntity() (line 274):
public boolean hasEntity() {
return isSent || streamingEntity;
}- isSent — only set to true in the closeRunnable (line 326), which runs when the sink is closed, not when it's created. An SSE sink is long-lived — it may not close until much later.
- streamingEntity — only set to true by outputStream() (line 451). The DataWriterSseSink bypasses outputStream() entirely — it writes directly to ctx.dataWriter().writeNow() (DataWriterSseSink line 124).
So, after the handler calls response.sink(SseSink.TYPE), writes status+headers, and emits SSE events, neither flag is set.
Suggested fix
In Http1ServerResponse.sink() (line 308), after the sink is successfully created, mark the response:
public <X extends Sink<?>> X sink(GenericType<X> sinkType) {
for (SinkProvider<?> p : SINK_PROVIDERS) {
if (p.supports(sinkType, request)) {
X sink = (X) p.create(...);
this.isSent = true; // ← the fix: sink took over the response
return sink;
}
}
...
}Setting isSent = true immediately is correct because the sink's constructor (DataWriterSseSink line 76) already calls writeStatusAndHeaders() — the HTTP status and headers have been flushed to the wire. The response is sent at that point.
Setting isSent = true in sink() is the right approach because:
1. Headers are already written - DataWriterSseSink constructor calls writeStatusAndHeaders() immediately
2. Sink owns the response - Once created, only the sink should write to the response
3. Consistent with send() - Calling res.send() also sets isSent = true
Minimal reproducer:
routing(HttpRouting.builder()
.any("/sse", (req, res) -> {
var sink = res.sink(SseSink.TYPE);
sink.emit(SseEvent.builder().data("test").build());
// BUG: No res.send() called, throws error even though SSE was sent
}))