Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 14, 2026

Add failing test for ResendRequest bug #1114

  • Explore repository structure and understand existing test patterns
  • Review Session, SessionState, and ResendRequest logic
  • Understand build and test infrastructure (Maven)
  • Create SessionResendRequestFailureTest.java with failing test
  • Build and run the new test to verify it demonstrates the bug
  • Verify test compiles and fails as expected (demonstrates bug successfully)
  • Request code review and address feedback
  • Address PR review comments:
    • Removed lines 82-83 (unnecessary processMessage call - ResendRequest already triggered by Logon)
    • Added log output of ResendRange after disconnect (line 78-79)
    • Added log output of ResendRange before reconnect logon (line 86-87)
  • Refactored test to directly call Session.generateResendRequest() using reflection
    • Test now follows the exact flow: logon → disconnect → call generateResendRequest() with failing responder
    • Added callGenerateResendRequest() helper method to invoke the private method via reflection
    • This directly demonstrates the bug where sendResendRequest() calls sendRaw() and then sets ResendRange regardless of sendRaw() return value

Test Status

Test successfully demonstrates the bug

  • Test compiles cleanly
  • Test fails as expected, showing ResendRequest is not re-sent after reconnection
  • Test directly calls Session.generateResendRequest() which triggers: generateResendRequest()sendResendRequest()sendRaw() (returns false) → state.setResendRange() (still called!)
  • Log output shows ResendRange state at key points for debugging
  • Test clearly demonstrates the problematic code path in Session.sendResendRequest()
Original prompt

Context

This PR adds a failing unit test to demonstrate the bug reported in issue #1114: #1114

Problem Summary:
When a QuickFIX session disconnects after a ResendRequest is triggered but before it's actually sent (because responder becomes null), the ResendRange is still marked as "sent" even though sendRaw() returns false. On reconnection, the ResendRequest is never re-sent because the engine thinks it was already sent.

Task

Create a new test file quickfixj-core/src/test/java/quickfix/SessionResendRequestFailureTest.java with the following content:

package quickfix;

import org.junit.Test;
import quickfix.field.*;
import quickfix.fix44.Heartbeat;
import quickfix.fix44.Logon;

import java.time.LocalDateTime;
import java.time.ZoneOffset;

import static org.junit.Assert.*;

/**
 * Test for issue #1114 - ResendRequest never re-sent after re-logon 
 * when previous request failed due to responder == null
 */
public class SessionResendRequestFailureTest {

    /**
     * Test demonstrates the problem: 
     * 1. Session connects and logs on
     * 2. Receives message with high sequence number, triggering ResendRequest
     * 3. Session disconnects BEFORE the ResendRequest is actually sent (responder becomes null)
     * 4. ResendRange is still marked as "sent" even though sendRaw returned false
     * 5. On reconnection, ResendRequest is NOT re-sent because it's marked as already sent
     */
    @Test
    public void testResendRequestNotResentAfterDisconnectBeforeSend() throws Exception {
        // Setup
        final SessionID sessionID = new SessionID(FixVersions.BEGINSTRING_FIX44, "SENDER", "TARGET");
        final UnitTestApplication application = new UnitTestApplication();
        final FailingResponder responder = new FailingResponder();
        
        final SessionSettings settings = new SessionSettings();
        settings.setString(sessionID, "BeginString", FixVersions.BEGINSTRING_FIX44);
        settings.setString(sessionID, "SenderCompID", "SENDER");
        settings.setString(sessionID, "TargetCompID", "TARGET");
        settings.setString(sessionID, "StartTime", "00:00:00");
        settings.setString(sessionID, "EndTime", "00:00:00");
        settings.setString(sessionID, "HeartBtInt", "30");
        
        final DefaultSessionFactory factory = new DefaultSessionFactory(
                application,
                new MemoryStoreFactory(),
                new ScreenLogFactory(true, true, true));
        
        try (Session session = factory.create(sessionID, settings)) {
            session.setResponder(responder);
            
            // Step 1: Logon normally
            logonTo(session, 1);
            assertTrue("Session should be logged on", session.isLoggedOn());
            
            // Step 2: Receive a message with sequence number gap (e.g., expect 2, get 10)
            // This should trigger a ResendRequest
            final SessionState state = getSessionState(session);
            assertEquals("Expected target seq num should be 2", 2, state.getNextTargetMsgSeqNum());
            
            // Step 3: Configure responder to fail (simulating disconnect before send completes)
            responder.setShouldFail(true);
            
            // Receive heartbeat with seq num 10 (gap from 2 to 9)
            // This should trigger ResendRequest, but it will fail to send
            processMessage(session, createHeartbeatMessage(10));
            
            // Step 4: Verify that ResendRange is marked as sent even though sendRaw failed
            assertTrue("ResendRange should be marked as requested (BUG!)", state.isResendRequested());
            SessionState.ResendRange resendRange = state.getResendRange();
            assertEquals("ResendRange begin should be 2", 2, resendRange.getBeginSeqNo());
            assertEquals("ResendRange end should be 9", 9, resendRange.getEndSeqNo());
            
            // Step 5: Disconnect and reconnect
            session.disconnect("Simulating disconnect", false);
            assertFalse("Session should be disconnected", session.isLoggedOn());
            
            // Reconnect with a fresh responder
            final UnitTestResponder freshResponder = new UnitTestResponder();
            session.setResponder(freshResponder);
            logonTo(session, 11); // Logon with next sequence number
            
            // Step 6: Send another message that should trigger the ResendRequest again
            processMessage(session, createHeartbeatMessage(12));
            
            // Expected: ResendRequest should be sent again because the previous one failed
            // Actual (BUG): ResendRequest is NOT sent because ResendRange is still marked as "sent"
            
            // This assertion will FAIL with the current bug - demonstrating the problem
            assertFalse("EXPECTED: ResendRequest should have been re-sent and satisfied...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

<!-- START COPILOT CODING AGENT TIPS -->
---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey).

Copilot AI and others added 2 commits January 14, 2026 19:51
Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>
Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>
Copilot AI changed the title [WIP] Add failing unit test for resend request bug Add failing test for ResendRequest not re-sent after failed send (issue #1114) Jan 14, 2026
Copilot AI requested a review from chrjohn January 14, 2026 19:55
Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>
Copilot AI requested a review from chrjohn January 15, 2026 10:44
// Step 5: Disconnect and reconnect
session.disconnect("Simulating disconnect", false);
assertFalse("Session should be disconnected", session.isLoggedOn());

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot the test does not exactly follow the description.
We need a call to Session.generateResendRequest() here, which will in turn trigger sendRaw() which will in turn set the resendRange and show the problematic behaviour.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in commit f910c2f. The test now calls Session.generateResendRequest() directly using reflection after disconnect, which triggers sendResendRequest()sendRaw() (fails) → state.setResendRange() (still called), demonstrating the bug exactly as described.

Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>
Copilot AI requested a review from chrjohn January 16, 2026 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants