This is an automated email from the ASF dual-hosted git repository.
Aias00 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shenyu.git
The following commit(s) were added to refs/heads/master by this push:
new 11bf955902 fix(register-client-http): propagate failed admin
responses. (#7555)
11bf955902 is described below
commit 11bf955902db8bd59192af1280d5daef19f8595b
Author: redyyang <[email protected]>
AuthorDate: Mon Oct 12 13:14:18 2026 +0800
fix(register-client-http): propagate failed admin responses. (#7555)
Co-authored-by: redyyang <[email protected]>
---
RELEASE-NOTES.md | 8 ++
.../register/client/http/utils/RegisterUtils.java | 10 ++
.../http/HttpClientRegisterRepositoryTest.java | 124 +++++++++++++++++++++
.../client/http/utils/RegisterUtilsTest.java | 57 ++++++++--
4 files changed, 191 insertions(+), 8 deletions(-)
diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md
index acb4c64256..2278e5f5f3 100644
--- a/RELEASE-NOTES.md
+++ b/RELEASE-NOTES.md
@@ -6,6 +6,14 @@
### Behavior Changes
+- HTTP client registration now treats non-success Admin responses as failures,
+ using the same bounded failback policy as transport errors (18 retry attempts
+ per pending task). The text response contract does not reliably distinguish
+ transient failures from permanent validation errors, so both consume that
+ budget. Heartbeat failures are reported to the caller. Unregistration retains
+ its existing policy: any successful Admin response is sufficient, and failure
+ at every Admin is reported without adding a registration retry. (#7440)
+
- Agent Gateway supports opt-in per-tool `mcp.structuredContentRemovals` field
deletion with strict result-copy handling. Complete JSON text copies are
rebuilt
from filtered structuredContent. Opaque text/media, nonempty metadata and
unknown
diff --git
a/shenyu-register-center/shenyu-register-client/shenyu-register-client-http/src/main/java/org/apache/shenyu/register/client/http/utils/RegisterUtils.java
b/shenyu-register-center/shenyu-register-client/shenyu-register-client-http/src/main/java/org/apache/shenyu/register/client/http/utils/RegisterUtils.java
index 4b2afbfb6a..b27e125207 100644
---
a/shenyu-register-center/shenyu-register-client/shenyu-register-client-http/src/main/java/org/apache/shenyu/register/client/http/utils/RegisterUtils.java
+++
b/shenyu-register-center/shenyu-register-client/shenyu-register-client-http/src/main/java/org/apache/shenyu/register/client/http/utils/RegisterUtils.java
@@ -35,6 +35,12 @@ import static
org.apache.shenyu.common.constant.Constants.SUCCESS;
/**
* RegisterUtils.
+ *
+ * <p>Registration, heartbeat and unregistration responses other than {@link
Constants#SUCCESS}
+ * propagate through the existing {@link IOException} contract. Registration
failures use the
+ * repository's bounded failback policy, including validation-error responses:
the text response
+ * contract cannot reliably distinguish permanent errors from transient
failures. Heartbeat and
+ * unregistration failures are handled by the caller without enqueueing a
registration retry.
*/
public final class RegisterUtils {
@@ -63,6 +69,7 @@ public final class RegisterUtils {
LOGGER.info("{} client register success: {} ", type, json);
} else {
LOGGER.error("{} client register error: {} ", type, json);
+ throw new IOException("Admin returned a non-success response for "
+ type + " registration at " + url);
}
}
@@ -80,6 +87,7 @@ public final class RegisterUtils {
LOGGER.info("{} client register success: {} ", type, json);
} else {
LOGGER.error("{} client register error: {} ", type, json);
+ throw new IOException("Admin returned a non-success response for "
+ type + " registration at " + url);
}
}
@@ -103,6 +111,7 @@ public final class RegisterUtils {
LOGGER.info("{} success: {} ", type, json);
} else {
LOGGER.error("{} error: {} ", type, json);
+ throw new IOException("Admin returned a non-success response for "
+ type + " at " + url);
}
}
@@ -125,6 +134,7 @@ public final class RegisterUtils {
LOGGER.info("{} client unregister success: {} ", Constants.URI,
json);
} else {
LOGGER.error("{} client unregister error: {} ", Constants.URI,
json);
+ throw new IOException("Admin returned a non-success response for
unregistration at " + url);
}
}
diff --git
a/shenyu-register-center/shenyu-register-client/shenyu-register-client-http/src/test/java/org/apache/shenyu/register/client/http/HttpClientRegisterRepositoryTest.java
b/shenyu-register-center/shenyu-register-client/shenyu-register-client-http/src/test/java/org/apache/shenyu/register/client/http/HttpClientRegisterRepositoryTest.java
index ce7d8e6fe4..eb0dfb23e7 100644
---
a/shenyu-register-center/shenyu-register-client/shenyu-register-client-http/src/test/java/org/apache/shenyu/register/client/http/HttpClientRegisterRepositoryTest.java
+++
b/shenyu-register-center/shenyu-register-client/shenyu-register-client-http/src/test/java/org/apache/shenyu/register/client/http/HttpClientRegisterRepositoryTest.java
@@ -17,8 +17,16 @@
package org.apache.shenyu.register.client.http;
+import okhttp3.Headers;
import org.apache.shenyu.common.constant.Constants;
+import org.apache.shenyu.common.timer.TaskEntity;
+import org.apache.shenyu.common.timer.Timer;
+import org.apache.shenyu.common.timer.TimerTask;
+import org.apache.shenyu.common.timer.WheelTimerFactory;
import org.apache.shenyu.common.utils.GsonUtils;
+import org.apache.shenyu.register.client.api.FailbackRegistryRepository;
+import org.apache.shenyu.register.client.api.retry.FailureRegistryTask;
+import org.apache.shenyu.register.client.http.utils.OkHttpTools;
import org.apache.shenyu.register.client.http.utils.RegisterUtils;
import org.apache.shenyu.register.client.http.utils.RuntimeUtils;
import org.apache.shenyu.register.common.config.ShenyuRegisterCenterConfig;
@@ -33,22 +41,31 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
+import org.mockito.ArgumentCaptor;
import org.mockito.MockedStatic;
import java.io.IOException;
+import java.lang.reflect.Field;
+import java.util.Map;
import java.util.Optional;
import java.util.Properties;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.when;
/**
* Test cases for {@link HttpClientRegisterRepository}.
@@ -375,6 +392,113 @@ public final class HttpClientRegisterRepositoryTest {
}
}
+ @ParameterizedTest
+ @ValueSource(strings = {FIRST_SERVER, SECOND_SERVER})
+ public void responseFailureShouldScheduleRetryAndRetainOtherFailures(final
String failedServer) throws IOException {
+ final Timer timer = mock(Timer.class);
+ final OkHttpTools http = mock(OkHttpTools.class);
+ try (MockedStatic<WheelTimerFactory> timers =
mockStatic(WheelTimerFactory.class);
+ MockedStatic<OkHttpTools> clients =
mockStatic(OkHttpTools.class);
+ MockedStatic<RuntimeUtils> runtime =
mockStatic(RuntimeUtils.class)) {
+ timers.when(WheelTimerFactory::getSharedTimer).thenReturn(timer);
+ clients.when(OkHttpTools::getInstance).thenReturn(http);
+ runtime.when(() ->
RuntimeUtils.listenByOther(anyInt())).thenReturn(false);
+ when(http.post(anyString(),
anyString())).thenReturn("{\"code\":200,\"data\":{\"token\":\"test-token\"}}");
+ when(http.post(anyString(), anyString(),
any(Headers.class))).thenReturn(Constants.SUCCESS);
+ when(http.post(eq(failedServer + Constants.URI_PATH), anyString(),
any(Headers.class)))
+ .thenReturn("failure", "failure", Constants.SUCCESS);
+ final HttpClientRegisterRepository actual = new
HttpClientRegisterRepository(config(FIRST_SERVER + "," + SECOND_SERVER));
+
+ actual.persistURI(uriRegisterDTO());
+
+ final Map<String, ?> failures = failureMap(actual);
+ assertEquals(1, failures.size());
+ final String uriKey = failures.keySet().iterator().next();
+ ArgumentCaptor<TimerTask> scheduled =
ArgumentCaptor.forClass(TimerTask.class);
+ verify(timer).add(scheduled.capture());
+ final FailureRegistryTask task = (FailureRegistryTask)
scheduled.getValue();
+
+ when(http.post(eq(FIRST_SERVER + Constants.META_PATH),
anyString(), any(Headers.class))).thenReturn("failure");
+ actual.persistInterface(metaDataRegisterDTO());
+ assertEquals(2, failures.size());
+ TaskEntity entity = mock(TaskEntity.class);
+ when(entity.getTimer()).thenReturn(timer);
+ when(entity.getTimerTask()).thenReturn(task);
+
+ task.run(entity);
+ assertEquals(2, failures.size());
+ assertTrue(failures.containsKey(uriKey));
+ task.run(entity);
+ assertEquals(1, failures.size());
+ assertFalse(failures.containsKey(uriKey));
+ for (String server : new String[]{FIRST_SERVER, SECOND_SERVER}) {
+ verify(http, times(3)).post(eq(server + Constants.URI_PATH),
anyString(), any(Headers.class));
+ }
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {FIRST_SERVER, SECOND_SERVER})
+ public void
responseHeartbeatFailureShouldPropagateWithoutRegisterRetry(final String
failedServer) throws IOException {
+ final Timer timer = mock(Timer.class);
+ final OkHttpTools http = mock(OkHttpTools.class);
+ try (MockedStatic<WheelTimerFactory> timers =
mockStatic(WheelTimerFactory.class);
+ MockedStatic<OkHttpTools> clients =
mockStatic(OkHttpTools.class);
+ MockedStatic<RuntimeUtils> runtime =
mockStatic(RuntimeUtils.class)) {
+ timers.when(WheelTimerFactory::getSharedTimer).thenReturn(timer);
+ clients.when(OkHttpTools::getInstance).thenReturn(http);
+ runtime.when(() ->
RuntimeUtils.listenByOther(anyInt())).thenReturn(false);
+ when(http.post(anyString(),
anyString())).thenReturn("{\"code\":200,\"data\":{\"token\":\"test-token\"}}");
+ when(http.post(anyString(), anyString(),
any(Headers.class))).thenReturn(Constants.SUCCESS);
+ when(http.post(eq(failedServer + Constants.URI_PATH), anyString(),
any(Headers.class))).thenReturn("failure");
+ HttpClientRegisterRepository actual = new
HttpClientRegisterRepository(config(FIRST_SERVER + "," + SECOND_SERVER));
+
+ RuntimeException failure = assertThrows(RuntimeException.class, ()
-> actual.sendHeartbeat(uriRegisterDTO()));
+
+ assertTrue(failure.getCause() instanceof IOException);
+ assertTrue(failureMap(actual).isEmpty());
+ verifyNoInteractions(timer);
+ for (String server : new String[]{FIRST_SERVER, SECOND_SERVER}) {
+ verify(http).post(eq(server + Constants.URI_PATH),
anyString(), any(Headers.class));
+ }
+ }
+ }
+
+ @Test
+ public void responseUnregisterFailureShouldKeepAnyServerSuccessPolicy()
throws IOException {
+ final Timer timer = mock(Timer.class);
+ final OkHttpTools http = mock(OkHttpTools.class);
+ try (MockedStatic<WheelTimerFactory> timers =
mockStatic(WheelTimerFactory.class);
+ MockedStatic<OkHttpTools> clients =
mockStatic(OkHttpTools.class)) {
+ timers.when(WheelTimerFactory::getSharedTimer).thenReturn(timer);
+ clients.when(OkHttpTools::getInstance).thenReturn(http);
+ when(http.post(anyString(),
anyString())).thenReturn("{\"code\":200,\"data\":{\"token\":\"test-token\"}}");
+ when(http.post(anyString(), anyString(),
any(Headers.class))).thenReturn(Constants.SUCCESS);
+ when(http.post(eq(FIRST_SERVER + Constants.OFFLINE_PATH),
anyString(), any(Headers.class))).thenReturn("failure");
+ HttpClientRegisterRepository actual = new
HttpClientRegisterRepository(config(FIRST_SERVER + "," + SECOND_SERVER));
+
+ assertDoesNotThrow(() -> actual.offline(uriRegisterDTO()));
+ when(http.post(eq(SECOND_SERVER + Constants.OFFLINE_PATH),
anyString(), any(Headers.class))).thenReturn("failure");
+ assertThrows(RuntimeException.class, () ->
actual.offline(uriRegisterDTO()));
+ assertTrue(failureMap(actual).isEmpty());
+ verifyNoInteractions(timer);
+ for (String server : new String[]{FIRST_SERVER, SECOND_SERVER}) {
+ verify(http, times(2)).post(eq(server +
Constants.OFFLINE_PATH), anyString(), any(Headers.class));
+ }
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ private Map<String, ?> failureMap(final HttpClientRegisterRepository
actual) {
+ try {
+ Field field =
FailbackRegistryRepository.class.getDeclaredField("concurrentHashMap");
+ field.setAccessible(true);
+ return (Map<String, ?>) field.get(actual);
+ } catch (ReflectiveOperationException e) {
+ throw new IllegalStateException(e);
+ }
+ }
+
private ShenyuRegisterCenterConfig config(final String serverLists) {
Properties props = new Properties();
props.setProperty(Constants.USER_NAME, "admin");
diff --git
a/shenyu-register-center/shenyu-register-client/shenyu-register-client-http/src/test/java/org/apache/shenyu/register/client/http/utils/RegisterUtilsTest.java
b/shenyu-register-center/shenyu-register-client/shenyu-register-client-http/src/test/java/org/apache/shenyu/register/client/http/utils/RegisterUtilsTest.java
index 151f0ac6f9..bc3e047835 100644
---
a/shenyu-register-center/shenyu-register-client/shenyu-register-client-http/src/test/java/org/apache/shenyu/register/client/http/utils/RegisterUtilsTest.java
+++
b/shenyu-register-center/shenyu-register-client/shenyu-register-client-http/src/test/java/org/apache/shenyu/register/client/http/utils/RegisterUtilsTest.java
@@ -25,6 +25,9 @@ import
org.apache.shenyu.register.common.enums.RegisterTypeEnum;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.NullAndEmptySource;
+import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.MockedStatic;
import java.io.IOException;
@@ -84,26 +87,64 @@ public final class RegisterUtilsTest {
RegisterUtils.doRegister(json, url,
RegisterTypeEnum.DUBBO.getName());
verify(okHttpTools, times(1)).post(eq(url), eq(json));
RegisterUtils.doRegister(json, url,
RegisterTypeEnum.DUBBO.getName(), accessToken);
- verify(okHttpTools, times(1)).post(eq(url), eq(json));
+ verify(okHttpTools, times(1)).post(eq(url), eq(json), eq(headers));
}
}
- @Test
- public void testDoRegisterWhenError() throws IOException {
- when(okHttpTools.post(url, json)).thenReturn("Error parameter！");
+ @ParameterizedTest
+ @NullAndEmptySource
+ @ValueSource(strings = {"Error parameter！",
"{\"code\":400,\"message\":\"invalid parameters\"}"})
+ public void testDoRegisterWhenError(final String response) throws
IOException {
+ when(okHttpTools.post(url, json)).thenReturn(response);
Headers headers = new Headers.Builder().add(Constants.X_ACCESS_TOKEN,
accessToken).build();
- when(okHttpTools.post(url, json, headers)).thenReturn("Error
parameter！");
+ when(okHttpTools.post(url, json, headers)).thenReturn(response);
try (MockedStatic<OkHttpTools> okHttpToolsMockedStatic =
mockStatic(OkHttpTools.class)) {
okHttpToolsMockedStatic.when(OkHttpTools::getInstance).thenReturn(okHttpTools);
- RegisterUtils.doRegister(json, url,
RegisterTypeEnum.DUBBO.getName());
- verify(okHttpTools, times(1)).post(eq(url), eq(json));
- RegisterUtils.doRegister(json, url,
RegisterTypeEnum.DUBBO.getName(), accessToken);
+ assertThrows(IOException.class, () ->
RegisterUtils.doRegister(json, url, RegisterTypeEnum.DUBBO.getName()));
verify(okHttpTools, times(1)).post(eq(url), eq(json));
+ assertThrows(IOException.class, () ->
RegisterUtils.doRegister(json, url, RegisterTypeEnum.DUBBO.getName(),
accessToken));
+ verify(okHttpTools, times(1)).post(eq(url), eq(json), eq(headers));
RegisterUtils.doRegister(json, url,
RegisterTypeEnum.DUBBO.getName(), null);
}
}
+ @ParameterizedTest
+ @NullAndEmptySource
+ @ValueSource(strings = {"failure", "{\"code\":400,\"message\":\"invalid
parameters\"}"})
+ public void testHeartbeatRejectsNonSuccessResponse(final String response)
throws IOException {
+ Headers headers = new Headers.Builder().add(Constants.X_ACCESS_TOKEN,
accessToken).build();
+ when(okHttpTools.post(url, json, headers)).thenReturn(response);
+ try (MockedStatic<OkHttpTools> mocked = mockStatic(OkHttpTools.class))
{
+ mocked.when(OkHttpTools::getInstance).thenReturn(okHttpTools);
+ assertThrows(IOException.class, () ->
RegisterUtils.doHeartBeat(json, url, Constants.HEARTBEAT, accessToken));
+ }
+ }
+
+ @ParameterizedTest
+ @NullAndEmptySource
+ @ValueSource(strings = {"failure", "{\"code\":400,\"message\":\"invalid
parameters\"}"})
+ public void testUnregisterRejectsNonSuccessResponse(final String response)
throws IOException {
+ Headers headers = new Headers.Builder().add(Constants.X_ACCESS_TOKEN,
accessToken).build();
+ when(okHttpTools.post(url, json, headers)).thenReturn(response);
+ try (MockedStatic<OkHttpTools> mocked = mockStatic(OkHttpTools.class))
{
+ mocked.when(OkHttpTools::getInstance).thenReturn(okHttpTools);
+ assertThrows(IOException.class, () ->
RegisterUtils.doUnregister(json, url, accessToken));
+ }
+ }
+
+ @Test
+ public void testHeartbeatAndUnregisterAcceptSuccess() throws IOException {
+ Headers headers = new Headers.Builder().add(Constants.X_ACCESS_TOKEN,
accessToken).build();
+ when(okHttpTools.post(url, json,
headers)).thenReturn(Constants.SUCCESS);
+ try (MockedStatic<OkHttpTools> mocked = mockStatic(OkHttpTools.class))
{
+ mocked.when(OkHttpTools::getInstance).thenReturn(okHttpTools);
+ RegisterUtils.doHeartBeat(json, url, Constants.HEARTBEAT,
accessToken);
+ RegisterUtils.doUnregister(json, url, accessToken);
+ verify(okHttpTools, times(2)).post(url, json, headers);
+ }
+ }
+
@Test
public void testDoRegisterWhenThrowException() throws IOException {
when(okHttpTools.post(url, json)).thenThrow(IOException.class);
