Ensure handleError doesn't throw

Summary:
If `handleError` is invoked with a supplied `AlertDescription`, it will attempt
to use the current write record layer to encode an alert to be sent to the peer.

The write record layer `write()` may throw. This is problematic since
`handleError` can be invoked itself in the catch handler (e.g. during
`processSocketData`). Uncaught exceptions will then propagate out of the state
machine into the I/O layer, which may be invoked under a noexcept context.

This diff catches exceptions that may be thrown by the write record layer during
`handleError`, skipping writing the alert if one is thrown.

Reviewed By: knekritz

Differential Revision: D66526269

fbshipit-source-id: 2e0ebb91767d3543d5f0aa10eaeff3e38ba821a2

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) {

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<MutateState, ReportError>(actions);
+  };
+  EXPECT_NO_THROW(shouldNotThrow());
+}
 } // namespace test
 } // namespace client
 } // namespace fizz

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) {

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<MutateState, ReportError>(actions);
+  };
+  EXPECT_NO_THROW(shouldNotThrow());
+}
 } // namespace test
 } // namespace server
 } // namespace fizz