diff --git a/fizz/client/ClientProtocol.cpp b/fizz/client/ClientProtocol.cpp index 4fd11e977c..df7669ae1e 100644 --- a/fizz/client/ClientProtocol.cpp +++ b/fizz/client/ClientProtocol.cpp @@ -286,15 +286,22 @@ Actions handleError( newState.writeRecordLayer() = nullptr; newState.readRecordLayer() = nullptr; }); + + Actions actions; + actions.emplace_back(std::move(transition)); + if (alertDesc && state.writeRecordLayer()) { - Alert alert(*alertDesc); - WriteToSocket write; - write.contents.emplace_back( - state.writeRecordLayer()->writeAlert(std::move(alert))); - return actions(std::move(transition), std::move(write), std::move(error)); - } else { - return actions(std::move(transition), std::move(error)); + try { + Alert alert(*alertDesc); + WriteToSocket write; + write.contents.emplace_back( + state.writeRecordLayer()->writeAlert(std::move(alert))); + actions.emplace_back(std::move(write)); + } catch (...) { + } } + actions.emplace_back(std::move(error)); + return actions; } Actions handleAppCloseImmediate(const State& state) { diff --git a/fizz/client/test/ClientProtocolTest.cpp b/fizz/client/test/ClientProtocolTest.cpp index d9baf56ac9..012f6b0e89 100644 --- a/fizz/client/test/ClientProtocolTest.cpp +++ b/fizz/client/test/ClientProtocolTest.cpp @@ -5918,6 +5918,20 @@ TEST_F(ClientProtocolTest, TestPskWithoutCerts) { EXPECT_EQ(state_.clientAuthRequested().value(), ClientAuthType::NotRequested); } +TEST_F(ClientProtocolTest, HandleErrorDoesntThrow) { + setupAcceptingData(); + EXPECT_CALL(*mockWrite_, _write(_, _)) + .WillOnce(Throw(std::runtime_error("write error"))); + + ReportError error("some error"); + AlertDescription ad = AlertDescription::internal_error; + + auto shouldNotThrow = [&] { + auto actions = detail::handleError(state_, std::move(error), ad); + expectActions(actions); + }; + EXPECT_NO_THROW(shouldNotThrow()); +} } // namespace test } // namespace client } // namespace fizz diff --git a/fizz/server/ServerProtocol.cpp b/fizz/server/ServerProtocol.cpp index 66e0600886..ccf1217242 100644 --- a/fizz/server/ServerProtocol.cpp +++ b/fizz/server/ServerProtocol.cpp @@ -286,15 +286,22 @@ Actions handleError( newState.writeRecordLayer() = nullptr; newState.readRecordLayer() = nullptr; }); + + Actions actions; + actions.emplace_back(std::move(transition)); + if (alertDesc && state.writeRecordLayer()) { - Alert alert(*alertDesc); - WriteToSocket write; - write.contents.emplace_back( - state.writeRecordLayer()->writeAlert(std::move(alert))); - return actions(std::move(transition), std::move(write), std::move(error)); - } else { - return actions(std::move(transition), std::move(error)); + try { + Alert alert(*alertDesc); + WriteToSocket write; + write.contents.emplace_back( + state.writeRecordLayer()->writeAlert(std::move(alert))); + actions.emplace_back(std::move(write)); + } catch (...) { + } } + actions.emplace_back(std::move(error)); + return actions; } Actions handleAppCloseImmediate(const State& state) { diff --git a/fizz/server/test/ServerProtocolTest.cpp b/fizz/server/test/ServerProtocolTest.cpp index cb3474f5c2..1d0fb9ce66 100644 --- a/fizz/server/test/ServerProtocolTest.cpp +++ b/fizz/server/test/ServerProtocolTest.cpp @@ -6743,6 +6743,21 @@ TEST_F(ServerProtocolTest, AsyncKeyExchangeTest) { EXPECT_TRUE( std::all_of(begin(cr), end(cr), [](auto c) { return c == 0x44; })); } + +TEST_F(ServerProtocolTest, HandleErrorDoesntThrow) { + setUpAcceptingData(); + EXPECT_CALL(*appWrite_, _write(_, _)) + .WillOnce(Throw(std::runtime_error("write error"))); + + ReportError error("some error"); + AlertDescription ad = AlertDescription::internal_error; + + auto shouldNotThrow = [&] { + auto actions = detail::handleError(state_, std::move(error), ad); + expectActions(actions); + }; + EXPECT_NO_THROW(shouldNotThrow()); +} } // namespace test } // namespace server } // namespace fizz