fix(core): honour HttpException status in the default exception handler
HttpException carries the status the caller meant, and its own javadoc says extensions map it to a structured response — but nothing did. Every one reached the catch-all and came back as 500, including the 400s that RequestHelper raises for a malformed query param and that flash-ext-jackson raises for an unparseable body. A handler doing the documented thing produced the wrong status. The default handler now renders HttpException at its own status, in both dev and prod modes, with the message JSON-escaped. Not pre-encoded like JSON_404 and JSON_500: the message is per-exception, and a path that already unwound a stack does not need the allocation shaved. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
58bae41f7a
commit
fe8c6ed162
@@ -4,6 +4,7 @@ import dev.relism.flash.extension.FlashApp;
|
||||
import dev.relism.flash.models.*;
|
||||
import dev.relism.flash.routing.routers.fastpathrouter.FastPathRouterImpl;
|
||||
import dev.relism.flash.Flash;
|
||||
import dev.relism.flash.exceptions.HttpException;
|
||||
import dev.relism.flash.http.ContentType;
|
||||
import dev.relism.flash.http.HttpMethod;
|
||||
import dev.relism.flash.template.ErrorPages;
|
||||
@@ -56,17 +57,56 @@ public abstract class AbstractRouter {
|
||||
|
||||
protected ExceptionHandler exceptionHandler = Flash.DEV
|
||||
? (ex, req, res) -> {
|
||||
if (ex instanceof HttpException http) return renderHttpException(http, res);
|
||||
res.status(500);
|
||||
res.type(ContentType.TEXT_HTML);
|
||||
return ErrorPages.renderException(req, ex);
|
||||
}
|
||||
: (ex, req, res) -> {
|
||||
if (ex instanceof HttpException http) return renderHttpException(http, res);
|
||||
log.error("Unhandled exception in {} {}", req.method(), req.path(), ex);
|
||||
res.status(500);
|
||||
res.type(ContentType.JSON);
|
||||
return JSON_500;
|
||||
};
|
||||
|
||||
/**
|
||||
* {@link HttpException} carries the status the caller meant; without this it reached the
|
||||
* catch-all above and every one of them came back as 500 — including the 400s
|
||||
* {@code RequestHelper} and {@code flash-ext-jackson} raise for malformed input.
|
||||
*
|
||||
* <p>Deliberately not pre-encoded like {@link #JSON_404}: the message is per-exception, and
|
||||
* an error path that already unwound a stack does not need the allocation shaved.
|
||||
*/
|
||||
private static byte[] renderHttpException(HttpException failure, Response res) {
|
||||
res.status(failure.status());
|
||||
res.type(ContentType.JSON);
|
||||
String message = failure.getMessage();
|
||||
StringBuilder out = new StringBuilder(48 + (message == null ? 0 : message.length()));
|
||||
out.append("{\"error\":\"");
|
||||
escapeJson(message == null ? "" : message, out);
|
||||
out.append("\",\"status\":").append(failure.status()).append('}');
|
||||
return out.toString().getBytes(StandardCharsets.UTF_8);
|
||||
}
|
||||
|
||||
/** Minimal RFC 8259 string escaping — enough for an exception message. */
|
||||
private static void escapeJson(String text, StringBuilder out) {
|
||||
for (int i = 0; i < text.length(); i++) {
|
||||
char c = text.charAt(i);
|
||||
switch (c) {
|
||||
case '"' -> out.append("\\\"");
|
||||
case '\\' -> out.append("\\\\");
|
||||
case '\n' -> out.append("\\n");
|
||||
case '\r' -> out.append("\\r");
|
||||
case '\t' -> out.append("\\t");
|
||||
default -> {
|
||||
if (c < 0x20) out.append(String.format("\\u%04x", (int) c));
|
||||
else out.append(c);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
public SimpleHandler getNotFoundHandler() { return notFoundHandler; }
|
||||
public ExceptionHandler getExceptionHandler() { return exceptionHandler; }
|
||||
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
package dev.relism.flash.routing;
|
||||
|
||||
import dev.relism.flash.exceptions.HttpException;
|
||||
import dev.relism.flash.http.ContentType;
|
||||
import dev.relism.flash.http.HttpMethod;
|
||||
import dev.relism.flash.models.Request;
|
||||
import dev.relism.flash.models.RequestHandler;
|
||||
@@ -7,6 +9,8 @@ import dev.relism.flash.models.Response;
|
||||
import dev.relism.flash.models.SimpleHandler;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import java.nio.charset.StandardCharsets;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.*;
|
||||
|
||||
class AbstractRouterTest {
|
||||
@@ -77,4 +81,23 @@ class AbstractRouterTest {
|
||||
router.onException((ex, req, res) -> "Caught");
|
||||
assertEquals("Caught", router.getExceptionHandler().handle(new RuntimeException(), null, null));
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* HttpException carries the status the caller meant. Before this was honoured every one of
|
||||
* them came back as 500, including the 400s RequestHelper and flash-ext-jackson raise.
|
||||
*/
|
||||
@Test
|
||||
void defaultExceptionHandlerHonoursHttpExceptionStatus() throws Exception {
|
||||
DummyRouter router = new DummyRouter();
|
||||
Response res = new Response(200, ContentType.TEXT_PLAIN);
|
||||
|
||||
Object body = router.getExceptionHandler()
|
||||
.handle(HttpException.badRequest("bad \"input\""), null, res);
|
||||
|
||||
assertEquals(400, res.getStatusCode());
|
||||
String rendered = new String((byte[]) body, StandardCharsets.UTF_8);
|
||||
assertTrue(rendered.contains("\\\"input\\\""), "message must be JSON-escaped: " + rendered);
|
||||
assertTrue(rendered.contains("\"status\":400"), rendered);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user