Skip to content

Commit 724b3d8

Browse files
committed
fix: Address final-review feedback for after-response hooks
1 parent 94fbe7b commit 724b3d8

3 files changed

Lines changed: 35 additions & 4 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ Map<String, RequestHandler> handlers = Map.of(
368368
```
369369

370370
Global hooks run first (registration order), then per-request runnables (FIFO). Pre-request
371-
failures (404, 405, validation) do not fire hooks.
371+
failures (404, 405, validation) do not fire hooks. On the error path (when a handler throws), `Response#body()` is `null` and the bytes have already been streamed; use `Response#status()` to detect errors.
372372

373373
### End-to-end example
374374

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,11 @@ public void afterResponse(Runnable runnable) {
337337
afterHooks.add(runnable);
338338
}
339339

340-
/** Internal accessor for the after-hook queue; used by RequestPreparationFilter. */
340+
/**
341+
* Returns an unmodifiable view of the queued after-response runnables. Intended for the framework
342+
* runner; consumers should use {@link #afterResponse(Runnable)} to register runnables rather than
343+
* inspecting this list directly.
344+
*/
341345
public List<Runnable> afterHooks() {
342346
return Collections.unmodifiableList(afterHooks);
343347
}

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

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,6 @@ void preRequestFailureSkipsHooks() throws Exception {
205205
BodyHandlers.discarding());
206206

207207
assertThat(resp.statusCode()).isEqualTo(HTTP_NOT_FOUND);
208-
// Give the server-side thread a moment to complete any potential async work
209-
Thread.sleep(100);
210208
assertThat(log).isEmpty();
211209
}
212210
}
@@ -246,4 +244,33 @@ void hookSeesScopedRequestAndSameThreadAsHandler() throws Exception {
246244
assertThat(hookThread.get()).isSameAs(handlerThread.get());
247245
}
248246
}
247+
248+
@Test
249+
void hookFiresWhenHandlerIsMissing() throws Exception {
250+
CountDownLatch latch = new CountDownLatch(1);
251+
AtomicReference<Response> capturedResponse = new AtomicReference<>();
252+
253+
try (OpenApiServer server =
254+
baseBuilder()
255+
.handlers(Map.of()) // no handler registered for OK_OPERATION_ID
256+
.afterResponseHook(
257+
(req, resp) -> {
258+
capturedResponse.set(resp);
259+
latch.countDown();
260+
})
261+
.build()) {
262+
263+
HttpResponse<Void> resp =
264+
HttpClient.newHttpClient()
265+
.send(
266+
HttpRequest.newBuilder(uri(server, OK_PATH)).GET().build(),
267+
BodyHandlers.discarding());
268+
269+
assertThat(latch.await(5, TimeUnit.SECONDS)).isTrue();
270+
assertThat(resp.statusCode()).isEqualTo(HTTP_INTERNAL_ERROR);
271+
assertThat(capturedResponse.get()).isNotNull();
272+
assertThat(capturedResponse.get().status()).isEqualTo(HTTP_INTERNAL_ERROR);
273+
assertThat(capturedResponse.get().body()).isNull();
274+
}
275+
}
249276
}

0 commit comments

Comments
 (0)