diff --git a/ambry-rest/src/main/java/com/github/ambry/rest/NettyResponseChannel.java b/ambry-rest/src/main/java/com/github/ambry/rest/NettyResponseChannel.java index c6d41bad63..be118b8419 100644 --- a/ambry-rest/src/main/java/com/github/ambry/rest/NettyResponseChannel.java +++ b/ambry-rest/src/main/java/com/github/ambry/rest/NettyResponseChannel.java @@ -589,9 +589,12 @@ private FullHttpResponse getErrorResponse(Throwable cause) { errorResponseStatus = ResponseStatus.getResponseStatus(restServiceErrorCode); status = getHttpResponseStatus(errorResponseStatus); if (shouldSendFailureReason(status, restServiceException)) { - errReason = new String( - Utils.getRootCause(cause).getMessage().replaceAll("[\n\t\r]", " ").getBytes(StandardCharsets.US_ASCII), - StandardCharsets.US_ASCII); + String rootMessage = Utils.getRootCause(cause).getMessage(); + if (rootMessage != null) { + errReason = new String( + rootMessage.replaceAll("[\n\t\r]", " ").getBytes(StandardCharsets.US_ASCII), + StandardCharsets.US_ASCII); + } } if (restServiceException.shouldIncludeExceptionMetadataInResponse()) { errHeaders = restServiceException.getExceptionHeadersMap(); diff --git a/ambry-rest/src/test/java/com/github/ambry/rest/NettyResponseChannelTest.java b/ambry-rest/src/test/java/com/github/ambry/rest/NettyResponseChannelTest.java index 24c42699d1..a2fe47d958 100644 --- a/ambry-rest/src/test/java/com/github/ambry/rest/NettyResponseChannelTest.java +++ b/ambry-rest/src/test/java/com/github/ambry/rest/NettyResponseChannelTest.java @@ -462,6 +462,27 @@ public void setFailureReasonInResponseTest() throws Exception { assertFalse("Channel not closed on the server", channel.isActive()); } + /** + * Tests that no NPE occurs when the root cause of a RestServiceException has a null message (e.g., + * {@link java.util.concurrent.TimeoutException} created by {@link java.util.concurrent.CompletableFuture#orTimeout}). + */ + @Test + public void setFailureReasonNullMessageNoNpeTest() throws Exception { + HttpRequest request = + createRequestWithHeaders(HttpMethod.GET, TestingUri.SetFailureReasonInResponseWithNullMessage.toString()); + HttpUtil.setKeepAlive(request, false); + + EmbeddedChannel channel = createEmbeddedChannel(); + channel.writeInbound(request); + + HttpResponse response = channel.readOutbound(); + assertEquals(HttpResponseStatus.SERVICE_UNAVAILABLE, response.status()); + // No FAILURE_REASON_HEADER when root cause message is null — no NPE should be thrown + assertFalse("Should not have failure reason header when root message is null", + response.headers().contains(NettyResponseChannel.FAILURE_REASON_HEADER)); + assertFalse("Channel not closed on the server", channel.isActive()); + } + /** * Sends null input to {@link NettyResponseChannel#setHeader(String, Object)} (through * {@link MockNettyMessageProcessor}) and tests for reaction. @@ -1339,6 +1360,11 @@ enum TestingUri { * to the client. */ SetFailureReasonInResponseWithException, + /** + * When this request is received, a RestServiceException whose root cause has a null message is used on + * onResponseComplete. Verifies no NPE when the root cause message is null. + */ + SetFailureReasonInResponseWithNullMessage, /** * Catch all TestingUri. */ @@ -1584,6 +1610,12 @@ private void handleRequest(HttpRequest httpRequest) throws Exception { request.setArg(RestUtils.InternalKeys.SEND_FAILURE_REASON, Boolean.TRUE); restResponseChannel.onResponseComplete(new Exception(TestingUri.SetFailureReasonInResponse.toString())); break; + case SetFailureReasonInResponseWithNullMessage: + request.setArg(RestUtils.InternalKeys.SEND_FAILURE_REASON, Boolean.TRUE); + // TimeoutException() with no message simulates CompletableFuture.orTimeout() behavior + restResponseChannel.onResponseComplete(new RestServiceException("outer message", + new TimeoutException(), RestServiceErrorCode.ServiceUnavailable)); + break; } }