Skip to content

Commit 799f6f2

Browse files
Raj Shahfacebook-github-bot
authored andcommitted
Preserve specific error types for ingress errors (#591)
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
1 parent 8b87678 commit 799f6f2

2 files changed

Lines changed: 17 additions & 1 deletion

File tree

proxygen/httpserver/RequestHandlerAdaptor.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ void RequestHandlerAdaptor::onError(const HTTPException& error) noexcept {
116116
.sendWithEOM();
117117
}
118118
} else if (error.getDirection() == HTTPException::Direction::INGRESS) {
119-
setError(kErrorRead);
119+
setError(error.hasProxygenError() ? error.getProxygenError() : kErrorRead);
120120

121121
if (!txn_->canSendHeaders()) {
122122
sendAbort(folly::none);

proxygen/httpserver/tests/RequestHandlerAdaptorTest.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,22 @@ TEST(RequestHandlerAdaptorTest, onStreamAbortError) {
9797
txn.onError(ex);
9898
}
9999

100+
TEST(RequestHandlerAdaptorTest, onIngressErrorPreservesSpecificError) {
101+
NiceMock<MockRequestHandler> requestHandler_;
102+
auto adaptor = new RequestHandlerAdaptor(&requestHandler_);
103+
NiceMock<MockHTTPTransactionTransport> transport;
104+
HTTP2PriorityQueue egressQueue;
105+
HTTPTransaction txn(
106+
TransportDirection::DOWNSTREAM, 1, 1, transport, egressQueue);
107+
txn.setHandler(adaptor);
108+
// ingress error with specific error type (e.g., chunk parse error)
109+
HTTPException ex(HTTPException::Direction::INGRESS, "chunk parse error");
110+
ex.setProxygenError(kErrorParseBody);
111+
// expect the specific error type is preserved, not converted to kErrorRead
112+
EXPECT_CALL(requestHandler_, onError(kErrorParseBody));
113+
txn.onError(ex);
114+
}
115+
100116
TEST(RequestHandlerAdaptorTest, onGoaway) {
101117
NiceMock<MockRequestHandler> requestHandler_;
102118
auto adaptor = std::make_shared<RequestHandlerAdaptor>(&requestHandler_);

0 commit comments

Comments
 (0)