Skip to content

Conversation

nzws
Copy link

@nzws nzws commented Oct 2, 2025

fix #1360

Fixes a bug where onAfterResponse is not called when an onError handler returns a value in AOT mode.

Summary by CodeRabbit

  • Bug Fixes

    • Ensures after-response hooks run even when requests end in errors, including cases where an error handler returns a value. This aligns post-response behavior between success and error flows and guarantees consistent lifecycle execution.
  • Tests

    • Added parameterized tests verifying the after-response callback executes exactly once after error handling and that the response body matches the value returned by the error handler (string or JSON) across different modes.

Copy link

coderabbitai bot commented Oct 2, 2025

Walkthrough

Adds an afterResponse() invocation to the error-handling path in src/compose.ts so onAfterResponse runs when onError returns a value. Introduces a test ensuring onAfterResponse executes once after error handling across AOT modes and different onError return types.

Changes

Cohort / File(s) Summary
Core error flow update
src/compose.ts
Insert afterResponse() call in the error branch before returning via mapEarlyResponse, aligning error responses with post-response hook execution.
Lifecycle tests
test/lifecycle/response.test.ts
Add parameterized test verifying onAfterResponse runs once when onError returns a value (string/object) under both aot true/false; asserts response body and after-response counter.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant H as composeHandler
  participant OE as onError
  participant MR as mapEarlyResponse
  participant AR as afterResponse

  C->>H: Request
  H->>H: Route handler throws
  H->>OE: Invoke onError(error)
  OE-->>H: Returns value (string/object)
  H->>MR: mapEarlyResponse(value)
  H->>AR: afterResponse()  Note over H,AR: Newly executed in error path
  MR-->>C: Send response
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • SaltyAom

Poem

A hiccup hops, an error pops—oh dear!
I twitch my ears and log it, never fear.
After the fuss, a gentle thump—complete,
Hooks all called, the cycle neat.
Carrot-shaped thanks for tidy flow—off we go! 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly states the core fix—ensuring onAfterResponse is called when onError returns a value in AOT mode—without extraneous details.
Linked Issues Check ✅ Passed This PR directly addresses issue #1360 by adding an afterResponse invocation in the error branch of composeHandler when onError returns a value and includes tests in both dynamic and AOT modes confirming that onAfterResponse is called as expected.
Out of Scope Changes Check ✅ Passed All modifications are limited to invoking afterResponse in the error-handling path and adding corresponding tests, with no unrelated alterations in this PR.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dc978ad and b081bd5.

📒 Files selected for processing (2)
  • src/compose.ts (1 hunks)
  • test/lifecycle/response.test.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/lifecycle/response.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/compose.ts (1)
test/types/index.ts (2)
  • afterResponse (1999-2009)
  • afterResponse (2473-2475)
🔇 Additional comments (1)
src/compose.ts (1)

1965-1965: LGTM! Fix correctly ensures onAfterResponse is called in error path.

This change properly addresses issue #1360 by invoking afterResponse() when an onError handler returns a value. The placement is correct—after trace resolution and before returning the error response—which matches the pattern used in success paths throughout the handler (lines 1774, 1817, 1831, 1863, 1870).

The fix ensures that the generated handler code will execute afterResponse hooks even when errors are handled and returned by onError handlers, maintaining consistency with the behavior when requests complete successfully.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@nzws nzws marked this pull request as draft October 2, 2025 06:06
@nzws nzws force-pushed the fix/on-after-response-call branch from d52ed26 to dc978ad Compare October 2, 2025 06:07
@nzws nzws marked this pull request as ready for review October 2, 2025 06:07
@nzws nzws force-pushed the fix/on-after-response-call branch from dc978ad to b081bd5 Compare October 2, 2025 06:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

onAfterResponse is not called when responding with an error manually.

1 participant