capture SSL error on send/send-bytes write failure - #6
Merged
Merged
Conversation
There was a problem hiding this comment.
Build & Tests
Build: clean, no warnings.
Tests: all 19 pass (including loopback TLS echo tests that exercise the send paths).
CI: passing on both ubuntu-latest and macos-latest.
Findings
None. The fix is correct and complete:
- The added
carp_tls_capture_ssl_error(SSL_get_error(s->ssl, n))calls inTlsStream_send_andTlsStream_send_MINUS_bytes_exactly match the pattern already used inTlsStream_send_MINUS_nb_,carp_tls_read_status,TlsStream_read_MINUS_append_MINUS_nb_,TlsStream_connect_, andTlsStream_accept_. SSL_get_erroris called immediately after the failingSSL_write, before any other OpenSSL calls — correct timing.- The
n <= 0guard correctly handles both errors (n < 0) and unclean shutdown (n == 0);carp_tls_capture_ssl_errorhandlesSSL_ERROR_ZERO_RETURNappropriately. - No other send-like functions are missing the error capture — these were the last two.
Verdict: merge
Straightforward, correct fix that brings the two blocking send paths in line with the rest of the file.
hellerve
approved these changes
Jun 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TlsStream_send_andTlsStream_send_MINUS_bytes_returned -1 on write failure without callingcarp_tls_capture_ssl_error, soTlsStream.error-would return a stale message from a prior operation. This adds the samecarp_tls_capture_ssl_error(SSL_get_error(...))call that the non-blockingsend-nband the read paths already use.Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.