Fix incorrect resend of session-level messages with ForceResendWhenCorruptedStore#1124
Draft
Fix incorrect resend of session-level messages with ForceResendWhenCorruptedStore#1124
ForceResendWhenCorruptedStore#1124Conversation
…tore is enabled - Modified Session.resendMessages() to remove dependency on forceResendWhenCorruptedStore flag for admin message detection - Added special handling for Reject messages (MsgType=3) - only Reject messages among admin messages are now resent - Other session-level messages (Logon, Logout, Heartbeat, etc.) are properly skipped with SequenceReset GapFill - Handle corrupted store by adding null placeholders instead of fake Heartbeat messages - Added comprehensive test to verify the fix - Added setter for forceResendWhenCorruptedStore in SessionFactoryTestSupport.Builder Co-authored-by: chrjohn <[email protected]>
Co-authored-by: chrjohn <[email protected]>
Copilot
AI
changed the title
[WIP] Fix incorrect resending of session-level messages
Fix incorrect resend of session-level messages with ForceResendWhenCorruptedStore
Jan 27, 2026
ForceResendWhenCorruptedStore
chrjohn
requested changes
Jan 28, 2026
Comment on lines
2355
to
2358
| LOG.error("Cannot read messages from stores, will gap fill over missing messages", e); | ||
| // Add null placeholders for corrupted messages instead of fake Heartbeats | ||
| for (int i = beginSeqNo; i < endSeqNo; i++) { | ||
| final Message heartbeat = messageFactory.create(sessionID.getBeginString(), | ||
| MsgType.HEARTBEAT); | ||
| initializeHeader(heartbeat.getHeader()); | ||
| heartbeat.getHeader().setInt(MsgSeqNum.FIELD, i); | ||
| messages.add(heartbeat.toString()); | ||
| messages.add(null); |
Member
There was a problem hiding this comment.
@copilot please roll back these changes and restore the former logic that puts Heartbeats into the messages list.
Moreover, use the session-specific log instead of the global LOG.
Contributor
Author
There was a problem hiding this comment.
Done in e180fa4. Restored the original logic that creates Heartbeat messages and changed to use getLog().onErrorEvent() for session-specific logging.
Comment on lines
2375
to
2383
| // Handle null placeholders from corrupted store | ||
| if (message == null) { | ||
| // Mark for gap fill | ||
| if (begin == 0) { | ||
| begin = current; | ||
| } | ||
| current++; | ||
| continue; | ||
| } |
…ific log - Restored original corrupted store handling that creates Heartbeat messages instead of null placeholders - Changed LOG.error to getLog().onErrorEvent to use session-specific logging - Removed null placeholder handling code - Kept the core fix for admin message resend logic (Reject messages handling) Co-authored-by: chrjohn <[email protected]>
This was referenced Jan 28, 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.
Fix: Prevent incorrect resend of session-level messages when ForceResendWhenCorruptedStore is enabled
Progress:
resendMessages()in Session.java to:forceResendWhenCorruptedStoreflag for admin message detectionSummary
✅ Core fix for admin message handling maintained
✅ Original Heartbeat logic for corrupted store restored
✅ Session-specific logging used throughout
✅ All session tests pass (72/72)
Original prompt
This pull request was created from Copilot chat.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.