Skip to content

Commit 5294028

Browse files
committed
fix: Address final-review feedback for after-response hooks
1 parent 592fffe commit 5294028

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
@@ -270,7 +270,11 @@ public void afterResponse(Runnable runnable) {
270270
afterHooks.add(runnable);
271271
}
272272

273-
/** Internal accessor for the after-hook queue; used by RequestPreparationFilter. */
273+
/**
274+
* Returns an unmodifiable view of the queued after-response runnables. Intended for the framework
275+
* runner; consumers should use {@link #afterResponse(Runnable)} to register runnables rather than
276+
* inspecting this list directly.
277+
*/
274278
public List<Runnable> afterHooks() {
275279
return Collections.unmodifiableList(afterHooks);
276280
}

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)