Skip to content

Conversation

@taosin
Copy link

@taosin taosin commented Jan 24, 2026

Summary

  • Add doc comments to errorHandler and wsErrorHandler clarifying they are final handlers and intentionally do not call next().

Rationale

  • Keeps behavior documentation consistent between HTTP and WebSocket error handlers.
  • Makes intent visible at the function level (more discoverable than an inline comment).

Test plan

  • Not applicable (comment-only change).

@taosin taosin requested a review from a team as a code owner January 24, 2026 12:45
Copy link
Member

@code-asher code-asher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! I am not convinced we need the comment, to me the end is self-explanatory.

We also make no such comments for why the non-websocket error handler also does not call next(), so it seems inconsistent.

I think it would make more sense as a comment on wsErrorHandler and errorHandler themselves though, because those would show up in the documentation when calling the functions, where end would not be visible and thus less self-explanatory. Something like:

This handler does not call next even though it accepts it as an argument. It should only be used as the final handler for a route.

@taosin taosin changed the title fix: enhance WebSocket error handling with status code logging docs: clarify error handlers are final and don't call next() Jan 27, 2026
@taosin
Copy link
Author

taosin commented Jan 27, 2026

Thanks for the PR! I am not convinced we need the comment, to me the end is self-explanatory.

We also make no such comments for why the non-websocket error handler also does not call next(), so it seems inconsistent.

I think it would make more sense as a comment on wsErrorHandler and errorHandler themselves though, because those would show up in the documentation when calling the functions, where end would not be visible and thus less self-explanatory. Something like:

This handler does not call next even though it accepts it as an argument. It should only be used as the final handler for a route.

Thanks for the review!

Good point — I agree the inline comment near end() is a bit noisy and also inconsistent with errorHandler.
I’ve moved the explanation to a doc comment on both errorHandler and wsErrorHandler to keep them consistent and make it visible where the functions are referenced, and removed the inline comment.

@taosin taosin requested a review from code-asher January 27, 2026 12:15
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.

2 participants