Allow NO_ERROR in ErrorCode2HTTPErrorCode/HTTP3ErrorCode2HTTPErrorCode; add regression tests

Summary:
Problem:
- Inbound H2 RST_STREAM(NO_ERROR) could crash via adaptor path: HTTPTransactionAdaptorSource::onError -> HTTPException2HTTPErrorCode -> ErrorCode2HTTPErrorCode(NO_ERROR) (pre-fix had a CHECK).
- NO_ERROR is valid on the wire per H2/H3; session layer also normalizes H2 NO_ERROR to CANCEL for consumers.

Fix:
- Allow NO_ERROR in ErrorCode2HTTPErrorCode and HTTP3ErrorCode2HTTPErrorCode (remove CHECK), preserving mapping semantics.

Tests added:
- H2DownstreamSessionTest.ResetStreamNoError: client sends RST_STREAM/NO_ERROR; verify source sees CANCEL and message suffix "details=received RST_STREAM from peer".
- HTTPTransactionAdaptorSourceTests.IngressNoErrorAbortNoCrash: drive adaptor onError with codec status NO_ERROR; verify error surfaced and no crash.

Repro (pre-fix):
- Reintroducing XCHECK(ec != ErrorCode::NO_ERROR) causes adaptor test to abort, confirming the crash path.

Notes:
- This aligns behavior with RFCs and avoids fatal checks for legitimate peer behavior.
 ---
> Generated by [Confucius Code Assist (CCA)](https://www.internalfb.com/wiki/Confucius/Analect/Shared_Analects/Confucius_Code_Assist_(CCA)/)
[Session](https://www.internalfb.com/confucius?session_id=c0ea225c-85ea-11f0-b2a3-fd678517c893&tab=Chat), [Trace](https://www.internalfb.com/confucius?session_id=c0ea225c-85ea-11f0-b2a3-fd678517c893&tab=Trace)

Reviewed By: HsiehYuho

Differential Revision: D81392253

fbshipit-source-id: 9d01036e267e1c1577c4f59c0e2c40a1d5e787f0

Author: Matt Joras

proxygen/lib/http/coro/HTTPError.cpp

@@ -208,8 +208,7 @@ HTTP3::ErrorCode HTTPErrorCode2HTTP3ErrorCode(HTTPErrorCode ec,
 // with the same meaning and return only one code
 // Convert an H2 error code read off the wire to an HTTPErrorCode
 HTTPErrorCode ErrorCode2HTTPErrorCode(ErrorCode ec) {
-  XCHECK(ec != ErrorCode::NO_ERROR);
-  if (ec >= ErrorCode::PROTOCOL_ERROR && ec <= ErrorCode::HTTP_1_1_REQUIRED) {
+  if (ec >= ErrorCode::NO_ERROR && ec <= ErrorCode::HTTP_1_1_REQUIRED) {
     return HTTPErrorCode(ec);
   }
   // Any unknown custom error codes are mapped to PROTOCOL_ERROR here.
@@ -218,8 +217,7 @@ HTTPErrorCode ErrorCode2HTTPErrorCode(ErrorCode ec) {
 
 // Convert an H3 error code read off the wire to an HTTPErrorCode
 HTTPErrorCode HTTP3ErrorCode2HTTPErrorCode(HTTP3::ErrorCode ec) {
-  XCHECK(ec != HTTP3::ErrorCode::HTTP_NO_ERROR);
-  if ((ec >= HTTP3::ErrorCode::HTTP_GENERAL_PROTOCOL_ERROR &&
+  if ((ec >= HTTP3::ErrorCode::HTTP_NO_ERROR &&
        ec <= HTTP3::ErrorCode::HTTP_VERSION_FALLBACK) ||
       (ec >= HTTP3::ErrorCode::HTTP_QPACK_DECOMPRESSION_FAILED &&
        ec <= HTTP3::ErrorCode::HTTP_QPACK_DECODER_STREAM_ERROR)) {

proxygen/lib/http/coro/test/HTTPDownstreamCoroSessionTests.cpp

@@ -1977,6 +1977,38 @@ TEST_P(HTTPDownstreamSessionTest, HoldContext) {
   }
 }
 
+TEST_P(H2DownstreamSessionTest, ResetStreamNoError) {
+  // Client sends RST_STREAM with NO_ERROR while server has not finished
+  // consuming ingress. This used to trigger a CHECK in
+  // ErrorCode2HTTPErrorCode(NO_ERROR) prior to the fix.
+  sendRequest("/", nullptr, /*eom=*/false, /*eof=*/false);
+
+  auto handler =
+      addSimpleStrictHandler([this](folly::EventBase * /*evb*/,
+                                    HTTPSessionContextPtr /*ctx*/,
+                                    HTTPSourceHolder requestSource)
+                                 -> folly::coro::Task<HTTPSourceHolder> {
+        auto id = *requestSource.getStreamID();
+        co_await expectRequest(
+            requestSource, HTTPMethod::GET, "/", /*eom=*/false);
+        // Inbound RST_STREAM with NO_ERROR from the peer.
+        resetStream(id, ErrorCode::NO_ERROR);
+        // Some transports in tests expect an extra read tick to drain.
+        transport_->addReadEvent(writeBuf_.move(), /*eom*/ true);
+        // Expect the body read to surface a CANCEL error (normalized).
+        auto bodyEvent =
+            co_await co_awaitTry(readBodyEventNoSuspend(requestSource));
+        EXPECT_TRUE(bodyEvent.hasException());
+        auto err = getHTTPError(bodyEvent);
+        EXPECT_TRUE(isCancelled(err.code));
+        EXPECT_TRUE(err.msg.ends_with("details=received RST_STREAM from peer"));
+        co_return nullptr;
+      });
+
+  evb_.loop();
+  parseOutput();
+}
+
 // H2 only, H1 needs to test with transport write error
 TEST_P(H2QDownstreamSessionTest, StopReadingDefault500Source) {
   // Stop reading as soon as headers are received without returning source. This

proxygen/lib/http/coro/test/HTTPTransactionAdaptorSourceTest.cpp

@@ -408,6 +408,34 @@ CO_TEST_P_X(HTTPTransactionAdaptorSourceTests, DownstreamErrors) {
   adaptor_->setEgressSource(std::move(response));
 }
 
+CO_TEST_P_X(HTTPTransactionAdaptorSourceTests, IngressNoErrorAbortNoCrash) {
+  auto ingressSource = adaptor_->getIngressSource();
+  auto handler = std::make_shared<TestHandler>();
+
+  auto msg = std::make_unique<HTTPMessage>();
+  msg->setMethod(HTTPMethod::GET);
+  msg->setURL("https://www.facebook.com/");
+
+  getHandler()->onHeadersComplete(std::move(msg));
+  getHandler()->onBody(folly::IOBuf::copyBuffer("hello world"));
+  // Do not send EOM to simulate incomplete ingress
+
+  // Simulate an inbound RST_STREAM(NO_ERROR) via codec status on the exception
+  HTTPException ex(HTTPException::Direction::INGRESS_AND_EGRESS,
+                   "rst_stream_no_error");
+  ex.setCodecStatusCode(ErrorCode::NO_ERROR);
+  EXPECT_CALL(*mockTxn_, sendAbort(_)).Times(1);
+  getHandler()->onError(ex);
+
+  // The adaptor will abort the ingress source; consuming should surface an
+  // error
+  auto responseTry = co_await co_awaitTry(folly::coro::detachOnCancel(
+      handler->handleRequest(&evb_, ctx_.acquireKeepAlive(), ingressSource)));
+  EXPECT_TRUE(responseTry.hasException());
+
+  getHandler()->detachTransaction();
+}
+
 INSTANTIATE_TEST_SUITE_P(HTTPTransactionAdaptorSourceTests,
                          HTTPTransactionAdaptorSourceTests,
                          Values(TransportType::TCP),