This is an automated email from the ASF dual-hosted git repository.
jmclean pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 9b40272aef issue/10268: Handled configs servlet write failures with
explicit HTT… (#10300)
9b40272aef is described below
commit 9b40272aeff9e93c44f64ec48f8061b66eb3aeff
Author: Tushar Singh <[email protected]>
AuthorDate: Wed Apr 8 10:19:43 2026 +0530
issue/10268: Handled configs servlet write failures with explicit HTT…
(#10300)
### What changes were proposed in this pull request?
Handle /configs servlet write failures with explicit HTTP 500 by setting
HttpServletResponse.SC_INTERNAL_SERVER_ERROR and returning JSON error
body.
### Why are the changes needed?
It's an improvement for proper error handling because without setting a
status code, the response defaults to 200 (success), which misleads the
client into thinking the operation succeeded when it actually failed.
Fix: Set status as HttpServletResponse.SC_INTERNAL_SERVER_ERROR which
represents 500 Internal server error so that client can properly
understand the response outcome.
### Does this PR introduce _any_ user-facing change?
Whenever exception occurs in doGet() it will return proper error code
the client.
### How was this patch tested?
<img width="1408" height="614" alt="image"
src="https://github.com/user-attachments/assets/48ee843a-0531-4216-8aae-4842b168d07c";
/>
<img width="1472" height="624" alt="image"
src="https://github.com/user-attachments/assets/26fd1aca-ef6a-4a6b-b81d-c4a02fa7fe87";
/>
---
.../apache/gravitino/server/web/ConfigServlet.java | 24 ++++++++++++++---
.../gravitino/server/web/TestConfigServlet.java | 31 ++++++++++++++++++++++
2 files changed, 52 insertions(+), 3 deletions(-)
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/ConfigServlet.java
b/server/src/main/java/org/apache/gravitino/server/web/ConfigServlet.java
index 3b48cc4295..a567a0ddcf 100644
--- a/server/src/main/java/org/apache/gravitino/server/web/ConfigServlet.java
+++ b/server/src/main/java/org/apache/gravitino/server/web/ConfigServlet.java
@@ -92,11 +92,29 @@ public class ConfigServlet extends HttpServlet {
res.setContentType("application/json;charset=utf-8");
writer.write(ObjectMapperProvider.objectMapper().writeValueAsString(configs));
} catch (IllegalStateException exception) {
- LOG.error("Illegal state occurred when calling getWriter()");
+ LOG.error("Illegal state occurred when calling getWriter()", exception);
+ res.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+ sendErrorResponse(res, "Failed to get response writer");
} catch (IOException exception) {
- LOG.error("Failed to perform IO");
+ LOG.error("Failed to perform IO", exception);
+ res.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+ sendErrorResponse(res, "IO error occurred");
} catch (Exception e) {
- LOG.error(e.getMessage());
+ LOG.error("Unexpected error: {}", e.getMessage(), e);
+ res.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+ sendErrorResponse(res, "Internal server error");
+ }
+ }
+
+ private void sendErrorResponse(HttpServletResponse res, String message) {
+ try (PrintWriter writer = res.getWriter()) {
+ res.setContentType("application/json;charset=utf-8");
+ Map<String, String> error = Map.of("error", message);
+
writer.write(ObjectMapperProvider.objectMapper().writeValueAsString(error));
+ } catch (IOException e) {
+ LOG.error("Failed to send error response", e);
+ } catch (IllegalStateException e) {
+ LOG.error("Failed to send error response: illegal state", e);
}
}
}
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/TestConfigServlet.java
b/server/src/test/java/org/apache/gravitino/server/web/TestConfigServlet.java
index d165dc2b42..24f3ac5122 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/TestConfigServlet.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/TestConfigServlet.java
@@ -19,11 +19,14 @@
package org.apache.gravitino.server.web;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import com.google.common.collect.Lists;
+import java.io.IOException;
import java.io.PrintWriter;
import javax.servlet.http.HttpServletResponse;
import org.apache.gravitino.Configs;
@@ -126,4 +129,32 @@ public class TestConfigServlet {
assertDoesNotThrow(() -> new ConfigServlet(serverConfig));
}
+
+ @Test
+ public void testConfigServletHandlesIOException() throws Exception {
+ ServerConfig serverConfig = new ServerConfig();
+ ConfigServlet configServlet = new ConfigServlet(serverConfig);
+ configServlet.init();
+ HttpServletResponse res = mock(HttpServletResponse.class);
+ PrintWriter writer = mock(PrintWriter.class);
+ when(res.getWriter()).thenReturn(writer);
+ doThrow(new IOException("Test IO
error")).when(writer).write(any(String.class));
+
+ assertDoesNotThrow(() -> configServlet.doGet(null, res));
+ verify(res).setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+ configServlet.destroy();
+ }
+
+ @Test
+ public void testConfigServletHandlesIllegalStateException() throws Exception
{
+ ServerConfig serverConfig = new ServerConfig();
+ ConfigServlet configServlet = new ConfigServlet(serverConfig);
+ configServlet.init();
+ HttpServletResponse res = mock(HttpServletResponse.class);
+ when(res.getWriter()).thenThrow(new IllegalStateException("Test state
error"));
+
+ assertDoesNotThrow(() -> configServlet.doGet(null, res));
+ verify(res).setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+ configServlet.destroy();
+ }
}
