fixes: fix the notification lost when failure getStub

[mattisonchao]

Motivation

as same as #243. The notification can also not be recovered if getStub returns an error.

Modification

• Catch the exception of getStub and retry later.

[Copilot]

Pull Request Overview

This PR fixes a notification loss issue where failures in getStub() would prevent notifications from being recovered. The fix follows the same pattern as issue #243 by catching exceptions from getStub() and triggering the retry mechanism.

Key Changes:

• Modified ShardNotificationReceiver constructor to accept OxiaStubManager and leader address instead of a pre-resolved OxiaStub
• Added try-catch block around getStub() call in start() method to handle failures and trigger retry via onError()
• Updated all test cases to use the new constructor signature with OxiaStubManager

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
client/src/main/java/io/oxia/client/notify/ShardNotificationReceiver.java	Refactored to defer stub resolution until start() is called, wrapped getStub() in try-catch to enable retry on failures
client/src/test/java/io/oxia/client/notify/ShardNotificationReceiverTest.java	Updated test setup to mock OxiaStubManager and adjusted all test cases to use new constructor signature

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

client/src/test/java/io/oxia/client/notify/ShardNotificationReceiverTest.java:197

• The test file ShardNotificationReceiverTest lacks a dedicated test case for the new error handling behavior when getStub() throws an exception. While NotificationManagerTest includes an integration test for this scenario, it would be beneficial to add a unit test in ShardNotificationReceiverTest that specifically verifies:

1. When stubManager.getStub() initially throws an exception
2. The receiver properly schedules a retry via onError()
3. After retry, when getStub() succeeds, notifications are received successfully

This would directly test the new try-catch block added in the start() method and ensure the fix for the issue described in the PR is properly covered.

    @Test
    public void recoveryFromError() {
        @Cleanup("shutdownNow")
        ScheduledExecutorService executorService = Executors.newSingleThreadScheduledExecutor();
        when(notificationManager.getExecutor()).thenReturn(executorService);
        when(notificationManager.getCounterNotificationsReceived()).thenReturn(mock(Counter.class));
        when(notificationManager.getCounterNotificationsBatchesReceived())
                .thenReturn(mock(Counter.class));

        responses.offer(new NotificationWrapper(null, Status.UNAVAILABLE.asException(), false));
        var notifications =
                NotificationBatch.newBuilder().putNotifications("key1", created(1L)).build();
        responses.offer(new NotificationWrapper(notifications, null, false));
        try (var notificationReceiver =
                new ShardNotificationReceiver(
                        stubManager,
                        leader,
                        shardId,
                        notificationCallback,
                        notificationManager,
                        OptionalLong.empty())) {
            await()
                    .untilAsserted(
                            () -> {
                                verify(notificationCallback).accept(new KeyCreated("key1", 1L));
                            });
        }
        assertThat(requests).hasValue(2);
    }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.