1
0
mirror of https://github.com/facebookincubator/mvfst.git synced 2025-08-08 09:42:06 +03:00

Report exception strings via onConnectionError.

Summary: It's not a great practice to leak the excpetion string via a conn close, but it is useful for the app to be able to report what the exception string was.

Reviewed By: yangchi

Differential Revision: D20628591

fbshipit-source-id: bf6eb5f33f516cec0034caed53da998643fcc120
This commit is contained in:
Matt Joras
2020-03-27 15:03:12 -07:00
committed by Facebook GitHub Bot
parent d7da8247e8
commit 19e1c14afd
3 changed files with 52 additions and 1 deletions

View File

@@ -390,8 +390,11 @@ void QuicTransportBase::closeImpl(
if (noError) { if (noError) {
connCallback_->onConnectionEnd(); connCallback_->onConnectionEnd();
} else { } else {
std::string closeStr = exceptionCloseWhat_
? std::move(exceptionCloseWhat_.value())
: cancelCode.second.str();
connCallback_->onConnectionError( connCallback_->onConnectionError(
std::make_pair(cancelCode.first, cancelCode.second.str())); std::make_pair(cancelCode.first, closeStr));
} }
} }
@@ -1279,16 +1282,19 @@ folly::Expected<std::pair<Buf, bool>, LocalErrorCode> QuicTransportBase::read(
return folly::makeExpected<LocalErrorCode>(std::move(result)); return folly::makeExpected<LocalErrorCode>(std::move(result));
} catch (const QuicTransportException& ex) { } catch (const QuicTransportException& ex) {
VLOG(4) << "read() error " << ex.what() << " " << *this; VLOG(4) << "read() error " << ex.what() << " " << *this;
exceptionCloseWhat_ = ex.what();
closeImpl(std::make_pair( closeImpl(std::make_pair(
QuicErrorCode(ex.errorCode()), std::string("read() error"))); QuicErrorCode(ex.errorCode()), std::string("read() error")));
return folly::makeUnexpected(LocalErrorCode::TRANSPORT_ERROR); return folly::makeUnexpected(LocalErrorCode::TRANSPORT_ERROR);
} catch (const QuicInternalException& ex) { } catch (const QuicInternalException& ex) {
VLOG(4) << __func__ << " " << ex.what() << " " << *this; VLOG(4) << __func__ << " " << ex.what() << " " << *this;
exceptionCloseWhat_ = ex.what();
closeImpl(std::make_pair( closeImpl(std::make_pair(
QuicErrorCode(ex.errorCode()), std::string("read() error"))); QuicErrorCode(ex.errorCode()), std::string("read() error")));
return folly::makeUnexpected(ex.errorCode()); return folly::makeUnexpected(ex.errorCode());
} catch (const std::exception& ex) { } catch (const std::exception& ex) {
VLOG(4) << "read() error " << ex.what() << " " << *this; VLOG(4) << "read() error " << ex.what() << " " << *this;
exceptionCloseWhat_ = ex.what();
closeImpl(std::make_pair( closeImpl(std::make_pair(
QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), QuicErrorCode(TransportErrorCode::INTERNAL_ERROR),
std::string("read() error"))); std::string("read() error")));
@@ -1405,17 +1411,20 @@ folly::
return folly::makeExpected<ConsumeError>(folly::Unit()); return folly::makeExpected<ConsumeError>(folly::Unit());
} catch (const QuicTransportException& ex) { } catch (const QuicTransportException& ex) {
VLOG(4) << "consume() error " << ex.what() << " " << *this; VLOG(4) << "consume() error " << ex.what() << " " << *this;
exceptionCloseWhat_ = ex.what();
closeImpl(std::make_pair( closeImpl(std::make_pair(
QuicErrorCode(ex.errorCode()), std::string("consume() error"))); QuicErrorCode(ex.errorCode()), std::string("consume() error")));
return folly::makeUnexpected( return folly::makeUnexpected(
ConsumeError{LocalErrorCode::TRANSPORT_ERROR, readOffset}); ConsumeError{LocalErrorCode::TRANSPORT_ERROR, readOffset});
} catch (const QuicInternalException& ex) { } catch (const QuicInternalException& ex) {
VLOG(4) << __func__ << " " << ex.what() << " " << *this; VLOG(4) << __func__ << " " << ex.what() << " " << *this;
exceptionCloseWhat_ = ex.what();
closeImpl(std::make_pair( closeImpl(std::make_pair(
QuicErrorCode(ex.errorCode()), std::string("consume() error"))); QuicErrorCode(ex.errorCode()), std::string("consume() error")));
return folly::makeUnexpected(ConsumeError{ex.errorCode(), readOffset}); return folly::makeUnexpected(ConsumeError{ex.errorCode(), readOffset});
} catch (const std::exception& ex) { } catch (const std::exception& ex) {
VLOG(4) << "consume() error " << ex.what() << " " << *this; VLOG(4) << "consume() error " << ex.what() << " " << *this;
exceptionCloseWhat_ = ex.what();
closeImpl(std::make_pair( closeImpl(std::make_pair(
QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), QuicErrorCode(TransportErrorCode::INTERNAL_ERROR),
std::string("consume() error"))); std::string("consume() error")));
@@ -1635,18 +1644,22 @@ void QuicTransportBase::onNetworkData(
} }
} catch (const QuicTransportException& ex) { } catch (const QuicTransportException& ex) {
VLOG(4) << __func__ << " " << ex.what() << " " << *this; VLOG(4) << __func__ << " " << ex.what() << " " << *this;
exceptionCloseWhat_ = ex.what();
return closeImpl( return closeImpl(
std::make_pair(QuicErrorCode(ex.errorCode()), std::string(ex.what()))); std::make_pair(QuicErrorCode(ex.errorCode()), std::string(ex.what())));
} catch (const QuicInternalException& ex) { } catch (const QuicInternalException& ex) {
VLOG(4) << __func__ << " " << ex.what() << " " << *this; VLOG(4) << __func__ << " " << ex.what() << " " << *this;
exceptionCloseWhat_ = ex.what();
return closeImpl( return closeImpl(
std::make_pair(QuicErrorCode(ex.errorCode()), std::string(ex.what()))); std::make_pair(QuicErrorCode(ex.errorCode()), std::string(ex.what())));
} catch (const QuicApplicationException& ex) { } catch (const QuicApplicationException& ex) {
VLOG(4) << __func__ << " " << ex.what() << " " << *this; VLOG(4) << __func__ << " " << ex.what() << " " << *this;
exceptionCloseWhat_ = ex.what();
return closeImpl( return closeImpl(
std::make_pair(QuicErrorCode(ex.errorCode()), std::string(ex.what()))); std::make_pair(QuicErrorCode(ex.errorCode()), std::string(ex.what())));
} catch (const std::exception& ex) { } catch (const std::exception& ex) {
VLOG(4) << __func__ << " " << ex.what() << " " << *this; VLOG(4) << __func__ << " " << ex.what() << " " << *this;
exceptionCloseWhat_ = ex.what();
return closeImpl(std::make_pair( return closeImpl(std::make_pair(
QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), QuicErrorCode(TransportErrorCode::INTERNAL_ERROR),
std::string("error onNetworkData()"))); std::string("error onNetworkData()")));
@@ -1868,18 +1881,21 @@ QuicSocket::WriteResult QuicTransportBase::writeChain(
} catch (const QuicTransportException& ex) { } catch (const QuicTransportException& ex) {
VLOG(4) << __func__ << " streamId=" << id << " " << ex.what() << " " VLOG(4) << __func__ << " streamId=" << id << " " << ex.what() << " "
<< *this; << *this;
exceptionCloseWhat_ = ex.what();
closeImpl(std::make_pair( closeImpl(std::make_pair(
QuicErrorCode(ex.errorCode()), std::string("writeChain() error"))); QuicErrorCode(ex.errorCode()), std::string("writeChain() error")));
return folly::makeUnexpected(LocalErrorCode::TRANSPORT_ERROR); return folly::makeUnexpected(LocalErrorCode::TRANSPORT_ERROR);
} catch (const QuicInternalException& ex) { } catch (const QuicInternalException& ex) {
VLOG(4) << __func__ << " streamId=" << id << " " << ex.what() << " " VLOG(4) << __func__ << " streamId=" << id << " " << ex.what() << " "
<< *this; << *this;
exceptionCloseWhat_ = ex.what();
closeImpl(std::make_pair( closeImpl(std::make_pair(
QuicErrorCode(ex.errorCode()), std::string("writeChain() error"))); QuicErrorCode(ex.errorCode()), std::string("writeChain() error")));
return folly::makeUnexpected(ex.errorCode()); return folly::makeUnexpected(ex.errorCode());
} catch (const std::exception& ex) { } catch (const std::exception& ex) {
VLOG(4) << __func__ << " streamId=" << id << " " << ex.what() << " " VLOG(4) << __func__ << " streamId=" << id << " " << ex.what() << " "
<< *this; << *this;
exceptionCloseWhat_ = ex.what();
closeImpl(std::make_pair( closeImpl(std::make_pair(
QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), QuicErrorCode(TransportErrorCode::INTERNAL_ERROR),
std::string("writeChain() error"))); std::string("writeChain() error")));
@@ -1994,18 +2010,21 @@ folly::Expected<folly::Unit, LocalErrorCode> QuicTransportBase::resetStream(
} catch (const QuicTransportException& ex) { } catch (const QuicTransportException& ex) {
VLOG(4) << __func__ << " streamId=" << id << " " << ex.what() << " " VLOG(4) << __func__ << " streamId=" << id << " " << ex.what() << " "
<< *this; << *this;
exceptionCloseWhat_ = ex.what();
closeImpl(std::make_pair( closeImpl(std::make_pair(
QuicErrorCode(ex.errorCode()), std::string("resetStream() error"))); QuicErrorCode(ex.errorCode()), std::string("resetStream() error")));
return folly::makeUnexpected(LocalErrorCode::TRANSPORT_ERROR); return folly::makeUnexpected(LocalErrorCode::TRANSPORT_ERROR);
} catch (const QuicInternalException& ex) { } catch (const QuicInternalException& ex) {
VLOG(4) << __func__ << " streamId=" << id << " " << ex.what() << " " VLOG(4) << __func__ << " streamId=" << id << " " << ex.what() << " "
<< *this; << *this;
exceptionCloseWhat_ = ex.what();
closeImpl(std::make_pair( closeImpl(std::make_pair(
QuicErrorCode(ex.errorCode()), std::string("resetStream() error"))); QuicErrorCode(ex.errorCode()), std::string("resetStream() error")));
return folly::makeUnexpected(ex.errorCode()); return folly::makeUnexpected(ex.errorCode());
} catch (const std::exception& ex) { } catch (const std::exception& ex) {
VLOG(4) << __func__ << " streamId=" << id << " " << ex.what() << " " VLOG(4) << __func__ << " streamId=" << id << " " << ex.what() << " "
<< *this; << *this;
exceptionCloseWhat_ = ex.what();
closeImpl(std::make_pair( closeImpl(std::make_pair(
QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), QuicErrorCode(TransportErrorCode::INTERNAL_ERROR),
std::string("resetStream() error"))); std::string("resetStream() error")));
@@ -2103,16 +2122,19 @@ void QuicTransportBase::lossTimeoutExpired() noexcept {
pacedWriteDataToSocket(false); pacedWriteDataToSocket(false);
} catch (const QuicTransportException& ex) { } catch (const QuicTransportException& ex) {
VLOG(4) << __func__ << " " << ex.what() << " " << *this; VLOG(4) << __func__ << " " << ex.what() << " " << *this;
exceptionCloseWhat_ = ex.what();
closeImpl(std::make_pair( closeImpl(std::make_pair(
QuicErrorCode(ex.errorCode()), QuicErrorCode(ex.errorCode()),
std::string("lossTimeoutExpired() error"))); std::string("lossTimeoutExpired() error")));
} catch (const QuicInternalException& ex) { } catch (const QuicInternalException& ex) {
VLOG(4) << __func__ << " " << ex.what() << " " << *this; VLOG(4) << __func__ << " " << ex.what() << " " << *this;
exceptionCloseWhat_ = ex.what();
closeImpl(std::make_pair( closeImpl(std::make_pair(
QuicErrorCode(ex.errorCode()), QuicErrorCode(ex.errorCode()),
std::string("lossTimeoutExpired() error"))); std::string("lossTimeoutExpired() error")));
} catch (const std::exception& ex) { } catch (const std::exception& ex) {
VLOG(4) << __func__ << " " << ex.what() << " " << *this; VLOG(4) << __func__ << " " << ex.what() << " " << *this;
exceptionCloseWhat_ = ex.what();
closeImpl(std::make_pair( closeImpl(std::make_pair(
QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), QuicErrorCode(TransportErrorCode::INTERNAL_ERROR),
std::string("lossTimeoutExpired() error"))); std::string("lossTimeoutExpired() error")));
@@ -2376,16 +2398,19 @@ void QuicTransportBase::writeSocketDataAndCatch() {
writeSocketData(); writeSocketData();
} catch (const QuicTransportException& ex) { } catch (const QuicTransportException& ex) {
VLOG(4) << __func__ << ex.what() << " " << *this; VLOG(4) << __func__ << ex.what() << " " << *this;
exceptionCloseWhat_ = ex.what();
closeImpl(std::make_pair( closeImpl(std::make_pair(
QuicErrorCode(ex.errorCode()), QuicErrorCode(ex.errorCode()),
std::string("writeSocketDataAndCatch() error"))); std::string("writeSocketDataAndCatch() error")));
} catch (const QuicInternalException& ex) { } catch (const QuicInternalException& ex) {
VLOG(4) << __func__ << ex.what() << " " << *this; VLOG(4) << __func__ << ex.what() << " " << *this;
exceptionCloseWhat_ = ex.what();
closeImpl(std::make_pair( closeImpl(std::make_pair(
QuicErrorCode(ex.errorCode()), QuicErrorCode(ex.errorCode()),
std::string("writeSocketDataAndCatch() error"))); std::string("writeSocketDataAndCatch() error")));
} catch (const std::exception& ex) { } catch (const std::exception& ex) {
VLOG(4) << __func__ << " error=" << ex.what() << " " << *this; VLOG(4) << __func__ << " error=" << ex.what() << " " << *this;
exceptionCloseWhat_ = ex.what();
closeImpl(std::make_pair( closeImpl(std::make_pair(
QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), QuicErrorCode(TransportErrorCode::INTERNAL_ERROR),
std::string("writeSocketDataAndCatch() error"))); std::string("writeSocketDataAndCatch() error")));

View File

@@ -627,6 +627,8 @@ class QuicTransportBase : public QuicSocket {
folly::SocketAddress localFallbackAddress; folly::SocketAddress localFallbackAddress;
// CongestionController factory // CongestionController factory
std::shared_ptr<CongestionControllerFactory> ccFactory_{nullptr}; std::shared_ptr<CongestionControllerFactory> ccFactory_{nullptr};
folly::Optional<std::string> exceptionCloseWhat_;
}; };
std::ostream& operator<<(std::ostream& os, const QuicTransportBase& qt); std::ostream& operator<<(std::ostream& os, const QuicTransportBase& qt);

View File

@@ -1030,6 +1030,30 @@ TEST_F(QuicTransportImplTest, ConnectionErrorOnWrite) {
QuicErrorCode(LocalErrorCode::CONNECTION_ABANDONED)); QuicErrorCode(LocalErrorCode::CONNECTION_ABANDONED));
} }
TEST_F(QuicTransportImplTest, ConnectionErrorUnhandledException) {
transport->transportConn->oneRttWriteCipher = test::createNoOpAead();
auto stream = transport->createBidirectionalStream().value();
EXPECT_CALL(
connCallback,
onConnectionError(std::make_pair(
QuicErrorCode(TransportErrorCode::INTERNAL_ERROR),
std::string("Well there's your problem"))));
EXPECT_CALL(*socketPtr, write(_, _)).WillOnce(Invoke([](auto&, auto&) {
throw std::runtime_error("Well there's your problem");
return 0;
}));
QuicSocket::WriteResult result = transport->writeChain(
stream, folly::IOBuf::copyBuffer("Hey"), true, false, nullptr);
transport->addDataToStream(
stream, StreamBuffer(folly::IOBuf::copyBuffer("Data"), 0));
evb->loopOnce();
EXPECT_TRUE(transport->isClosed());
EXPECT_EQ(
transport->getConnectionError(),
QuicErrorCode(TransportErrorCode::INTERNAL_ERROR));
}
TEST_F(QuicTransportImplTest, LossTimeoutNoLessThanTickInterval) { TEST_F(QuicTransportImplTest, LossTimeoutNoLessThanTickInterval) {
auto tickInterval = evb->timer().getTickInterval(); auto tickInterval = evb->timer().getTickInterval();
transport->scheduleLossTimeout(tickInterval - 1ms); transport->scheduleLossTimeout(tickInterval - 1ms);