Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -315,18 +315,20 @@ private static class RetryInterceptor implements Interceptor {
public Response intercept(Chain chain) throws IOException {
Request request = chain.request();
Response response = null;
IOException lastException = null;
boolean responseReturned = false;

try {
for (int attempt = 0; attempt <= maxRetries; attempt++) {
try {
if (response != null) {
response.close();
response = null;
}
response = chain.proceed(request);

// Don't retry on successful responses or client errors (4xx)
if (response.isSuccessful() || response.code() < 500) {
responseReturned = true;
return response;
}

Expand All @@ -337,7 +339,6 @@ public Response intercept(Chain chain) throws IOException {
maxRetries + 1);

} catch (IOException e) {
lastException = e;
logger.warn(
"RAGFlow request failed with exception, attempt {}/{}: {}",
attempt + 1,
Expand All @@ -361,14 +362,12 @@ public Response intercept(Chain chain) throws IOException {
}
}

if (lastException != null) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

删除了这个异常会不会有问题?
我理解删除了就改变了这个拦截器的动作。

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

是的,这里确实改变了一个分支,但改变的是历史异常覆盖最终请求结果的行为(之前bot提出的问题)。
lastException 只可能来自之前的重试;如果最后一次请求仍抛出 IOException,当前代码会在 attempt == maxRetries 的 catch 中直接 抛出异常,因此最终网络异常会正常向上传递。
删除后各终止路径为:

  • 最终成功或 4xx:直接返回响应;
  • 最终 5xx:返回最终响应,交由上层读取响应体并按状态码处理;
  • 最终 IOException:直接抛出该次异常。
    原本的逻辑的问题只发生在“前一次 IOException、最后一次得到 5xx”时:旧的 lastException 会覆盖最后一次 HTTP 响应。新增的 testRetryReturnsFinalErrorResponseAfterEarlierIOException 覆盖了这个场景。
    所以这里不会放宽或吞掉最终异常,而是让最后一次尝试的结果决定最终行为。

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

已补充测试提交 58bca961,生产代码未再修改。

新增 testRetryThrowsFinalIOExceptionAfterEarlierIOException,确定性模拟 IOException -> IOException,并验证:

  • 最终向上传递的异常对象就是最后一次尝试抛出的 finalException
  • chain.proceed(request) 恰好执行 2 次。

验证结果:聚焦测试 1/1、RAGFlowClientTest 39/39、RAGFlow 模块 99/99,Spotless 均通过。这条测试直接确认删除历史 lastException 后,最终传输异常仍会正常抛出;改变的仅是“历史 IOException 覆盖最终 HTTP 5xx 响应”的错误分支。

throw lastException;
}

responseReturned = true;
return response;
} finally {
// Ensure response is closed if we're not returning it successfully
if (response != null && (lastException != null || !response.isSuccessful())) {
// Keep the response open for the caller to consume, including final error
// responses.
if (response != null && !responseReturned) {
response.close();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,29 @@

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import com.fasterxml.jackson.core.type.TypeReference;
import io.agentscope.core.rag.integration.ragflow.exception.RAGFlowApiException;
import io.agentscope.core.rag.integration.ragflow.exception.RAGFlowAuthException;
import io.agentscope.core.rag.integration.ragflow.model.RAGFlowResponse;
import io.agentscope.core.util.JsonUtils;
import java.io.IOException;
import java.lang.reflect.Field;
import java.util.List;
import java.util.Map;
import okhttp3.Interceptor;
import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.Response;
import okhttp3.ResponseBody;
import okhttp3.mockwebserver.MockResponse;
import okhttp3.mockwebserver.MockWebServer;
import okhttp3.mockwebserver.RecordedRequest;
Expand Down Expand Up @@ -742,6 +754,83 @@ void testHttp503ServiceUnavailable() {
assertTrue(exception.getMessage().contains("server error"));
}

@Test
void testRetryKeepsFinalErrorResponseBodyReadable() {
mockWebServer.enqueue(
new MockResponse()
.setResponseCode(500)
.setBody("{\"message\": \"first failure\"}"));
mockWebServer.enqueue(
new MockResponse()
.setResponseCode(500)
.setBody("{\"message\": \"final failure\"}"));

RAGFlowConfig config =
RAGFlowConfig.builder()
.apiKey("test-api-key")
.baseUrl(mockWebServer.url("").toString().replaceAll("/$", ""))
.addDatasetId("dataset-123")
.maxRetries(1)
.build();

RAGFlowClient client = new RAGFlowClient(config);

RAGFlowApiException exception =
assertThrows(
RAGFlowApiException.class,
() -> client.retrieve("test query", null, null, null).block());

assertTrue(exception.getMessage().contains("final failure"));
assertEquals(2, mockWebServer.getRequestCount());
}

@Test
void testRetryReturnsFinalErrorResponseAfterEarlierIOException() throws Exception {
RAGFlowConfig config =
RAGFlowConfig.builder()
.apiKey("test-api-key")
.baseUrl(mockWebServer.url("").toString().replaceAll("/$", ""))
.addDatasetId("dataset-123")
.maxRetries(1)
.build();
RAGFlowClient client = new RAGFlowClient(config);

Field httpClientField = RAGFlowClient.class.getDeclaredField("httpClient");
httpClientField.setAccessible(true);
OkHttpClient httpClient = (OkHttpClient) httpClientField.get(client);
Interceptor retryInterceptor =
httpClient.interceptors().stream()
.filter(
interceptor ->
interceptor
.getClass()
.getSimpleName()
.equals("RetryInterceptor"))
.findFirst()
.orElseThrow();

Interceptor.Chain chain = mock(Interceptor.Chain.class);
Request request = new Request.Builder().url(mockWebServer.url("/api/v1/retrieval")).build();
ResponseBody responseBody = mock(ResponseBody.class);
Response finalErrorResponse = mock(Response.class);

when(chain.request()).thenReturn(request);
when(chain.proceed(request))
.thenThrow(new IOException("first attempt failed"))
.thenReturn(finalErrorResponse);
when(finalErrorResponse.isSuccessful()).thenReturn(false);
when(finalErrorResponse.code()).thenReturn(500);
when(finalErrorResponse.body()).thenReturn(responseBody);
when(responseBody.string()).thenReturn("{\"message\": \"final failure\"}");

Response returned = retryInterceptor.intercept(chain);

assertSame(finalErrorResponse, returned);
assertEquals("{\"message\": \"final failure\"}", returned.body().string());
verify(chain, times(2)).proceed(request);
verify(finalErrorResponse, never()).close();
}

@Test
void testApiErrorWithNonZeroCode() {
String errorResponse =
Expand Down
Loading