Skip to content

Commit 8c9e140

Browse files
committed
feat: ExceptionHandler returns Response; problem+json via TypeMapper
1 parent e327640 commit 8c9e140

8 files changed

Lines changed: 112 additions & 92 deletions

File tree

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,14 @@
11
package com.retailsvc.http;
22

3-
import com.sun.net.httpserver.HttpExchange;
4-
import com.sun.net.httpserver.HttpHandler;
5-
import java.io.IOException;
6-
73
/**
8-
* Handle exceptions thrown from a {@link HttpHandler}.
4+
* Maps a {@link Throwable} thrown anywhere in the request pipeline to a {@link Response}.
95
*
10-
* @author thced
6+
* <p>Runs outside any {@code ScopedValue} bindings established by filters or interceptors — scopes
7+
* are torn down as the exception unwinds. Context-aware error mapping (trace IDs, etc.) should be
8+
* done in a {@link RequestInterceptor} that wraps {@code next.proceed()} in try/catch.
119
*/
1210
@FunctionalInterface
1311
public interface ExceptionHandler {
1412

15-
void handle(HttpExchange exchange, Throwable t) throws IOException;
13+
Response handle(Throwable t);
1614
}

src/main/java/com/retailsvc/http/Handlers.java

Lines changed: 22 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,11 @@
77
import static java.net.HttpURLConnection.HTTP_NO_CONTENT;
88
import static java.net.HttpURLConnection.HTTP_OK;
99
import static java.net.HttpURLConnection.HTTP_UNAVAILABLE;
10-
import static java.nio.charset.StandardCharsets.UTF_8;
1110

1211
import com.retailsvc.http.internal.ClasspathResourceHandler;
1312
import com.retailsvc.http.internal.MethodLimitedHandler;
14-
import com.retailsvc.http.internal.ProblemDetailRenderer;
13+
import com.retailsvc.http.internal.ProblemDetail;
1514
import com.sun.net.httpserver.HttpHandler;
16-
import java.io.IOException;
1715
import java.util.List;
1816
import java.util.Objects;
1917
import java.util.function.Supplier;
@@ -27,31 +25,31 @@ public final class Handlers {
2725

2826
private Handlers() {}
2927

30-
public static ExceptionHandler defaultExceptionHandler() {
31-
return (exchange, t) -> {
32-
try (exchange) {
28+
public static ExceptionHandler defaultExceptionHandler(TypeMapper jsonMapper) {
29+
Objects.requireNonNull(jsonMapper, "jsonMapper must not be null");
30+
return t ->
3331
switch (t) {
34-
case ValidationException ve -> {
35-
byte[] body = ProblemDetailRenderer.render(ve.error()).getBytes(UTF_8);
36-
exchange.getResponseHeaders().add("Content-Type", "application/problem+json");
37-
exchange.sendResponseHeaders(HTTP_BAD_REQUEST, body.length);
38-
exchange.getResponseBody().write(body);
39-
}
40-
case NotFoundException _ -> exchange.sendResponseHeaders(HTTP_NOT_FOUND, -1);
41-
case MethodNotAllowedException mna -> {
42-
String allow = mna.allowed().stream().map(Enum::name).collect(Collectors.joining(", "));
43-
exchange.getResponseHeaders().add("Allow", allow);
44-
exchange.sendResponseHeaders(HTTP_BAD_METHOD, -1);
45-
}
32+
case ValidationException ve ->
33+
Response.bytes(
34+
HTTP_BAD_REQUEST,
35+
jsonMapper.writeTo(ProblemDetail.forValidation(ve.error())),
36+
"application/problem+json");
37+
case BadRequestException bre ->
38+
Response.bytes(
39+
bre.status(),
40+
jsonMapper.writeTo(ProblemDetail.forBadRequest(bre)),
41+
"application/problem+json");
42+
case NotFoundException _ -> Response.notFound();
43+
case MethodNotAllowedException mna ->
44+
Response.status(HTTP_BAD_METHOD)
45+
.withHeader(
46+
"Allow",
47+
mna.allowed().stream().map(Enum::name).collect(Collectors.joining(", ")));
4648
default -> {
4749
LOG.error("Unhandled exception in handler", t);
48-
exchange.sendResponseHeaders(HTTP_INTERNAL_ERROR, -1);
50+
yield Response.status(HTTP_INTERNAL_ERROR);
4951
}
50-
}
51-
} catch (IOException io) {
52-
LOG.error("Failed writing error response", io);
53-
}
54-
};
52+
};
5553
}
5654

5755
public static HttpHandler notFoundHandler() {

src/main/java/com/retailsvc/http/OpenApiServer.java

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,6 @@ record HandlerConfig(
7474
requireNonNull(bodyMappers, "bodyMappers must not be null");
7575
requireNonNull(handlerConfig.handlers(), "handlers must not be null");
7676
ExceptionHandler exceptionHandler = handlerConfig.exceptionHandler();
77-
if (exceptionHandler == null) {
78-
LOG.warn("No ExceptionHandler set, using default");
79-
exceptionHandler = Handlers.defaultExceptionHandler();
80-
}
8177

8278
long t0 = System.currentTimeMillis();
8379
Router router = new Router(spec.operations());
@@ -93,9 +89,11 @@ record HandlerConfig(
9389
this.httpServer = HttpServer.create(socketAddress, 0);
9490
httpServer.setExecutor(newThreadPerTaskExecutor(ofVirtual().name("http-", 0).factory()));
9591

92+
ResponseRenderer renderer = new ResponseRenderer(bodyMappers);
93+
9694
String basePath = Optional.ofNullable(spec.basePath()).orElse("/");
9795
HttpContext ctx = httpServer.createContext(basePath);
98-
ctx.getFilters().add(new ExceptionFilter(exceptionHandler));
96+
ctx.getFilters().add(new ExceptionFilter(exceptionHandler, renderer));
9997
ctx.getFilters().add(new RequestPreparationFilter(spec, router, validator, bodyMappers));
10098
ctx.getFilters()
10199
.add(
@@ -110,11 +108,11 @@ record HandlerConfig(
110108
handlerConfig.handlers(),
111109
handlerConfig.interceptors(),
112110
handlerConfig.decorators(),
113-
new ResponseRenderer(bodyMappers)));
111+
renderer));
114112

115113
for (Map.Entry<String, HttpHandler> e : handlerConfig.extras().entrySet()) {
116114
HttpContext extraCtx = httpServer.createContext(e.getKey());
117-
extraCtx.getFilters().add(new ExceptionFilter(exceptionHandler));
115+
extraCtx.getFilters().add(new ExceptionFilter(exceptionHandler, renderer));
118116
extraCtx.setHandler(e.getValue());
119117
}
120118

@@ -320,12 +318,16 @@ public OpenApiServer build() throws IOException {
320318
validateSecurityWiring(spec, securityValidators);
321319
}
322320
Map<String, TypeMapper> resolved = resolveBodyMappers(bodyMappers);
321+
ExceptionHandler effectiveExceptionHandler =
322+
exceptionHandler != null
323+
? exceptionHandler
324+
: Handlers.defaultExceptionHandler(resolved.get(JSON));
323325
HandlerConfig handlerConfig =
324326
new HandlerConfig(
325327
handlers,
326328
interceptors,
327329
decorators,
328-
exceptionHandler,
330+
effectiveExceptionHandler,
329331
extras,
330332
Map.copyOf(securityValidators),
331333
externalAuth);

src/main/java/com/retailsvc/http/internal/ExceptionFilter.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,28 @@
11
package com.retailsvc.http.internal;
22

33
import com.retailsvc.http.ExceptionHandler;
4+
import com.retailsvc.http.Response;
45
import com.sun.net.httpserver.Filter;
56
import com.sun.net.httpserver.HttpExchange;
67
import java.io.IOException;
78

89
public final class ExceptionFilter extends Filter {
10+
911
private final ExceptionHandler handler;
12+
private final ResponseRenderer renderer;
1013

11-
public ExceptionFilter(ExceptionHandler handler) {
14+
public ExceptionFilter(ExceptionHandler handler, ResponseRenderer renderer) {
1215
this.handler = handler;
16+
this.renderer = renderer;
1317
}
1418

1519
@Override
1620
public void doFilter(HttpExchange exchange, Chain chain) throws IOException {
1721
try {
1822
chain.doFilter(exchange);
1923
} catch (RuntimeException | IOException t) {
20-
handler.handle(exchange, t);
24+
Response response = handler.handle(t);
25+
renderer.render(exchange, response);
2126
}
2227
}
2328

src/test/java/com/retailsvc/http/ExtraHandlersIT.java

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package com.retailsvc.http;
22

3-
import static com.retailsvc.http.Handlers.defaultExceptionHandler;
43
import static org.assertj.core.api.Assertions.assertThat;
54

65
import java.net.URI;
@@ -17,7 +16,6 @@ void aliveExtraReturns204AndBypassesValidation() throws Exception {
1716
newBuilder()
1817
.spec(spec)
1918
.handlers(Map.of())
20-
.exceptionHandler(defaultExceptionHandler())
2119
.port(0)
2220
.extraRoute("/alive", Handlers.aliveHandler())
2321
.build();
@@ -41,7 +39,6 @@ void specHandlerServesClasspathResource() throws Exception {
4139
newBuilder()
4240
.spec(spec)
4341
.handlers(Map.of())
44-
.exceptionHandler(defaultExceptionHandler())
4542
.port(0)
4643
.extraRoute("/openapi.yaml", Handlers.specHandler("/openapi.yaml"))
4744
.build();
@@ -68,13 +65,7 @@ void extraHandlerExceptionFlowsThroughExceptionHandler() throws Exception {
6865
};
6966

7067
try (var s =
71-
newBuilder()
72-
.spec(spec)
73-
.handlers(Map.of())
74-
.exceptionHandler(defaultExceptionHandler())
75-
.port(0)
76-
.extraRoute("/boom", boom)
77-
.build();
68+
newBuilder().spec(spec).handlers(Map.of()).port(0).extraRoute("/boom", boom).build();
7869
var client = httpClient()) {
7970

8071
var req =
Lines changed: 59 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,54 +1,80 @@
11
package com.retailsvc.http;
22

33
import static org.assertj.core.api.Assertions.assertThat;
4-
import static org.mockito.Mockito.mock;
54

65
import com.retailsvc.http.spec.HttpMethod;
76
import com.retailsvc.http.validate.ValidationError;
8-
import com.sun.net.httpserver.Headers;
9-
import com.sun.net.httpserver.HttpExchange;
10-
import java.io.ByteArrayOutputStream;
7+
import java.nio.charset.StandardCharsets;
8+
import java.util.Map;
119
import java.util.Set;
1210
import org.junit.jupiter.api.Test;
13-
import org.mockito.Mockito;
1411

1512
class HandlersDefaultExceptionTest {
16-
private HttpExchange newExchange(ByteArrayOutputStream sink) {
17-
HttpExchange ex = mock(HttpExchange.class);
18-
Mockito.when(ex.getResponseHeaders()).thenReturn(new Headers());
19-
Mockito.when(ex.getResponseBody()).thenReturn(sink);
20-
return ex;
13+
14+
private static final TypeMapper JSON = new GsonTypeMapper();
15+
16+
@Test
17+
void validationExceptionRendersProblemJson() {
18+
Response resp =
19+
Handlers.defaultExceptionHandler(JSON)
20+
.handle(
21+
new ValidationException(
22+
new ValidationError("/x", "type", "expected string", null)));
23+
24+
assertThat(resp.status()).isEqualTo(400);
25+
assertThat(resp.contentType()).isEqualTo("application/problem+json");
26+
byte[] bytes = (byte[]) resp.body();
27+
String json = new String(bytes, StandardCharsets.UTF_8);
28+
@SuppressWarnings("unchecked")
29+
Map<String, Object> parsed = (Map<String, Object>) JSON.readFrom(bytes, "application/json");
30+
assertThat(parsed).containsEntry("keyword", "type");
31+
assertThat(((Number) parsed.get("status")).intValue()).isEqualTo(400);
32+
assertThat(json).contains("expected string");
2133
}
2234

2335
@Test
24-
void validationExceptionRendersProblem() throws Exception {
25-
ByteArrayOutputStream sink = new ByteArrayOutputStream();
26-
HttpExchange ex = newExchange(sink);
27-
28-
Handlers.defaultExceptionHandler()
29-
.handle(
30-
ex,
31-
new ValidationException(new ValidationError("/x", "type", "expected string", null)));
32-
33-
Mockito.verify(ex).sendResponseHeaders(Mockito.eq(400), Mockito.anyLong());
34-
assertThat(ex.getResponseHeaders().getFirst("Content-Type"))
35-
.isEqualTo("application/problem+json");
36-
assertThat(sink.toString()).contains("\"keyword\":\"type\"");
36+
void badRequestExceptionRendersProblemJsonWithCustomStatus() {
37+
Response resp =
38+
Handlers.defaultExceptionHandler(JSON)
39+
.handle(new BadRequestException(422, "email taken", "/email", "unique"));
40+
41+
assertThat(resp.status()).isEqualTo(422);
42+
assertThat(resp.contentType()).isEqualTo("application/problem+json");
43+
@SuppressWarnings("unchecked")
44+
Map<String, Object> parsed =
45+
(Map<String, Object>) JSON.readFrom((byte[]) resp.body(), "application/json");
46+
assertThat(((Number) parsed.get("status")).intValue()).isEqualTo(422);
47+
assertThat(parsed)
48+
.containsEntry("title", "Unprocessable Content")
49+
.containsEntry("detail", "email taken")
50+
.containsEntry("pointer", "/email")
51+
.containsEntry("keyword", "unique");
3752
}
3853

3954
@Test
40-
void notFoundReturns404() throws Exception {
41-
HttpExchange ex = newExchange(new ByteArrayOutputStream());
42-
Handlers.defaultExceptionHandler().handle(ex, new NotFoundException("GET /x"));
43-
Mockito.verify(ex).sendResponseHeaders(404, -1);
55+
void notFoundReturns404() {
56+
Response resp = Handlers.defaultExceptionHandler(JSON).handle(new NotFoundException("GET /x"));
57+
58+
assertThat(resp.status()).isEqualTo(404);
59+
assertThat(resp.body()).isNull();
4460
}
4561

4662
@Test
47-
void methodNotAllowedReturns405WithAllowHeader() throws Exception {
48-
HttpExchange ex = newExchange(new ByteArrayOutputStream());
49-
Handlers.defaultExceptionHandler()
50-
.handle(ex, new MethodNotAllowedException(Set.of(HttpMethod.GET, HttpMethod.POST)));
51-
Mockito.verify(ex).sendResponseHeaders(405, -1);
52-
assertThat(ex.getResponseHeaders().getFirst("Allow")).contains("GET").contains("POST");
63+
void methodNotAllowedReturns405WithAllowHeader() {
64+
Response resp =
65+
Handlers.defaultExceptionHandler(JSON)
66+
.handle(new MethodNotAllowedException(Set.of(HttpMethod.GET, HttpMethod.POST)));
67+
68+
assertThat(resp.status()).isEqualTo(405);
69+
assertThat(resp.headers()).containsKey("Allow");
70+
assertThat(resp.headers().get("Allow")).contains("GET").contains("POST");
71+
}
72+
73+
@Test
74+
void unknownExceptionReturns500() {
75+
Response resp = Handlers.defaultExceptionHandler(JSON).handle(new RuntimeException("kaboom"));
76+
77+
assertThat(resp.status()).isEqualTo(500);
78+
assertThat(resp.body()).isNull();
5379
}
5480
}

src/test/java/com/retailsvc/http/internal/ExceptionFilterTest.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,28 +4,33 @@
44

55
import com.retailsvc.http.ExceptionHandler;
66
import com.retailsvc.http.NotFoundException;
7+
import com.retailsvc.http.Response;
78
import com.sun.net.httpserver.Filter;
89
import com.sun.net.httpserver.HttpExchange;
910
import org.junit.jupiter.api.Test;
1011
import org.mockito.Mockito;
1112

1213
class ExceptionFilterTest {
14+
1315
@Test
1416
void delegatesToExceptionHandler() throws Exception {
1517
HttpExchange ex = mock(HttpExchange.class);
18+
ResponseRenderer renderer = mock(ResponseRenderer.class);
1619
ExceptionHandler handler = mock(ExceptionHandler.class);
17-
Filter f = new ExceptionFilter(handler);
20+
Mockito.when(handler.handle(Mockito.any())).thenReturn(Response.status(404));
21+
Filter f = new ExceptionFilter(handler, renderer);
1822
Filter.Chain chain = mock(Filter.Chain.class);
1923
Mockito.doThrow(new NotFoundException("x")).when(chain).doFilter(ex);
2024
f.doFilter(ex, chain);
21-
Mockito.verify(handler).handle(Mockito.eq(ex), Mockito.any(NotFoundException.class));
25+
Mockito.verify(handler).handle(Mockito.any(NotFoundException.class));
2226
}
2327

2428
@Test
2529
void passThroughOnSuccess() throws Exception {
2630
HttpExchange ex = mock(HttpExchange.class);
31+
ResponseRenderer renderer = mock(ResponseRenderer.class);
2732
ExceptionHandler handler = mock(ExceptionHandler.class);
28-
Filter f = new ExceptionFilter(handler);
33+
Filter f = new ExceptionFilter(handler, renderer);
2934
Filter.Chain chain = mock(Filter.Chain.class);
3035
f.doFilter(ex, chain);
3136
Mockito.verify(chain).doFilter(ex);

src/test/java/com/retailsvc/http/start/ServerLauncher.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
package com.retailsvc.http.start;
22

3-
import com.retailsvc.http.ExceptionHandler;
4-
import com.retailsvc.http.Handlers;
53
import com.retailsvc.http.OpenApiServer;
64
import com.retailsvc.http.RequestHandler;
75
import com.retailsvc.http.Response;
@@ -44,12 +42,9 @@ public ServerLauncher() throws IOException {
4442
handlers.put("secureBasic", req -> Response.status(200));
4543
handlers.put("secureOpen", req -> Response.status(200));
4644

47-
ExceptionHandler exceptionHandler = Handlers.defaultExceptionHandler();
48-
4945
OpenApiServer.builder()
5046
.spec(spec)
5147
.handlers(handlers)
52-
.exceptionHandler(exceptionHandler)
5348
.securityValidator("apiKeyAuth", (req, cred) -> Optional.empty())
5449
.securityValidator("bearerAuth", (req, cred) -> Optional.empty())
5550
.securityValidator("basicAuth", (req, cred) -> Optional.empty())

0 commit comments

Comments
 (0)