Skip to content

Optimize callback in HTTP/Websocket#4

Open
mwfj wants to merge 2 commits intomainfrom
optimize-ws-http-callbacks
Open

Optimize callback in HTTP/Websocket#4
mwfj wants to merge 2 commits intomainfrom
optimize-ws-http-callbacks

Conversation

@mwfj
Copy link
Owner

@mwfj mwfj commented Mar 24, 2026

Summary

Align the HTTP/WS layer's callback organization with the base reactor's established pattern, and improve readability of the largest method in the codebase.

Changes

Callback Registry Extraction

  • Added include/http/http_callbacks.h with HTTP_CALLBACKS_NAMESPACE containing:
    • HttpConnCallbacks
    • WsCallbacks
  • This mirrors how include/callbacks.h organizes core reactor callbacks.
  • Updated HttpConnectionHandler:
    • Replaced 4 inline using declarations and 4 individual member fields
    • Consolidated them into a single callbacks_ struct
  • Updated WebSocketConnection:
    • Replaced 4 callback types and 4 member fields
    • Consolidated them into a single callbacks_ struct
  • Preserved public type aliases for backward compatibility, so no caller changes are required.

OnRawData Split

Split HttpConnectionHandler::OnRawData() (~500 lines) into 4 private member functions:

  • HandleUpgradedData()
    • WebSocket data forwarding with exception safety
  • HandleParseError()
    • Error response dispatch by parser error type
  • HandleCompleteRequest()
    • HTTP validation, WebSocket upgrade path, request dispatch, and keep-alive handling
  • HandleIncompleteRequest()
    • Early rejection, Expect header handling, and body-size checks

Notes:

  • All extracted paths are direct method calls
  • No std::function callbacks were introduced
  • No runtime overhead added

Helper Extraction

  • Extracted HttpServer::SafeNotifyWsClose()
  • Replaces 4 identical null-check + try/catch WebSocket close-notify blocks across:
    • HandleNewConnection
    • HandleCloseConnection
    • HandleErrorConnection
    • HandleMessage

Test Plan

  • make clean && make -j4 compiles cleanly
  • 51/51 tests pass via ./run
  • Verified stability across 3 consecutive runs

Notes

  • Pure refactor only
  • No behavior changes
  • No new tests needed

@gemini-code-assist
Copy link

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request focuses on significant internal refactoring of the HTTP and WebSocket layers to enhance code organization and readability. It centralizes callback definitions into dedicated structs, breaks down a complex data handling method into more manageable private functions, and consolidates duplicated WebSocket close notification logic. These changes are purely structural, ensuring no alterations to existing behavior or introduction of runtime overhead, thereby improving the maintainability and clarity of the codebase.

Highlights

  • Callback Registry Extraction: Introduced a new header file, http_callbacks.h, to centralize HTTP and WebSocket callback definitions into HttpConnCallbacks and WsCallbacks structs. The HttpConnectionHandler and WebSocketConnection classes were updated to utilize these new structs for their callback member fields, while maintaining public type aliases for backward compatibility.
  • OnRawData Method Split: The large HttpConnectionHandler::OnRawData() method, previously around 500 lines, was refactored and split into four distinct private helper functions: HandleUpgradedData(), HandleParseError(), HandleCompleteRequest(), and HandleIncompleteRequest(). This significantly improves the readability and maintainability of the HTTP request processing logic.
  • Helper Function Extraction: A new helper function, HttpServer::SafeNotifyWsClose(), was extracted to encapsulate the logic for safely notifying WebSocket connections of transport closure. This helper replaces four identical null-check and try/catch blocks across various HttpServer methods, reducing code duplication.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@mwfj
Copy link
Owner Author

mwfj commented Mar 24, 2026

@codex review

@chatgpt-codex-connector
Copy link

Codex Review: Didn't find any major issues. 👍

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors callback management for HTTP and WebSocket connections by introducing a new http_callbacks.h header. This centralizes callback type definitions into HttpConnCallbacks and WsCallbacks structs, replacing individual callback members in HttpConnectionHandler and WebSocketConnection for improved organization. The HttpConnectionHandler::OnRawData method was also refactored into smaller, more readable helper methods, and a SafeNotifyWsClose helper was added to HttpServer for robust WebSocket close notifications. A review comment suggests shortening the HTTP_CALLBACKS_NAMESPACE name for better readability.

struct HttpRequest;
class HttpResponse;

namespace HTTP_CALLBACKS_NAMESPACE {

Choose a reason for hiding this comment

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

medium

The namespace HTTP_CALLBACKS_NAMESPACE is quite verbose. Consider using a shorter, more conventional name like HttpCallbacks or HttpWsCallbacks to improve readability and reduce verbosity in qualified names.

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.

1 participant