Yong Tan

This commit **completes the implementation** of the `RefactoredRocketServerConnection` within the **Thrift Rocket server transport**, significantly enhancing its capabilities and correctness. It introduces robust handling for **sink and BiDi stream payloads** and inline frames, alongside a crucial **bug fix** addressing silent drops of `REQUEST_N` frames in `ExistingStreamFrameHandler`. Furthermore, this **feature completion** adds support for `setDecodeMetadataUsingBinary` in `ConnectionAdapter` and `SetupFrameAcceptor`, and integrates new stream management methods like `getInteractionSnapshots` and `scheduleStreamTimeout`. This work ensures proper stream lifecycle management, prevents data loss, and improves the overall reliability and feature set of the new Rocket server connection.

This commit introduces several **critical bug fixes** and minor **refactorings** to the **Rocket transport's outgoing frame handling** components, primarily addressing issues within `OutgoingFrameHandler`, `RefactoredRocketServerConnection`, and `ConnectionAdapter`. Key changes include defaulting `rocket_use_outgoing_frame_handler` to `false` due to performance concerns, correcting frame buffering logic by removing a misplaced `clear()` call, and ensuring proper initialization of `SerializationEvent` `streamId`. Furthermore, it bypasses the `OutgoingFrameHandler` for **FD-bearing payloads**, **connection-level errors**, and `sendMetadataPush()` to prevent data loss or incorrect behavior, and fixes `isOutgoingFrameHandlerBusy()` for graceful shutdown. These changes significantly improve the **correctness and stability** of outgoing frame serialization and writing in the **Thrift Rocket server**, also including a fix for `RingBuffer` allocation and a test data race.

This commit performs a **refactoring** within the **Thrift Rocket server transport** by replacing direct usage of the concrete `RocketServerConnection` type with its `IRocketServerConnection` interface. Specifically, method signatures in `RocketSetupProcessor`, `RocketRequestProcessor`, `RocketRequestOrchestrator`, and `RefactoredThriftRocketServerHandler` are updated to accept `IRocketServerConnection&` parameters. This change removes previously unsafe `static_cast` operations and is a crucial prerequisite for safely enabling the `rocket_server_use_refactored_handler` and `rocket_use_factored_server_connection` flags concurrently. As all methods called on the connection are virtual on `IRocketServerConnection`, this is a mechanical type change with no alteration to existing logic.

This commit introduces the `RocketRequestOrchestrator` class, **refactoring** the complex request processing pipeline within the **Thrift Rocket server transport**. It extracts the monolithic `handleRequestCommon` function from `ThriftRocketServerHandler` into this dedicated orchestrator, decomposing its logic into several focused methods such as `parsePayloadAndExtractFds` and `finalizeAndDispatchRequest`. This change significantly improves the **code organization and testability** of the **Rocket server's request handling**, as the new class now coordinates payload parsing, validation, context setup, overload checking, and request dispatching. The functionality remains identical, ensuring no external impact on the server's behavior.

This commit introduces `RefactoredThriftRocketServerHandler`, completing a significant **refactoring** effort to modernize the **Thrift Rocket server transport** architecture. The new handler leverages previously extracted modular components like `RocketErrorHandler`, `RocketContextManager`, `RocketRequestProcessor`, `RocketRequestOrchestrator`, and `RocketSetupProcessor` to manage error handling, request context, payload processing, orchestration, and connection setup, respectively. This architectural improvement replaces the monolithic `ThriftRocketServerHandler` with a more focused and maintainable design. By integrating with `Cpp2ConnContext` and `ThriftServer`, this change enhances the overall robustness, testability, and extensibility of the **Thrift server**'s Rocket transport layer.

This commit performs a significant **refactoring** within the **Thrift Rocket server transport** by **extracting the entire setup handshake logic** from the monolithic `ThriftRocketServerHandler` into a new, dedicated class, `RocketSetupProcessor`. The new `RocketSetupProcessor` is now solely responsible for managing the initial Rocket client connection setup, encompassing tasks like frame validation, protocol negotiation, interceptor execution, and processor configuration. This **improves the modularity, testability, and maintainability** of the **Thrift Rocket server** by adhering to the Single Responsibility Principle, without altering existing runtime behavior. It lays the groundwork for further decomposition of `ThriftRocketServerHandler` into more focused components, enhancing the overall design of the `thrift/lib/cpp2/transport/rocket/server` module.

This commit **enables the controlled rollout** of the `RefactoredThriftRocketServerHandler` within the **Thrift Rocket server** infrastructure. It introduces a new feature flag, `rocket_server_use_refactored_handler`, which allows `RocketRoutingHandler::handleConnection` to dynamically select between the legacy and refactored handler implementations. This **refactoring enablement** completes the integration of the new modular handler architecture, ensuring new connections can utilize it while existing connections remain unaffected. The change facilitates a safe, gradual transition to the refactored handler, with benchmarking indicating no significant performance regression.

This commit introduces a **new component**, `RocketRequestHandler`, to the **Thrift Rocket server transport**, significantly **refactoring** its request handling mechanism. This class now **encapsulates all request processing logic**, including setup results, request factories, and a built-in sampling method, making it a fully self-contained unit. By centralizing these responsibilities, the commit enhances the **modularity and maintainability** of the server's request processing subsystem, reducing dependencies on facade internals.

This commit performs a significant **refactoring** by **extracting request payload processing logic** from the monolithic `ThriftRocketServerHandler` into a new, dedicated class, `RocketRequestProcessor`. This new module centralizes responsibilities such as payload parsing, compression handling, checksum validation, and file descriptor extraction for the **Thrift Rocket server transport**. The change significantly improves the **modularity and testability** of the server-side request handling, making the codebase more maintainable and focused without altering external behavior.

This commit performs a significant **refactoring** by extracting error handling logic from the monolithic `ThriftRocketServerHandler` into a new, dedicated class: `RocketErrorHandler`. This change improves the **modularity and testability** of the **Thrift Rocket transport layer** by centralizing all error-related methods, such as `handleRequestWithBadMetadata()` and `handleAppError()`, into `thrift/lib/cpp2/transport/rocket/server/detail/RocketErrorHandler.cpp`. The new `RocketErrorHandler` now provides a clean, focused interface for managing various error scenarios, making the `ThriftRocketServerHandler` more streamlined and easier to maintain. This continues the effort to break down large components into smaller, more manageable units, enhancing overall code quality.

This commit performs a **refactoring** of the **Thrift Rocket server transport** by extracting all request context management logic from the monolithic `ThriftRocketServerHandler` into a new, dedicated module, `RocketContextUtils`. This move centralizes functions such as `createRequestContext`, `setupRequestContext`, and `extractRequestContextData`, which were previously embedded within the handler. The primary goal is to improve the **modularity and testability** of the server-side request handling, making `ThriftRocketServerHandler` less complex and easier to maintain. This architectural improvement streamlines how request contexts are created, configured, and managed for Rocket server connections.

This commit delivers a **critical bug fix** addressing **threading violations** within the **Thrift stress test client** when attempting `stopTLSv1` negotiation. It resolves `EventBase` thread affinity issues that caused crashes when `AsyncSocket` was created or `fizz::client::AsyncFizzClient` was destroyed on incorrect threads, particularly during negotiation failures with non-supporting servers. The fix simplifies socket creation in `ClientFactory.cpp`, ensures thread-safe `AsyncFizzClient` destruction in `FizzStopTLSConnector.h`, and adds a **graceful fallback mechanism** for `stopTLSv1` negotiation failures. This significantly improves the **Thrift conformance stress test client's robustness** and reliability, preventing crashes and ensuring proper handling of TLS negotiation.

This commit performs a **cleanup and maintenance** operation by **removing deprecated legacy payload serialization utilities** from the **Thrift C++2 Rocket transport payload subsystem**. Specifically, the files `LegacyPayloadSerializerStrategy.h`, `PayloadUtils.h`, and `PayloadUtils.cpp` are deleted, indicating that their functionality has been successfully replaced or is no longer needed after prior migration efforts. This action improves the overall **code health and maintainability** of the `thrift/lib/cpp2/transport/rocket/payload` module. Additionally, a minor **typo fix** is applied to a test case name in `PayloadSerializerTest.cpp`.

This commit implements a **performance fix** for a **bottleneck** encountered when using the **`--gen_load=true` option** in the stress test client. It refactors the **`ClientRunner`** to distribute load generation across multiple threads by replacing a single `PoissonLoadGenerator` instance with a vector of unique pointers to `PoissonLoadGenerator` objects. The `ClientRunner`'s constructor now initializes a separate load generator for each client thread, dividing the `targetQps` to ensure balanced load. This change significantly improves the **load generation throughput** and overall performance of the **`thrift/conformance/stresstest/client`** module.

This commit **introduces comprehensive support for StopTLS v1 and v2 within the Thrift conformance stress test framework**, enabling robust testing of **selective encryption capabilities** for Thrift connections. It adds client-side configuration, a new `FizzStopTLSConnector` for v1, and server-side handling to process unencrypted payloads for v2, along with new test scenarios like `RequestResponseTm_32k_unencrypted`. Additionally, this work **fixes a critical bug** where StopTLS v1 negotiation could get stuck due to `EventBase` conflicts and resolves a dev-mode crash in `ClientRunner.cpp` related to timeout object destruction. This **enhances the stability and reliability of the stress test environment**, allowing for thorough validation of Thrift's advanced TLS features under load.

This commit initiates a **refactoring** effort within the **Thrift Rocket transport layer** by **migrating the default payload serialization strategy from `LegacyPayloadSerializerStrategy` to `DefaultPayloadSerializerStrategy`**. This change updates the core `PayloadSerializer` implementation and transitions the `thrift/conformance/stresstest` client and server to utilize the new default. The work is a **preparatory step towards deprecating and eventually removing legacy serialization components**, ensuring internal systems adopt the modern approach. While the legacy components are not yet deleted due to external dependencies, this foundational change sets the stage for future cleanup and modernization of the serialization infrastructure.

This commit introduces a **new capability** to enhance observability around checksum handling in Thrift RPCs. A `recordChecksumSkipped` callback is added to the **Thrift C++2 Rocket transport's checksum payload serialization strategy**, which is invoked when the `rpcOptions` explicitly set the checksum algorithm to `NONE`. This allows for better instrumentation and monitoring of scenarios where checksum validation is intentionally bypassed, impacting the **Thrift conformance stress test client** by integrating this new callback into its `co_echo` method.

This commit introduces a **defensive check** within the **Thrift Rocket transport** to prevent clients from attempting to send checksums when the underlying `PayloadSerializer` does not support a checksumming strategy. Specifically, the `packWithFds` method in `thrift/lib/cpp2/transport/rocket/payload/PayloadSerializer.h` now verifies if checksums are supported; if not, it logs a warning and resets the checksum algorithm to `NONE`. This **bug fix** enhances **data integrity** and **system reliability** by preventing erroneous checksum transmissions, ensuring that only valid checksums are processed.

This commit **refactors error handling** within the **Thrift C++2 Rocket transport's payload serialization and validation logic**. It modifies the `calculateChecksum` and `validateChecksum` functions in `thrift/lib/cpp2/transport/rocket/payload/ChecksumPayloadSerializerStrategy.h` to throw a **`TApplicationException::CHECKSUM_MISMATCH`** when an unsupported checksum algorithm is encountered. This change replaces the previous use of a generic `std::runtime_error`, providing a **more specific and standardized exception type** for consumers to handle. A new test case has been added to verify that `packWithFds` correctly throws this exception, improving the robustness of the **checksum validation subsystem**.

This commit **fixes a test failure** in the **Thrift C++2 library's `RequestChannelTest`** on Mac, specifically addressing an issue with platform-specific exception messages. When a server is missing, Mac and Linux report different `TTransportException` messages, causing the `with_missing_server_fails` test to fail on Mac. The change updates the expected error string in `thrift/lib/cpp2/test/RequestChannelTest.cpp` to match the more general `TTransportException` type. This **bug fix** ensures the test passes reliably on both **Mac and Linux** by accommodating these platform variations in error reporting.