test: Update JsonRpcExecutorHandlerTest for timeout handling and 408 …

…status

- Add verification for 408 (Request Timeout) status code on timeout
- Add verification for timer cancellation in timeout scenario
- Update successful execution test to verify timer cancellation
- Mock timerId retrieval from context

This change ensures that a 408 status code is returned on timeout and
that timer cancellation is properly tested in both timeout and successful
execution scenarios, improving the test coverage and error handling for
the JsonRpcExecutorHandler.

Issue: hyperledger#5589
PR: hyperledger#7469
Signed-off-by: Ade Lucas <ade.lucas@consensys.net>

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/handlers/AbstractJsonRpcExecutor.java

@@ -106,6 +106,7 @@ protected static void handleJsonRpcError(
   private static HttpResponseStatus statusCodeFromError(final RpcErrorType error) {
     return switch (error) {
       case INVALID_REQUEST, PARSE_ERROR -> HttpResponseStatus.BAD_REQUEST;
+      case TIMEOUT_ERROR -> HttpResponseStatus.REQUEST_TIMEOUT;
       default -> HttpResponseStatus.OK;
     };
   }

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/handlers/JsonRpcExecutorHandler.java

@@ -35,7 +35,7 @@ public class JsonRpcExecutorHandler {
   private static final Logger LOG = LoggerFactory.getLogger(JsonRpcExecutorHandler.class);
 
   // Default timeout for RPC calls in seconds
-  private static final long DEFAULT_TIMEOUT_MILLISECONDS = 5000;
+  private static final long DEFAULT_TIMEOUT_MILLISECONDS = 30000;
 
   private JsonRpcExecutorHandler() {}
 
@@ -60,10 +60,7 @@ public static Handler<RoutingContext> handler(
                     final String method =
                         ctx.get(ContextKey.REQUEST_BODY_AS_JSON_OBJECT.name()).toString();
                     LOG.error("Timeout occurred in JSON-RPC executor for method {}", method);
-                    if (!ctx.response().ended()) {
-                      handleJsonRpcError(ctx, null, RpcErrorType.TIMEOUT_ERROR);
-                      ctx.response().close();
-                    }
+                    handleErrorAndEndResponse(ctx, null, RpcErrorType.TIMEOUT_ERROR);
                   });
 
       ctx.put("timerId", timerId);
@@ -74,29 +71,23 @@ public static Handler<RoutingContext> handler(
                 executor -> {
                   try {
                     executor.execute();
-                    cancelTimer(ctx);
                   } catch (IOException e) {
                     final String method = executor.getRpcMethodName(ctx);
                     LOG.error("{} - Error streaming JSON-RPC response", method, e);
-                    if (!ctx.response().ended()) {
-                      handleJsonRpcError(ctx, null, RpcErrorType.INTERNAL_ERROR);
-                      ctx.response().close();
-                    }
+                    handleErrorAndEndResponse(ctx, null, RpcErrorType.INTERNAL_ERROR);
+                  } finally {
+                    cancelTimer(ctx);
                   }
                 },
                 () -> {
-                  if (!ctx.response().ended()) {
-                    handleJsonRpcError(ctx, null, RpcErrorType.PARSE_ERROR);
-                    ctx.response().close();
-                  }
+                  handleErrorAndEndResponse(ctx, null, RpcErrorType.PARSE_ERROR);
+                  cancelTimer(ctx);
                 });
       } catch (final RuntimeException e) {
         final String method = ctx.get(ContextKey.REQUEST_BODY_AS_JSON_OBJECT.name()).toString();
         LOG.error("Unhandled exception in JSON-RPC executor for method {}", method, e);
-        if (!ctx.response().ended()) {
-          handleJsonRpcError(ctx, null, RpcErrorType.INTERNAL_ERROR);
-          ctx.response().close();
-        }
+        handleErrorAndEndResponse(ctx, null, RpcErrorType.INTERNAL_ERROR);
+        cancelTimer(ctx);
       }
     };
   }
@@ -108,6 +99,12 @@ private static void cancelTimer(final RoutingContext ctx) {
     }
   }
 
+  private static void handleErrorAndEndResponse(final RoutingContext ctx, final Object id, final RpcErrorType errorType) {
+    if (!ctx.response().ended()) {
+      handleJsonRpcError(ctx, id, errorType);
+    }
+  }
+
   private static Optional<AbstractJsonRpcExecutor> createExecutor(
       final JsonRpcExecutor jsonRpcExecutor,
       final Tracer tracer,

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/handlers/JsonRpcExecutorHandlerTest.java

@@ -0,0 +1,94 @@
+package org.hyperledger.besu.ethereum.api.handlers;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.ArgumentMatchers.contains;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import io.netty.handler.codec.http.HttpResponseStatus;
+import io.vertx.core.Handler;
+import io.vertx.core.Vertx;
+import io.vertx.core.http.HttpServerResponse;
+import io.vertx.ext.web.RoutingContext;
+import org.hyperledger.besu.ethereum.api.jsonrpc.execution.JsonRpcExecutor;
+import org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcConfiguration;
+import org.hyperledger.besu.ethereum.api.jsonrpc.context.ContextKey;
+import io.opentelemetry.api.trace.Tracer;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
+
+import java.io.IOException;
+
+class JsonRpcExecutorHandlerTest {
+
+    private JsonRpcExecutor mockExecutor;
+    private Tracer mockTracer;
+    private JsonRpcConfiguration mockConfig;
+    private RoutingContext mockContext;
+    private Vertx mockVertx;
+    private HttpServerResponse mockResponse;
+
+    @BeforeEach
+    void setUp() {
+        mockExecutor = mock(JsonRpcExecutor.class);
+        mockTracer = mock(Tracer.class);
+        mockConfig = mock(JsonRpcConfiguration.class);
+        mockContext = mock(RoutingContext.class);
+        mockVertx = mock(Vertx.class);
+        mockResponse = mock(HttpServerResponse.class);
+
+        when(mockContext.vertx()).thenReturn(mockVertx);
+        when(mockContext.response()).thenReturn(mockResponse);
+        when(mockResponse.ended()).thenReturn(false);
+        when(mockResponse.setStatusCode(anyInt())).thenReturn(mockResponse);
+    }
+
+    @Test
+    void testTimeoutHandling() {
+        // Arrange
+        Handler<RoutingContext> handler = JsonRpcExecutorHandler.handler(mockExecutor, mockTracer, mockConfig);
+        ArgumentCaptor<Long> delayCaptor = ArgumentCaptor.forClass(Long.class);
+        @SuppressWarnings("unchecked")
+        ArgumentCaptor<Handler<Long>> timerHandlerCaptor = ArgumentCaptor.forClass(Handler.class);
+
+        when(mockContext.get(eq(ContextKey.REQUEST_BODY_AS_JSON_OBJECT.name()))).thenReturn("{}");
+        when(mockVertx.setTimer(delayCaptor.capture(), timerHandlerCaptor.capture())).thenReturn(1L);
+        when(mockContext.get("timerId")).thenReturn(1L);
+
+        // Act
+        handler.handle(mockContext);
+
+        // Assert
+        verify(mockVertx).setTimer(eq(30000L), any());
+
+        // Simulate timeout
+        timerHandlerCaptor.getValue().handle(1L);
+
+        // Verify timeout handling
+        verify(mockResponse, times(1)).setStatusCode(eq(HttpResponseStatus.REQUEST_TIMEOUT.code())); // Expect 408 Request Timeout
+        verify(mockResponse, times(1)).end(contains("Timeout expired"));
+        verify(mockVertx, times(1)).cancelTimer(1L);
+    }
+
+    @Test
+    void testCancelTimerOnSuccessfulExecution() throws IOException {
+        // Arrange
+        Handler<RoutingContext> handler = JsonRpcExecutorHandler.handler(mockExecutor, mockTracer, mockConfig);
+        when(mockContext.get(eq(ContextKey.REQUEST_BODY_AS_JSON_OBJECT.name()))).thenReturn("{}");
+        when(mockVertx.setTimer(anyLong(), any())).thenReturn(1L);
+        when(mockContext.get("timerId")).thenReturn(1L);
+
+        // Act
+        handler.handle(mockContext);
+
+        // Assert
+        verify(mockVertx).setTimer(anyLong(), any());
+        verify(mockVertx).cancelTimer(1L);
+    }
+}