bryancall commented on PR #13316: URL: https://github.com/apache/trafficserver/pull/13316#issuecomment-4792550156
Heads up on a possible gap in this downgrade. The STREAM-class branch in `rcv_frame` that this moves to `Http2StreamDebug` is reached not only by client protocol errors but by server-side `INTERNAL_ERROR` returns from `rcv_data_frame`: - `read_vio_writer() == nullptr` -> `HTTP2_ERROR_CLASS_STREAM` / `INTERNAL_ERROR` (`"no writer"`) - short `writer->write()` -> `HTTP2_ERROR_CLASS_STREAM` / `INTERNAL_ERROR` (`"Write mismatch"`) The #13059 rationale covers malformed inbound *requests* (header/parse failures), which are genuine bad-client noise. It doesn't cover a mid-DATA internal write fault: that's ATS failing internally, and after this change the only trace is gated behind the `http2` debug tag, which is off in production. A recurring internal buffer/VIO fault would then surface only as clients seeing intermittent RST_STREAM with no server-side log. Would it be worth branching on `error.code` here, keeping `Error()` (or a rate-limited `Warning()`) for `HTTP2_ERROR_INTERNAL_ERROR` and using `Http2StreamDebug` for the client-fault codes? One thing I wasn't sure of: whether a null read-VIO or short write is ever client-reachable (e.g. a client resetting its read side mid-stream). If it is, the severity argument weakens and debug is fine. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
