From 799f6f2f97e5408cb7195ed9a794d86cd2a7ca8a Mon Sep 17 00:00:00 2001 From: Raj Shah Date: Tue, 10 Feb 2026 18:39:24 -0800 Subject: [PATCH] Preserve specific error types for ingress errors (#591) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Previously, `RequestHandlerAdaptor::onError` would convert ALL ingress errors to `kErrorRead`, losing the specific error type from the HTTP codec. For example, a chunked encoding parse error (`kErrorParseBody`) would be reported as generic `kErrorRead`, making debugging difficult. This change preserves the specific `ProxygenError` from the `HTTPException` when available, falling back to `kErrorRead` only when no specific error is set. This matches the behavior already used for egress errors. Before: Malformed chunked encoding → `kErrorRead` After: Malformed chunked encoding → `kErrorParseBody` For upload service we log the specific proxygen error but all I saw was "READ" making it difficult to figure out what the actual issue was, seeing "ParseBody" would've made the error clear Reviewed By: dddmello Differential Revision: D92352456 --- proxygen/httpserver/RequestHandlerAdaptor.cpp | 2 +- .../tests/RequestHandlerAdaptorTest.cpp | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/proxygen/httpserver/RequestHandlerAdaptor.cpp b/proxygen/httpserver/RequestHandlerAdaptor.cpp index 3f27c38df5..baa29db3a8 100644 --- a/proxygen/httpserver/RequestHandlerAdaptor.cpp +++ b/proxygen/httpserver/RequestHandlerAdaptor.cpp @@ -116,7 +116,7 @@ void RequestHandlerAdaptor::onError(const HTTPException& error) noexcept { .sendWithEOM(); } } else if (error.getDirection() == HTTPException::Direction::INGRESS) { - setError(kErrorRead); + setError(error.hasProxygenError() ? error.getProxygenError() : kErrorRead); if (!txn_->canSendHeaders()) { sendAbort(folly::none); diff --git a/proxygen/httpserver/tests/RequestHandlerAdaptorTest.cpp b/proxygen/httpserver/tests/RequestHandlerAdaptorTest.cpp index 51c581c17b..5fdb646128 100644 --- a/proxygen/httpserver/tests/RequestHandlerAdaptorTest.cpp +++ b/proxygen/httpserver/tests/RequestHandlerAdaptorTest.cpp @@ -97,6 +97,22 @@ TEST(RequestHandlerAdaptorTest, onStreamAbortError) { txn.onError(ex); } +TEST(RequestHandlerAdaptorTest, onIngressErrorPreservesSpecificError) { + NiceMock requestHandler_; + auto adaptor = new RequestHandlerAdaptor(&requestHandler_); + NiceMock transport; + HTTP2PriorityQueue egressQueue; + HTTPTransaction txn( + TransportDirection::DOWNSTREAM, 1, 1, transport, egressQueue); + txn.setHandler(adaptor); + // ingress error with specific error type (e.g., chunk parse error) + HTTPException ex(HTTPException::Direction::INGRESS, "chunk parse error"); + ex.setProxygenError(kErrorParseBody); + // expect the specific error type is preserved, not converted to kErrorRead + EXPECT_CALL(requestHandler_, onError(kErrorParseBody)); + txn.onError(ex); +} + TEST(RequestHandlerAdaptorTest, onGoaway) { NiceMock requestHandler_; auto adaptor = std::make_shared(&requestHandler_);