Alex Snast

This commit introduces **new macros**, `BENCHMARK_COUNTERS_NAMED_PARAM` and `BENCHMARK_COUNTERS_PARAM`, to the **`folly::Benchmark` library**, providing a **new capability** for performance testing. These additions enable benchmarks that utilize custom parameters to also record **user-defined counters**, extending the functionality of existing parameterized benchmark macros. Specifically, `BENCHMARK_COUNTERS_NAMED_PARAM` allows a `UserCounters&` object to be passed to the benchmark function, facilitating the collection of custom metrics. This enhancement allows developers to conduct more comprehensive performance analysis by tracking specific, user-defined metrics alongside varying test configurations.

This commit introduces a **performance optimization** and **refactoring** within the **Thrift C++2 asynchronous HTTP client channel**. It significantly **reduces heap allocations** in the `HTTPClientChannel::setHeaders` method by optimizing the header encoding path. Specifically, it pre-allocates buffers and encodes directly into them, cutting allocations from approximately four to two per header containing a colon, thereby **reducing memory overhead** for HTTP requests. The encoding logic is also **refactored** into a new helper function, `encodeHeader`, improving code maintainability and clarity. This change directly impacts the efficiency of HTTP request processing for `HTTPClientChannel` users.

This commit **refactors** the **`HTTPClientChannel`** class within the **Thrift C++2 async library** by making its `setHeaders` method **static**. This change clarifies that the method does not access any instance-specific data, improving code readability and maintainability for the **HTTP client channel**. As a **refactoring** effort, it allows `setHeaders` to be called without an object instance, potentially enabling better compiler optimizations for header manipulation. The change is internal and does not alter the external behavior or API of the **Thrift HTTP client**.

This commit performs **refactoring** and a **build fix** within the **Thrift C++2 asynchronous HTTP client channel**. It addresses a **missing dependency** by adding `//wangle/ssl:ssl_config` to the `HTTPClientChannel` BUCK file, ensuring proper SSL configuration. Additionally, the `setHeaders()` method in `HTTPClientChannel.cpp` is **optimized for performance and clarity** by using char literals for single-character operations, enabling move semantics for header processing, and being made static as it doesn't access instance state. These changes improve the efficiency of header manipulation and simplify header extraction within `buildHTTPMessage()`.

This commit **fixes** a critical issue in the **Thrift HTTP client channel** (`HTTPClientChannel`) by ensuring proper validation of HTTP response status codes. Previously, non-2xx responses (e.g., 4xx or 5xx errors) were incorrectly treated as successful, causing confusing deserialization errors when HTML error pages were processed as Thrift responses. The change modifies the `onEOM` method in `HTTPClientChannel.cpp` to throw a `TTransportException` for invalid status codes, aligning its behavior with `SingleRpcChannel`. This **bug fix** prevents misleading protocol errors and makes **Thrift client-side communication** more robust, also simplifying error messages for empty HTTP responses.

This commit **refactors** the **Thrift C++ transport library** by updating the `eraseReadHeader` method within the `THeader` class. The parameter type for `eraseReadHeader` has been changed from `const std::string&` to `std::string_view` in both its declaration and implementation. This **performance optimization** ensures consistency with other `THeader` methods and **avoids unnecessary string copies**, leading to **improved efficiency** when removing headers from a Thrift message.

This commit **refactors** and **optimizes** the **Thrift `HeaderChannel`** to improve efficiency and code clarity in its compression configuration handling. It introduces **move semantics** in `setDesiredCompressionConfig` within `THeader.h` to avoid unnecessary data copies, enhancing performance. Additionally, methods like `preprocessHeader` and `getPersistentWriteHeaders` in `HeaderChannel.h` and `HeaderChannel.cpp` are marked as **const-correct** to reflect their non-modifying nature, improving code correctness. Finally, compression codec initialization is simplified using the `emplace()` pattern, contributing to overall code modernization and efficiency within the asynchronous communication layer.

This commit introduces a **refactoring** change to the **`folly::io::async::AsyncSSLSocket`** component, specifically within `folly/io/async/AsyncSSLSocket.h`. It updates the `AsyncSSLSocket`'s constructor and setter methods to accept `std::shared_ptr<const CertificateIdentityVerifier>` instead of a non-const shared pointer. This **improves const-correctness** by explicitly declaring that the underlying `CertificateIdentityVerifier` instance will only have its `const` methods called, preventing unintended modifications. The change enhances code clarity and robustness within the **asynchronous SSL socket handling** subsystem without altering its functional behavior.

This commit **fixes a bug** in the **Thrift C++2 asynchronous client library's `ReconnectingRequestChannel`** where `folly::RequestContext` was lost for requests queued during channel reconnection. Previously, requests processed via methods like `sendRequestResponse` or `sendRequestStream` would lose their context if the channel was temporarily unavailable. The change now explicitly captures and restores the `RequestContext` when requests are enqueued and subsequently executed, ensuring that **all queued requests maintain their original execution context**. This **bug fix** prevents potential issues where downstream services rely on context information that would otherwise be missing, improving the reliability of asynchronous request handling.

This commit introduces a **new capability** to the **Thrift C++2 asynchronous client** by enhancing its **HTTP transaction observation mechanism**. It modifies the `HTTPTransactionObserverCreator` interface, specifically its `create` virtual method, to now accept both the `HTTPTransaction` object and `RpcOptions` as parameters. The `sendRequest_` method in `HTTPClientChannel.cpp` is updated to pass these objects to the observer creator. This change ensures that **HTTP transaction observers** receive comprehensive context, including connection information, flow control state, timing details, and RPC metadata like logging and timeout settings, at their creation time. Consequently, this significantly improves **observability and debugging capabilities** for HTTP-based RPCs by providing richer data to monitoring tools.

This commit introduces a significant **refactoring** to the **Thrift C++2 HTTP client channel's observability mechanism**, converting the `TransactionObserverCreator` from a `folly::Function` to a dedicated `HTTPTransactionObserverCreator` **interface**. This **enhancement** allows for more sophisticated and flexible transaction observation patterns within `HTTPClientChannel`, enabling custom implementations to manage state, pool observers, or conditionally create them. The change primarily affects `thrift/lib/cpp2/async/HTTPClientChannel.h` and `thrift/lib/cpp2/async/HTTPClientChannel.cpp`, updating the `setTransactionObserverCreator` method and `sendRequest_` to utilize this new interface. Furthermore, a **safety improvement** was added to prevent the attachment of null observers, ensuring more robust operation of the channel.

This commit introduces a **new capability** to the **Thrift C++ library**, allowing clients to explicitly control the HTTP session write buffer limit through the `HTTPClientChannel`. By adding the `setSessionWriteBufferLimit` method, this **feature enhancement** addresses performance bottlenecks where the default 65536-byte buffer was insufficient for large **Thrift requests**, such as those containing images or other substantial payloads. This enables users to tune the buffer size, effectively mitigating transfer pauses and improving the efficiency of data transmission for specific workloads.

This commit introduces a **new capability** to the **`HTTPClientChannel`** within the `thrift/lib/cpp2/async` library. It adds the `setTransactionObserverCreator` method, allowing users to configure a custom function that creates and attaches **monitoring observers** to each new HTTP transaction. This enables **per-transaction lifecycle tracking** for enhanced diagnostics and metrics collection, providing granular insights into HTTP request processing. The change involves adding a new type alias and the setter method to `HTTPClientChannel.h` and integrating the observer attachment logic into `sendRequest_` within `HTTPClientChannel.cpp`, all without affecting existing behavior when not explicitly configured.

This commit introduces a **new capability** to the **`CertificateIdentityVerifier`** interface by adding a virtual `verifyContext()` method, which is invoked for **each certificate in the chain** during the SSL handshake. This method provides implementers with direct access to OpenSSL's `X509_STORE_CTX`, enabling more flexible and advanced custom certificate chain validation beyond just the leaf certificate. The `AsyncSSLSocket::sslVerifyCallback()` is updated to call this new hook, allowing implementers to inspect the entire chain, override OpenSSL's verification decisions, and perform policy-based validation at different depths. This enhancement significantly improves the customizability of SSL certificate verification within the **`folly::io::async`** module, with comprehensive test updates to reflect the new verification flow.

This commit **refactors** the **`HTTPClientChannel`** module by moving the `HTTPTransactionCallback` class from its header file into an anonymous namespace within `HTTPClientChannel.cpp`. This change **improves encapsulation** by making `HTTPTransactionCallback` a true implementation detail, no longer exposed in the public interface. The primary benefits are **reduced header dependencies** and **faster compilation** for any code including `HTTPClientChannel.h`, as changes to the callback no longer trigger widespread recompilation. Additionally, the public interface of `HTTPClientChannel` becomes cleaner and more focused.

This commit introduces a **new capability** to configure the default timeout for the HTTP session's wheel timer within the **HTTPClientChannel**. Previously hardcoded to 500ms, this value can now be set via a `sessionDefaultTimeout` parameter in both the `newHTTP2Channel()` factory method and the `HTTPClientChannel` constructor. This allows callers to customize the default timeout for various HTTP operations during session creation, providing greater flexibility for different use cases. The change also **refactors** `kDefaultTransactionTimeout` to an `inline constexpr` for improved code hygiene, while maintaining backward compatibility with the default 500ms timeout.

This commit performs a **dependency cleanup** within the **thrift HTTP client channel implementation**. It **removes the unused dependency** on `proxygen/lib/http:client` and specifically `HTTPConnector` from `HTTPClientChannel`, as the channel's logic does not utilize any symbols from it. This **refactoring** involves deleting the corresponding `#include` statements in `HTTPClientChannel.h` and updating BUCK build files to remove the `//proxygen/lib/http:client` dependency. The change also prunes other unnecessary includes like `deque` and `unordered_map` from the xplat version, ultimately **reducing build dependencies** and **improving build times** for the affected components.

This commit delivers a **bug fix** to enhance the stability of the **Thrift C++2 transport core** by preventing server crashes when processing malformed HTTP2 thrift requests. The `EnvelopeUtil::stripEnvelope` function, responsible for handling thrift envelopes, was updated to catch `std::exception` instead of only `TException`. This change addresses an issue where `folly::Cursor` could throw `std::out_of_range` for invalid binary data, which was not previously handled, leading to server termination. By catching the broader `std::exception`, the system now gracefully handles all standard exceptions, returning `folly::none` and significantly improving the **robustness of thrift services** against malformed input.

This commit introduces a **new capability** to the **`ScopedServerInterfaceThread` utility** by extending `newStickyClient()` to accept an optional protocol parameter. This **enhancement** allows test code to specify protocols other than the previously hardcoded `T_BINARY_PROTOCOL`, making it consistent with the existing `newClient()` method. The change primarily impacts **testing scenarios**, enabling the creation of sticky clients with protocols like `T_COMPACT_PROTOCOL` for HTTP/2 testing that requires custom headers. The default protocol remains `T_BINARY_PROTOCOL` to ensure backward compatibility.

This commit introduces a **new `poll()` method** to the **`folly::coro::SharedPromise`** component, providing a **non-blocking, synchronous way** to check and retrieve a promise's result. This **new feature** allows consumers to opportunistically query if a shared promise has been fulfilled, returning an `std::optional<TryType>` with the value or an empty optional if not ready, without suspending the current execution. It enhances the `SharedPromise` API by complementing its asynchronous `getFuture()` and `getSemiFuture()` methods, enabling more flexible control flow in concurrent scenarios while ensuring **thread-safe access** to the promise's state.