Ensure a Message's Response streamId is always set#4936
Conversation
patch by Francisco Guerrero; reviewed by TBD for CASSANDRA-21508
| ErrorMessage errorMessage = ErrorMessage.fromException(cause, handler); | ||
| // No request in scope at the channel level; a WrappedException cause carries the frame's stream id | ||
| // and overrides this 0 fallback, otherwise the channel is torn down for fatal errors. | ||
| ErrorMessage errorMessage = ErrorMessage.fromException(cause, 0, handler); |
There was a problem hiding this comment.
I would probably not use a valid streamId as a placeholder you intend to overwrite later
There was a problem hiding this comment.
Yeah, that's a very good point. I will update it
There was a problem hiding this comment.
One thing to consider here is that we won't always overwrite the stream id. I considered using the sentinel value, but this would trip the assertion during the encoding. So we'll need to keep a valid value here.
The cases where the streamId will not be overwritten are:
- ExceptionHandlers.java:81 / PreV5Handlers.java:342 : This occurs during channel negotiations (SSL/TLS handshake failures) and a stream ID is not available
- Unexpected non-wrapped pipeline throwables where no frame/stream ID exists at all
Everything else arrives in a wrapped exception and carries a real stream ID :
- CQLMessageHandler:517 : extracted.streamId() (best-effort)
- CQLMessageHandler:534 (oversized auth) : header.streamId
- CQLMessageHandler:725 (corrupt frame) : NO_REQUEST_STREAM_ID — explicit, the one true context-free CQL path
I think keeping 0 for the NO_REQUEST_STREAM_ID is reasonable because the only sources of missing stream ids are when an error occurs before a stream id is available or when a a corrupt frame does not allow us to extract the stream id.
2 participants
patch by Francisco Guerrero; reviewed by TBD for CASSANDRA-21508