diff --git a/quic/QuicException.cpp b/quic/QuicException.cpp index 80d239760..d0eda0a87 100644 --- a/quic/QuicException.cpp +++ b/quic/QuicException.cpp @@ -255,26 +255,24 @@ std::string toString(QuicErrorCode code) { folly::assume_unreachable(); } -std::string toString( - const std::pair>& - error) { +std::string toString(const QuicError& error) { std::string err; - switch (error.first.type()) { + switch (error.code.type()) { case QuicErrorCode::Type::ApplicationErrorCode: err = "ApplicationError: " + - toString(*error.first.asApplicationErrorCode()) + ", "; + toString(*error.code.asApplicationErrorCode()) + ", "; break; case QuicErrorCode::Type::LocalErrorCode: err = "LocalError: " + - folly::to(toString(*error.first.asLocalErrorCode())) + + folly::to(toString(*error.code.asLocalErrorCode())) + ", "; break; case QuicErrorCode::Type::TransportErrorCode: - err = "TransportError: " + toString(*error.first.asTransportErrorCode()) + + err = "TransportError: " + toString(*error.code.asTransportErrorCode()) + ", "; } - if (error.second) { - err = folly::to(err, error.second.value()); + if (!error.message.empty()) { + err = folly::to(err, error.message); } return err; } diff --git a/quic/QuicException.h b/quic/QuicException.h index 423696b56..0df9b17e1 100644 --- a/quic/QuicException.h +++ b/quic/QuicException.h @@ -23,6 +23,18 @@ namespace quic { DECLARE_VARIANT_TYPE(QuicErrorCode, QUIC_ERROR_CODE) +struct QuicError { + QuicError(QuicErrorCode codeIn, const std::string& messageIn) + : code(codeIn), message(messageIn) {} + explicit QuicError(QuicErrorCode codeIn) : code(codeIn) {} + bool operator==(const QuicError& other) const { + return code == other.code && message == other.message; + } + + QuicErrorCode code; + std::string message; +}; + class QuicTransportException : public std::runtime_error { public: explicit QuicTransportException( @@ -99,8 +111,7 @@ folly::StringPiece toString(LocalErrorCode code); // copy on return here as well. std::string toString(TransportErrorCode code); std::string toString(QuicErrorCode code); -std::string toString( - const std::pair>& error); +std::string toString(const QuicError& error); std::string cryptoErrorToString(TransportErrorCode code); std::vector getAllTransportErrorCodes(); @@ -111,10 +122,7 @@ inline std::ostream& operator<<(std::ostream& os, const QuicErrorCode& error) { return os; } -inline std::ostream& operator<<( - std::ostream& os, - const std::pair>& - error) { +inline std::ostream& operator<<(std::ostream& os, const QuicError& error) { os << toString(error); return os; } diff --git a/quic/api/Observer.h b/quic/api/Observer.h index 74886b70d..e367a042d 100644 --- a/quic/api/Observer.h +++ b/quic/api/Observer.h @@ -386,8 +386,7 @@ class Observer { */ virtual void close( QuicSocket* /* socket */, - const folly::Optional< - std::pair>& /* errorOpt */) noexcept {} + const folly::Optional& /* errorOpt */) noexcept {} /** * evbAttach() will be invoked when a new event base is attached to this diff --git a/quic/api/QuicSocket.h b/quic/api/QuicSocket.h index bfb40969e..f1380ca57 100644 --- a/quic/api/QuicSocket.h +++ b/quic/api/QuicSocket.h @@ -48,8 +48,7 @@ class QuicSocket { /** * Invoked when the connection setup fails. */ - virtual void onConnectionSetupError( - std::pair code) noexcept = 0; + virtual void onConnectionSetupError(QuicError code) noexcept = 0; /** * Called when the transport is ready to send/receive data. @@ -110,8 +109,7 @@ class QuicSocket { /** * Invoked when the connection closed in error */ - virtual void onConnectionError( - std::pair code) noexcept = 0; + virtual void onConnectionError(QuicError code) noexcept = 0; /** * Called when more bidirectional streams become available for creation @@ -333,8 +331,7 @@ class QuicSocket { * Close this socket with a drain period. If closing with an error, it may be * specified. */ - virtual void close( - folly::Optional> errorCode) = 0; + virtual void close(folly::Optional errorCode) = 0; /** * Close this socket gracefully, by waiting for all the streams to be idle @@ -346,8 +343,7 @@ class QuicSocket { * Close this socket without a drain period. If closing with an error, it may * be specified. */ - virtual void closeNow( - folly::Optional> errorCode) = 0; + virtual void closeNow(folly::Optional errorCode) = 0; /** * Returns the event base associated with this socket @@ -532,10 +528,7 @@ class QuicSocket { /** * Called from the transport layer when there is an error on the stream. */ - virtual void readError( - StreamId id, - std::pair> - error) noexcept = 0; + virtual void readError(StreamId id, QuicError error) noexcept = 0; }; /** @@ -661,10 +654,7 @@ class QuicSocket { * Called from the transport layer during peek time when there is an error * on the stream. */ - virtual void peekError( - StreamId id, - std::pair> - error) noexcept = 0; + virtual void peekError(StreamId id, QuicError error) noexcept = 0; }; virtual folly::Expected setPeekCallback( @@ -824,16 +814,14 @@ class QuicSocket { */ virtual void onStreamWriteError( StreamId /* id */, - std::pair> - /* error */) noexcept {} + QuicError /* error */) noexcept {} /** * Invoked when a connection is being torn down after * notifyPendingWriteOnConnection has been called */ - virtual void onConnectionWriteError( - std::pair> - /* error */) noexcept {} + virtual void onConnectionWriteError(QuicError + /* error */) noexcept {} }; /** diff --git a/quic/api/QuicStreamAsyncTransport.cpp b/quic/api/QuicStreamAsyncTransport.cpp index 51fd0875e..52d950393 100644 --- a/quic/api/QuicStreamAsyncTransport.cpp +++ b/quic/api/QuicStreamAsyncTransport.cpp @@ -356,8 +356,7 @@ void QuicStreamAsyncTransport::readAvailable( void QuicStreamAsyncTransport::readError( quic::StreamId /*streamId*/, - std::pair> - error) noexcept { + QuicError error) noexcept { ex_ = folly::AsyncSocketException( folly::AsyncSocketException::UNKNOWN, folly::to("Quic read error: ", toString(error))); @@ -513,8 +512,7 @@ void QuicStreamAsyncTransport::onStreamWriteReady( void QuicStreamAsyncTransport::onStreamWriteError( StreamId /*id*/, - std::pair> - error) noexcept { + QuicError error) noexcept { closeNowImpl(folly::AsyncSocketException( folly::AsyncSocketException::UNKNOWN, folly::to("Quic write error: ", toString(error)))); diff --git a/quic/api/QuicStreamAsyncTransport.h b/quic/api/QuicStreamAsyncTransport.h index 50b4f9077..33eaa1612 100644 --- a/quic/api/QuicStreamAsyncTransport.h +++ b/quic/api/QuicStreamAsyncTransport.h @@ -127,20 +127,15 @@ class QuicStreamAsyncTransport : public folly::AsyncTransport, // QucSocket::ReadCallback overrides // void readAvailable(quic::StreamId /*streamId*/) noexcept override; - void readError( - quic::StreamId /*streamId*/, - std::pair> - error) noexcept override; + void readError(quic::StreamId /*streamId*/, QuicError error) noexcept + override; // // QucSocket::WriteCallback overrides // void onStreamWriteReady(quic::StreamId /*id*/, uint64_t maxToSend) noexcept override; - void onStreamWriteError( - StreamId /*id*/, - std::pair> - error) noexcept override; + void onStreamWriteError(StreamId /*id*/, QuicError error) noexcept override; // // folly::EventBase::LoopCallback overrides diff --git a/quic/api/QuicTransportBase.cpp b/quic/api/QuicTransportBase.cpp index baf51f6de..29439b71d 100644 --- a/quic/api/QuicTransportBase.cpp +++ b/quic/api/QuicTransportBase.cpp @@ -143,7 +143,7 @@ QuicTransportBase::~QuicTransportBase() { resetConnectionCallbacks(); closeImpl( - std::make_pair( + QuicError( QuicErrorCode(LocalErrorCode::SHUTTING_DOWN), std::string("Closing from base destructor")), false); @@ -173,8 +173,7 @@ bool QuicTransportBase::error() const { return conn_->localConnectionError.has_value(); } -void QuicTransportBase::close( - folly::Optional> errorCode) { +void QuicTransportBase::close(folly::Optional errorCode) { FOLLY_MAYBE_UNUSED auto self = sharedGuard(); // The caller probably doesn't need a conn callback any more because they // explicitly called close. @@ -183,7 +182,7 @@ void QuicTransportBase::close( // If we were called with no error code, ensure that we are going to write // an application close, so the peer knows it didn't come from the transport. if (!errorCode) { - errorCode = std::make_pair( + errorCode = QuicError( GenericApplicationErrorCode::NO_ERROR, toString(GenericApplicationErrorCode::NO_ERROR)); } @@ -191,13 +190,12 @@ void QuicTransportBase::close( conn_->logger.reset(); } -void QuicTransportBase::closeNow( - folly::Optional> errorCode) { +void QuicTransportBase::closeNow(folly::Optional errorCode) { DCHECK(getEventBase() && getEventBase()->isInEventBaseThread()); FOLLY_MAYBE_UNUSED auto self = sharedGuard(); VLOG(4) << __func__ << " " << *this; if (!errorCode) { - errorCode = std::make_pair( + errorCode = QuicError( GenericApplicationErrorCode::NO_ERROR, toString(GenericApplicationErrorCode::NO_ERROR)); } @@ -230,8 +228,8 @@ void QuicTransportBase::closeGracefully() { VLOG(10) << "Stopping read and peek loopers due to graceful close " << *this; readLooper_->stop(); peekLooper_->stop(); - cancelAllAppCallbacks(std::make_pair( - QuicErrorCode(LocalErrorCode::NO_ERROR), "Graceful Close")); + cancelAllAppCallbacks( + QuicError(QuicErrorCode(LocalErrorCode::NO_ERROR), "Graceful Close")); // All streams are closed, close the transport for realz. if (conn_->streamManager->streamCount() == 0) { closeImpl(folly::none); @@ -241,7 +239,7 @@ void QuicTransportBase::closeGracefully() { // TODO: t64691045 change the closeImpl API to include both the sanitized and // unsanited error message, remove exceptionCloseWhat_. void QuicTransportBase::closeImpl( - folly::Optional> errorCode, + folly::Optional errorCode, bool drainConnection, bool sendCloseImmediately) { if (closeState_ == CloseState::CLOSED) { @@ -298,9 +296,9 @@ void QuicTransportBase::closeImpl( // TODO: truncate the error code string to be 1MSS only. closeState_ = CloseState::CLOSED; updatePacingOnClose(*conn_); - auto cancelCode = std::make_pair( + auto cancelCode = QuicError( QuicErrorCode(LocalErrorCode::NO_ERROR), - toString(LocalErrorCode::NO_ERROR)); + toString(LocalErrorCode::NO_ERROR).str()); if (conn_->peerConnectionError) { cancelCode = *conn_->peerConnectionError; } else if (errorCode) { @@ -310,14 +308,14 @@ void QuicTransportBase::closeImpl( // errorCode will be used for localConnectionError, and sent in close frames. // It's safe to include the unsanitized error message in cancelCode if (exceptionCloseWhat_) { - cancelCode.second = exceptionCloseWhat_.value(); + cancelCode.message = exceptionCloseWhat_.value(); } bool isReset = false; bool isAbandon = false; bool isInvalidMigration = false; - LocalErrorCode* localError = cancelCode.first.asLocalErrorCode(); - TransportErrorCode* transportError = cancelCode.first.asTransportErrorCode(); + LocalErrorCode* localError = cancelCode.code.asLocalErrorCode(); + TransportErrorCode* transportError = cancelCode.code.asTransportErrorCode(); if (localError) { isReset = *localError == LocalErrorCode::CONNECTION_RESET; isAbandon = *localError == LocalErrorCode::CONNECTION_ABANDONED; @@ -329,8 +327,8 @@ void QuicTransportBase::closeImpl( << *this; if (errorCode) { conn_->localConnectionError = errorCode; - std::string errorStr = conn_->localConnectionError->second; - std::string errorCodeStr = errorCode->second; + std::string errorStr = conn_->localConnectionError->message; + std::string errorCodeStr = errorCode->message; if (conn_->qLogger) { conn_->qLogger->addConnectionClose( errorStr, errorCodeStr, drainConnection, sendCloseImmediately); @@ -435,12 +433,11 @@ void QuicTransportBase::closeImpl( } } -bool QuicTransportBase::processCancelCode( - const std::pair& cancelCode) { +bool QuicTransportBase::processCancelCode(const QuicError& cancelCode) { bool noError = false; - switch (cancelCode.first.type()) { + switch (cancelCode.code.type()) { case QuicErrorCode::Type::LocalErrorCode: { - LocalErrorCode localErrorCode = *cancelCode.first.asLocalErrorCode(); + LocalErrorCode localErrorCode = *cancelCode.code.asLocalErrorCode(); noError = localErrorCode == LocalErrorCode::NO_ERROR || localErrorCode == LocalErrorCode::IDLE_TIMEOUT || localErrorCode == LocalErrorCode::SHUTTING_DOWN; @@ -448,45 +445,44 @@ bool QuicTransportBase::processCancelCode( } case QuicErrorCode::Type::TransportErrorCode: { TransportErrorCode transportErrorCode = - *cancelCode.first.asTransportErrorCode(); + *cancelCode.code.asTransportErrorCode(); noError = transportErrorCode == TransportErrorCode::NO_ERROR; break; } case QuicErrorCode::Type::ApplicationErrorCode: - auto appErrorCode = *cancelCode.first.asApplicationErrorCode(); + auto appErrorCode = *cancelCode.code.asApplicationErrorCode(); noError = appErrorCode == GenericApplicationErrorCode::NO_ERROR; } return noError; } -void QuicTransportBase::processConnectionEndError( - const std::pair& cancelCode) { +void QuicTransportBase::processConnectionEndError(const QuicError& cancelCode) { bool noError = processCancelCode(cancelCode); if (noError) { connCallback_->onConnectionEnd(); } else { connCallback_->onConnectionError( - std::make_pair(cancelCode.first, cancelCode.second.str())); + QuicError(cancelCode.code, cancelCode.message)); } } void QuicTransportBase::processConnectionEndErrorSplitCallbacks( - const std::pair& cancelCode) { + const QuicError& cancelCode) { bool noError = processCancelCode(cancelCode); if (noError) { if (transportReadyNotified_) { connCallback_->onConnectionEnd(); } else { connCallback_->onConnectionSetupError( - std::make_pair(cancelCode.first, cancelCode.second.str())); + QuicError(cancelCode.code, cancelCode.message)); } } else { if (transportReadyNotified_) { connCallback_->onConnectionError( - std::make_pair(cancelCode.first, cancelCode.second.str())); + QuicError(cancelCode.code, cancelCode.message)); } else { connCallback_->onConnectionSetupError( - std::make_pair(cancelCode.first, cancelCode.second.str())); + QuicError(cancelCode.code, cancelCode.message)); } } } @@ -849,8 +845,7 @@ void QuicTransportBase::invokeReadDataAndCallbacks() { peekCallbacks_.erase(streamId); VLOG(10) << "invoking read error callbacks on stream=" << streamId << " " << *this; - readCb->readError( - streamId, std::make_pair(*stream->streamReadError, folly::none)); + readCb->readError(streamId, QuicError(*stream->streamReadError)); } else if ( readCb && callback->second.resumed && stream->hasReadableData()) { VLOG(10) << "invoking read callbacks on stream=" << streamId << " " @@ -996,8 +991,7 @@ void QuicTransportBase::invokePeekDataAndCallbacks() { if (peekCb && stream->streamReadError) { VLOG(10) << "invoking peek error callbacks on stream=" << streamId << " " << *this; - peekCb->peekError( - streamId, std::make_pair(*stream->streamReadError, folly::none)); + peekCb->peekError(streamId, QuicError(*stream->streamReadError)); } else if ( peekCb && !stream->streamReadError && stream->hasPeekableData()) { VLOG(10) << "invoking peek callbacks on stream=" << streamId << " " @@ -1263,19 +1257,19 @@ folly::Expected, LocalErrorCode> QuicTransportBase::read( } catch (const QuicTransportException& ex) { VLOG(4) << "read() error " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - closeImpl(std::make_pair( - QuicErrorCode(ex.errorCode()), std::string("read() error"))); + closeImpl( + QuicError(QuicErrorCode(ex.errorCode()), std::string("read() error"))); return folly::makeUnexpected(LocalErrorCode::TRANSPORT_ERROR); } catch (const QuicInternalException& ex) { VLOG(4) << __func__ << " " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - closeImpl(std::make_pair( - QuicErrorCode(ex.errorCode()), std::string("read() error"))); + closeImpl( + QuicError(QuicErrorCode(ex.errorCode()), std::string("read() error"))); return folly::makeUnexpected(ex.errorCode()); } catch (const std::exception& ex) { VLOG(4) << "read() error " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - closeImpl(std::make_pair( + closeImpl(QuicError( QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), std::string("read() error"))); return folly::makeUnexpected(LocalErrorCode::INTERNAL_ERROR); @@ -1374,20 +1368,20 @@ folly:: } catch (const QuicTransportException& ex) { VLOG(4) << "consume() error " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - closeImpl(std::make_pair( + closeImpl(QuicError( QuicErrorCode(ex.errorCode()), std::string("consume() error"))); return folly::makeUnexpected( ConsumeError{LocalErrorCode::TRANSPORT_ERROR, readOffset}); } catch (const QuicInternalException& ex) { VLOG(4) << __func__ << " " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - closeImpl(std::make_pair( + closeImpl(QuicError( QuicErrorCode(ex.errorCode()), std::string("consume() error"))); return folly::makeUnexpected(ConsumeError{ex.errorCode(), readOffset}); } catch (const std::exception& ex) { VLOG(4) << "consume() error " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - closeImpl(std::make_pair( + closeImpl(QuicError( QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), std::string("consume() error"))); return folly::makeUnexpected( @@ -1809,21 +1803,21 @@ void QuicTransportBase::onNetworkData( VLOG(4) << __func__ << " " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); return closeImpl( - std::make_pair(QuicErrorCode(ex.errorCode()), std::string(ex.what()))); + QuicError(QuicErrorCode(ex.errorCode()), std::string(ex.what()))); } catch (const QuicInternalException& ex) { VLOG(4) << __func__ << " " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); return closeImpl( - std::make_pair(QuicErrorCode(ex.errorCode()), std::string(ex.what()))); + QuicError(QuicErrorCode(ex.errorCode()), std::string(ex.what()))); } catch (const QuicApplicationException& ex) { VLOG(4) << __func__ << " " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); return closeImpl( - std::make_pair(QuicErrorCode(ex.errorCode()), std::string(ex.what()))); + QuicError(QuicErrorCode(ex.errorCode()), std::string(ex.what()))); } catch (const std::exception& ex) { VLOG(4) << __func__ << " " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - return closeImpl(std::make_pair( + return closeImpl(QuicError( QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), std::string("error onNetworkData()"))); } @@ -1997,14 +1991,14 @@ QuicTransportBase::notifyPendingWriteOnStream(StreamId id, WriteCallback* wcb) { if (!self->conn_->streamManager->streamExists(id)) { self->pendingWriteCallbacks_.erase(wcbIt); writeCallback->onStreamWriteError( - id, std::make_pair(LocalErrorCode::STREAM_NOT_EXISTS, folly::none)); + id, QuicError(LocalErrorCode::STREAM_NOT_EXISTS)); return; } auto stream = self->conn_->streamManager->getStream(id); if (!stream->writable()) { self->pendingWriteCallbacks_.erase(wcbIt); writeCallback->onStreamWriteError( - id, std::make_pair(LocalErrorCode::STREAM_NOT_EXISTS, folly::none)); + id, QuicError(LocalErrorCode::STREAM_NOT_EXISTS)); return; } auto maxCanWrite = self->maxWritableOnStream(*stream); @@ -2077,21 +2071,21 @@ QuicSocket::WriteResult QuicTransportBase::writeChain( VLOG(4) << __func__ << " streamId=" << id << " " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - closeImpl(std::make_pair( + closeImpl(QuicError( QuicErrorCode(ex.errorCode()), std::string("writeChain() error"))); return folly::makeUnexpected(LocalErrorCode::TRANSPORT_ERROR); } catch (const QuicInternalException& ex) { VLOG(4) << __func__ << " streamId=" << id << " " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - closeImpl(std::make_pair( + closeImpl(QuicError( QuicErrorCode(ex.errorCode()), std::string("writeChain() error"))); return folly::makeUnexpected(ex.errorCode()); } catch (const std::exception& ex) { VLOG(4) << __func__ << " streamId=" << id << " " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - closeImpl(std::make_pair( + closeImpl(QuicError( QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), std::string("writeChain() error"))); return folly::makeUnexpected(LocalErrorCode::INTERNAL_ERROR); @@ -2153,21 +2147,21 @@ QuicSocket::WriteResult QuicTransportBase::writeBufMeta( VLOG(4) << __func__ << " streamId=" << id << " " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - closeImpl(std::make_pair( + closeImpl(QuicError( QuicErrorCode(ex.errorCode()), std::string("writeChain() error"))); return folly::makeUnexpected(LocalErrorCode::TRANSPORT_ERROR); } catch (const QuicInternalException& ex) { VLOG(4) << __func__ << " streamId=" << id << " " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - closeImpl(std::make_pair( + closeImpl(QuicError( QuicErrorCode(ex.errorCode()), std::string("writeChain() error"))); return folly::makeUnexpected(ex.errorCode()); } catch (const std::exception& ex) { VLOG(4) << __func__ << " streamId=" << id << " " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - closeImpl(std::make_pair( + closeImpl(QuicError( QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), std::string("writeChain() error"))); return folly::makeUnexpected(LocalErrorCode::INTERNAL_ERROR); @@ -2347,21 +2341,21 @@ folly::Expected QuicTransportBase::resetStream( VLOG(4) << __func__ << " streamId=" << id << " " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - closeImpl(std::make_pair( + closeImpl(QuicError( QuicErrorCode(ex.errorCode()), std::string("resetStream() error"))); return folly::makeUnexpected(LocalErrorCode::TRANSPORT_ERROR); } catch (const QuicInternalException& ex) { VLOG(4) << __func__ << " streamId=" << id << " " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - closeImpl(std::make_pair( + closeImpl(QuicError( QuicErrorCode(ex.errorCode()), std::string("resetStream() error"))); return folly::makeUnexpected(ex.errorCode()); } catch (const std::exception& ex) { VLOG(4) << __func__ << " streamId=" << id << " " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - closeImpl(std::make_pair( + closeImpl(QuicError( QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), std::string("resetStream() error"))); return folly::makeUnexpected(LocalErrorCode::INTERNAL_ERROR); @@ -2480,19 +2474,19 @@ void QuicTransportBase::lossTimeoutExpired() noexcept { } catch (const QuicTransportException& ex) { VLOG(4) << __func__ << " " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - closeImpl(std::make_pair( + closeImpl(QuicError( QuicErrorCode(ex.errorCode()), std::string("lossTimeoutExpired() error"))); } catch (const QuicInternalException& ex) { VLOG(4) << __func__ << " " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - closeImpl(std::make_pair( + closeImpl(QuicError( QuicErrorCode(ex.errorCode()), std::string("lossTimeoutExpired() error"))); } catch (const std::exception& ex) { VLOG(4) << __func__ << " " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - closeImpl(std::make_pair( + closeImpl(QuicError( QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), std::string("lossTimeoutExpired() error"))); } @@ -2525,7 +2519,7 @@ void QuicTransportBase::pathValidationTimeoutExpired() noexcept { // TODO junqiw probing is not supported, so pathValidation==connMigration // We decide to close conn when pathValidation to migrated path fails. FOLLY_MAYBE_UNUSED auto self = sharedGuard(); - closeImpl(std::make_pair( + closeImpl(QuicError( QuicErrorCode(TransportErrorCode::INVALID_MIGRATION), std::string("Path validation timed out"))); } @@ -2540,7 +2534,7 @@ void QuicTransportBase::idleTimeoutExpired(bool drain) noexcept { auto localError = drain ? LocalErrorCode::IDLE_TIMEOUT : LocalErrorCode::SHUTTING_DOWN; closeImpl( - std::make_pair( + quic::QuicError( QuicErrorCode(localError), folly::to( toString(localError), @@ -2727,8 +2721,7 @@ void QuicTransportBase::setEarlyDataAppParamsFunctions( conn_->earlyDataAppParamsGetter = std::move(getter); } -void QuicTransportBase::cancelAllAppCallbacks( - const std::pair& err) noexcept { +void QuicTransportBase::cancelAllAppCallbacks(const QuicError& err) noexcept { SCOPE_EXIT { checkForClosedStream(); updateReadLooper(); @@ -2795,7 +2788,8 @@ void QuicTransportBase::resetNonControlStreams( if (isSendingStream(conn_->nodeType, id) || isBidirectionalStream(id)) { auto writeCallbackIt = pendingWriteCallbacks_.find(id); if (writeCallbackIt != pendingWriteCallbacks_.end()) { - writeCallbackIt->second->onStreamWriteError(id, {error, errorMsg}); + writeCallbackIt->second->onStreamWriteError( + id, QuicError(error, errorMsg.str())); } resetStream(id, error); } @@ -2803,7 +2797,8 @@ void QuicTransportBase::resetNonControlStreams( auto readCallbackIt = readCallbacks_.find(id); if (readCallbackIt != readCallbacks_.end() && readCallbackIt->second.readCb) { - readCallbackIt->second.readCb->readError(id, {error, errorMsg}); + readCallbackIt->second.readCb->readError( + id, QuicError(error, errorMsg.str())); } peekCallbacks_.erase(id); stopSending(id, error); @@ -3083,19 +3078,19 @@ void QuicTransportBase::writeSocketDataAndCatch() { } catch (const QuicTransportException& ex) { VLOG(4) << __func__ << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - closeImpl(std::make_pair( + closeImpl(QuicError( QuicErrorCode(ex.errorCode()), std::string("writeSocketDataAndCatch() error"))); } catch (const QuicInternalException& ex) { VLOG(4) << __func__ << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - closeImpl(std::make_pair( + closeImpl(QuicError( QuicErrorCode(ex.errorCode()), std::string("writeSocketDataAndCatch() error"))); } catch (const std::exception& ex) { VLOG(4) << __func__ << " error=" << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - closeImpl(std::make_pair( + closeImpl(QuicError( QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), std::string("writeSocketDataAndCatch() error"))); } @@ -3497,21 +3492,21 @@ QuicSocket::WriteResult QuicTransportBase::setDSRPacketizationRequestSender( VLOG(4) << __func__ << " streamId=" << id << " " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - closeImpl(std::make_pair( + closeImpl(QuicError( QuicErrorCode(ex.errorCode()), std::string("writeChain() error"))); return folly::makeUnexpected(LocalErrorCode::TRANSPORT_ERROR); } catch (const QuicInternalException& ex) { VLOG(4) << __func__ << " streamId=" << id << " " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - closeImpl(std::make_pair( + closeImpl(QuicError( QuicErrorCode(ex.errorCode()), std::string("writeChain() error"))); return folly::makeUnexpected(ex.errorCode()); } catch (const std::exception& ex) { VLOG(4) << __func__ << " streamId=" << id << " " << ex.what() << " " << *this; exceptionCloseWhat_ = ex.what(); - closeImpl(std::make_pair( + closeImpl(QuicError( QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), std::string("writeChain() error"))); return folly::makeUnexpected(LocalErrorCode::INTERNAL_ERROR); diff --git a/quic/api/QuicTransportBase.h b/quic/api/QuicTransportBase.h index 3072d06e1..e13bca68b 100644 --- a/quic/api/QuicTransportBase.h +++ b/quic/api/QuicTransportBase.h @@ -68,13 +68,11 @@ class QuicTransportBase : public QuicSocket, QuicStreamPrioritiesObserver { bool error() const override; - void close( - folly::Optional> error) override; + void close(folly::Optional error) override; void closeGracefully() override; - void closeNow( - folly::Optional> error) override; + void closeNow(folly::Optional error) override; folly::Expected getStreamReadOffset( StreamId id) const override; @@ -622,8 +620,7 @@ class QuicTransportBase : public QuicSocket, QuicStreamPrioritiesObserver { conn_->loopDetectorCallback = std::move(callback); } - virtual void cancelAllAppCallbacks( - const std::pair& error) noexcept; + virtual void cancelAllAppCallbacks(const QuicError& error) noexcept; /** * Adds an observer. @@ -732,7 +729,7 @@ class QuicTransportBase : public QuicSocket, QuicStreamPrioritiesObserver { folly::Function)> func); void closeImpl( - folly::Optional> error, + folly::Optional error, bool drainConnection = true, bool sendCloseImmediately = true); folly::Expected pauseOrResumeRead( @@ -858,12 +855,9 @@ class QuicTransportBase : public QuicSocket, QuicStreamPrioritiesObserver { connCallback_ = nullptr; } - bool processCancelCode( - const std::pair& cancelCode); - void processConnectionEndError( - const std::pair& cancelCode); - void processConnectionEndErrorSplitCallbacks( - const std::pair& cancelCode); + bool processCancelCode(const QuicError& cancelCode); + void processConnectionEndError(const QuicError& cancelCode); + void processConnectionEndErrorSplitCallbacks(const QuicError& cancelCode); class CallbackDispatcher : public folly::DelayedDestruction, public ConnectionSetupCallback, @@ -888,8 +882,7 @@ class QuicTransportBase : public QuicSocket, QuicStreamPrioritiesObserver { folly::DelayedDestruction::DestructorGuard dg(this); CHECK_NOTNULL(connSetupCallback_)->onFirstPeerPacketProcessed(); } - void onConnectionSetupError( - std::pair code) noexcept override { + void onConnectionSetupError(QuicError code) noexcept override { folly::DelayedDestruction::DestructorGuard dg(this); CHECK_NOTNULL(connSetupCallback_) ->onConnectionSetupError(std::move(code)); @@ -917,8 +910,7 @@ class QuicTransportBase : public QuicSocket, QuicStreamPrioritiesObserver { folly::DelayedDestruction::DestructorGuard dg(this); CHECK_NOTNULL(connStreamsCallback_)->onConnectionEnd(); } - void onConnectionError( - std::pair code) noexcept override { + void onConnectionError(QuicError code) noexcept override { folly::DelayedDestruction::DestructorGuard dg(this); CHECK_NOTNULL(connStreamsCallback_)->onConnectionError(std::move(code)); } diff --git a/quic/api/QuicTransportFunctions.cpp b/quic/api/QuicTransportFunctions.cpp index db9875340..e42fe44fa 100644 --- a/quic/api/QuicTransportFunctions.cpp +++ b/quic/api/QuicTransportFunctions.cpp @@ -1138,7 +1138,7 @@ void writeCloseCommon( folly::AsyncUDPSocket& sock, QuicConnectionStateBase& connection, PacketHeader&& header, - folly::Optional> closeDetails, + folly::Optional closeDetails, const Aead& aead, const PacketNumberCipher& headerCipher) { // close is special, we're going to bypass all the packet sent logic for all @@ -1161,20 +1161,20 @@ void writeCloseCommon( std::string("No error")), packetBuilder); } else { - switch (closeDetails->first.type()) { + switch (closeDetails->code.type()) { case QuicErrorCode::Type::ApplicationErrorCode: written = writeFrame( ConnectionCloseFrame( - QuicErrorCode(*closeDetails->first.asApplicationErrorCode()), - closeDetails->second, + QuicErrorCode(*closeDetails->code.asApplicationErrorCode()), + closeDetails->message, quic::FrameType::CONNECTION_CLOSE_APP_ERR), packetBuilder); break; case QuicErrorCode::Type::TransportErrorCode: written = writeFrame( ConnectionCloseFrame( - QuicErrorCode(*closeDetails->first.asTransportErrorCode()), - closeDetails->second, + QuicErrorCode(*closeDetails->code.asTransportErrorCode()), + closeDetails->message, quic::FrameType::CONNECTION_CLOSE), packetBuilder); break; @@ -1240,7 +1240,7 @@ void writeLongClose( const ConnectionId& srcConnId, const ConnectionId& dstConnId, LongHeader::Types headerType, - folly::Optional> closeDetails, + folly::Optional closeDetails, const Aead& aead, const PacketNumberCipher& headerCipher, QuicVersion version) { @@ -1269,7 +1269,7 @@ void writeShortClose( folly::AsyncUDPSocket& sock, QuicConnectionStateBase& connection, const ConnectionId& connId, - folly::Optional> closeDetails, + folly::Optional closeDetails, const Aead& aead, const PacketNumberCipher& headerCipher) { auto header = ShortHeader( diff --git a/quic/api/QuicTransportFunctions.h b/quic/api/QuicTransportFunctions.h index 120367300..d14cdd008 100644 --- a/quic/api/QuicTransportFunctions.h +++ b/quic/api/QuicTransportFunctions.h @@ -219,7 +219,7 @@ void writeCloseCommon( folly::AsyncUDPSocket& sock, QuicConnectionStateBase& connection, PacketHeader&& header, - folly::Optional> closeDetails, + folly::Optional closeDetails, const Aead& aead, const PacketNumberCipher& headerCipher); @@ -233,7 +233,7 @@ void writeLongClose( const ConnectionId& srcConnId, const ConnectionId& dstConnId, LongHeader::Types headerType, - folly::Optional> closeDetails, + folly::Optional closeDetails, const Aead& aead, const PacketNumberCipher& headerCipher, QuicVersion); @@ -246,7 +246,7 @@ void writeShortClose( folly::AsyncUDPSocket& sock, QuicConnectionStateBase& connection, const ConnectionId& connId, - folly::Optional> closeDetails, + folly::Optional closeDetails, const Aead& aead, const PacketNumberCipher& headerCipher); diff --git a/quic/api/test/MockQuicSocket.h b/quic/api/test/MockQuicSocket.h index b93b7bd52..fd530d226 100644 --- a/quic/api/test/MockQuicSocket.h +++ b/quic/api/test/MockQuicSocket.h @@ -26,13 +26,9 @@ class MockQuicSocket : public QuicSocket { MOCK_CONST_METHOD0(good, bool()); MOCK_CONST_METHOD0(replaySafe, bool()); MOCK_CONST_METHOD0(error, bool()); - MOCK_METHOD1( - close, - void(folly::Optional>)); + MOCK_METHOD1(close, void(folly::Optional)); MOCK_METHOD0(closeGracefully, void()); - MOCK_METHOD1( - closeNow, - void(folly::Optional>)); + MOCK_METHOD1(closeNow, void(folly::Optional)); MOCK_CONST_METHOD0( getClientConnectionId, folly::Optional()); diff --git a/quic/api/test/Mocks.h b/quic/api/test/Mocks.h index bf37e2cfe..76b3c2ece 100644 --- a/quic/api/test/Mocks.h +++ b/quic/api/test/Mocks.h @@ -44,14 +44,7 @@ class MockReadCallback : public QuicSocket::ReadCallback { public: ~MockReadCallback() override = default; GMOCK_METHOD1_(, noexcept, , readAvailable, void(StreamId)); - GMOCK_METHOD2_( - , - noexcept, - , - readError, - void( - StreamId, - std::pair>)); + GMOCK_METHOD2_(, noexcept, , readError, void(StreamId, QuicError)); }; class MockPeekCallback : public QuicSocket::PeekCallback { @@ -63,14 +56,7 @@ class MockPeekCallback : public QuicSocket::PeekCallback { , onDataAvailable, void(StreamId, const folly::Range&)); - GMOCK_METHOD2_( - , - noexcept, - , - peekError, - void( - StreamId, - std::pair>)); + GMOCK_METHOD2_(, noexcept, , peekError, void(StreamId, QuicError)); }; class MockDatagramCallback : public QuicSocket::DatagramCallback { @@ -85,31 +71,14 @@ class MockWriteCallback : public QuicSocket::WriteCallback { GMOCK_METHOD2_(, noexcept, , onStreamWriteReady, void(StreamId, uint64_t)); GMOCK_METHOD1_(, noexcept, , onConnectionWriteReady, void(uint64_t)); - GMOCK_METHOD2_( - , - noexcept, - , - onStreamWriteError, - void( - StreamId, - std::pair>)); - GMOCK_METHOD1_( - , - noexcept, - , - onConnectionWriteError, - void(std::pair>)); + GMOCK_METHOD2_(, noexcept, , onStreamWriteError, void(StreamId, QuicError)); + GMOCK_METHOD1_(, noexcept, , onConnectionWriteError, void(QuicError)); }; class MockConnectionSetupCallback : public QuicSocket::ConnectionSetupCallback { public: ~MockConnectionSetupCallback() override = default; - GMOCK_METHOD1_( - , - noexcept, - , - onConnectionSetupError, - void(std::pair)); + GMOCK_METHOD1_(, noexcept, , onConnectionSetupError, void(QuicError)); GMOCK_METHOD0_(, noexcept, , onReplaySafe, void()); GMOCK_METHOD0_(, noexcept, , onTransportReady, void()); GMOCK_METHOD0_(, noexcept, , onFirstPeerPacketProcessed, void()); @@ -129,12 +98,7 @@ class MockConnectionCallbackNew : public QuicSocket::ConnectionCallbackNew { onStopSending, void(StreamId, ApplicationErrorCode)); GMOCK_METHOD0_(, noexcept, , onConnectionEnd, void()); - GMOCK_METHOD1_( - , - noexcept, - , - onConnectionError, - void(std::pair)); + GMOCK_METHOD1_(, noexcept, , onConnectionError, void(QuicError)); GMOCK_METHOD1_(, noexcept, , onBidirectionalStreamsAvailable, void(uint64_t)); GMOCK_METHOD1_( , @@ -289,19 +253,9 @@ class MockQuicTransport : public QuicServerTransport { setServerConnectionIdParams, void(ServerConnectionIdParams)); - GMOCK_METHOD1_( - , - noexcept, - , - close, - void(folly::Optional>)); + GMOCK_METHOD1_(, noexcept, , close, void(folly::Optional)); - GMOCK_METHOD1_( - , - noexcept, - , - closeNow, - void(folly::Optional>)); + GMOCK_METHOD1_(, noexcept, , closeNow, void(folly::Optional)); GMOCK_METHOD0_(, const, , hasShutdown, bool()); @@ -354,9 +308,7 @@ class MockObserver : public Observer { noexcept, , close, - void( - QuicSocket*, - const folly::Optional>&)); + void(QuicSocket*, const folly::Optional&)); GMOCK_METHOD2_( , noexcept, diff --git a/quic/api/test/QuicTransportBaseTest.cpp b/quic/api/test/QuicTransportBaseTest.cpp index ecc3faf7b..f18122c88 100644 --- a/quic/api/test/QuicTransportBaseTest.cpp +++ b/quic/api/test/QuicTransportBaseTest.cpp @@ -218,7 +218,7 @@ class TestQuicTransport resetConnectionCallbacks(); // we need to call close in the derived class. closeImpl( - std::make_pair( + QuicError( QuicErrorCode(LocalErrorCode::SHUTTING_DOWN), std::string("shutdown")), false); @@ -439,7 +439,7 @@ class TestQuicTransport } QuicErrorCode getConnectionError() { - return conn_->localConnectionError->first; + return conn_->localConnectionError->code; } bool isClosed() const noexcept { @@ -578,8 +578,7 @@ TEST_F(QuicTransportImplTest, IdleTimeoutStreamMaessage) { EXPECT_CALL(readCb1, readError(stream1, _)) .Times(1) .WillOnce(Invoke([](auto, auto error) { - EXPECT_EQ( - "Idle timeout, num non control streams: 2", error.second->str()); + EXPECT_EQ("Idle timeout, num non control streams: 2", error.message); })); transport->invokeIdleTimeout(); } @@ -1386,12 +1385,9 @@ TEST_F(QuicTransportImplTest, ReadErrorUnsanitizedErrorMsg) { transport->setReadCallback(stream, &rcb); EXPECT_CALL(rcb, readError(stream, _)) .Times(1) - .WillOnce(Invoke( - [](StreamId, - std::pair> - error) { - EXPECT_EQ("You need to calm down.", *error.second); - })); + .WillOnce(Invoke([](StreamId, QuicError error) { + EXPECT_EQ("You need to calm down.", error.message); + })); EXPECT_CALL(*socketPtr, write(_, _)).WillOnce(Invoke([](auto&, auto&) { throw std::runtime_error("You need to calm down."); @@ -1412,7 +1408,7 @@ TEST_F(QuicTransportImplTest, ConnectionErrorUnhandledException) { auto stream = transport->createBidirectionalStream().value(); EXPECT_CALL( connSetupCallback, - onConnectionSetupError(std::make_pair( + onConnectionSetupError(QuicError( QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), std::string("Well there's your problem")))); EXPECT_CALL(*socketPtr, write(_, _)).WillOnce(Invoke([](auto&, auto&) { @@ -2505,7 +2501,7 @@ TEST_P(QuicTransportImplTestClose, TestNotifyPendingConnWriteOnCloseWithError) { wcb, onConnectionWriteError( IsAppError(GenericApplicationErrorCode::UNKNOWN))); - transport->close(std::make_pair( + transport->close(QuicError( QuicErrorCode(GenericApplicationErrorCode::UNKNOWN), std::string("Bye"))); } else { @@ -2546,7 +2542,7 @@ TEST_P(QuicTransportImplTestClose, TestNotifyPendingWriteOnCloseWithError) { wcb, onStreamWriteError( stream, IsAppError(GenericApplicationErrorCode::UNKNOWN))); - transport->close(std::make_pair( + transport->close(QuicError( QuicErrorCode(GenericApplicationErrorCode::UNKNOWN), std::string("Bye"))); } else { @@ -2709,7 +2705,7 @@ TEST_F(QuicTransportImplTest, TestImmediateClose) { EXPECT_CALL(txCb, onByteEventRegistered(getTxMatcher(stream, 4))); EXPECT_FALSE(transport->registerTxCallback(stream, 0, &txCb).hasError()); EXPECT_FALSE(transport->registerTxCallback(stream, 4, &txCb).hasError()); - transport->close(std::make_pair( + transport->close(QuicError( QuicErrorCode(GenericApplicationErrorCode::UNKNOWN), std::string("Error"))); @@ -3635,15 +3631,11 @@ TEST_F(QuicTransportImplTest, ObserverCloseNoErrorThenDestroyTransport) { transport->addObserver(cb.get()); EXPECT_THAT(transport->getObservers(), UnorderedElementsAre(cb.get())); - const std::pair defaultError = std::make_pair( + const QuicError defaultError = QuicError( GenericApplicationErrorCode::NO_ERROR, toString(GenericApplicationErrorCode::NO_ERROR)); EXPECT_CALL( - *cb, - close( - transport.get(), - folly::Optional>( - defaultError))); + *cb, close(transport.get(), folly::Optional(defaultError))); transport->close(folly::none); Mock::VerifyAndClearExpectations(cb.get()); InSequence s; @@ -3658,14 +3650,11 @@ TEST_F(QuicTransportImplTest, ObserverCloseWithErrorThenDestroyTransport) { transport->addObserver(cb.get()); EXPECT_THAT(transport->getObservers(), UnorderedElementsAre(cb.get())); - const auto testError = std::make_pair( + const auto testError = QuicError( QuicErrorCode(LocalErrorCode::CONNECTION_RESET), std::string("testError")); EXPECT_CALL( - *cb, - close( - transport.get(), - folly::Optional>(testError))); + *cb, close(transport.get(), folly::Optional(testError))); transport->close(testError); Mock::VerifyAndClearExpectations(cb.get()); InSequence s; diff --git a/quic/api/test/QuicTransportTest.cpp b/quic/api/test/QuicTransportTest.cpp index 52930749f..6ace78706 100644 --- a/quic/api/test/QuicTransportTest.cpp +++ b/quic/api/test/QuicTransportTest.cpp @@ -1675,9 +1675,9 @@ TEST_F(QuicTransportTest, PathValidationTimeoutExpired) { EXPECT_EQ(transport_->closeState(), CloseState::CLOSED); EXPECT_TRUE(conn.localConnectionError); EXPECT_EQ( - conn.localConnectionError->first, + conn.localConnectionError->code, QuicErrorCode(TransportErrorCode::INVALID_MIGRATION)); - EXPECT_EQ(conn.localConnectionError->second, "Path validation timed out"); + EXPECT_EQ(conn.localConnectionError->message, "Path validation timed out"); } TEST_F(QuicTransportTest, SendPathValidationWhileThereIsOutstandingOne) { diff --git a/quic/api/test/QuicTypedTransportTest.cpp b/quic/api/test/QuicTypedTransportTest.cpp index e6442fa7d..4d8c34235 100644 --- a/quic/api/test/QuicTypedTransportTest.cpp +++ b/quic/api/test/QuicTypedTransportTest.cpp @@ -490,15 +490,11 @@ TYPED_TEST( transport->addObserver(observer.get()); EXPECT_THAT(transport->getObservers(), UnorderedElementsAre(observer.get())); - const std::pair defaultError = std::make_pair( + const QuicError defaultError = QuicError( GenericApplicationErrorCode::NO_ERROR, toString(GenericApplicationErrorCode::NO_ERROR)); EXPECT_CALL( - *observer, - close( - transport, - folly::Optional>( - defaultError))); + *observer, close(transport, folly::Optional(defaultError))); transport->close(folly::none); Mock::VerifyAndClearExpectations(observer.get()); InSequence s; @@ -517,14 +513,11 @@ TYPED_TEST( transport->addObserver(observer.get()); EXPECT_THAT(transport->getObservers(), UnorderedElementsAre(observer.get())); - const auto testError = std::make_pair( + const auto testError = QuicError( QuicErrorCode(LocalErrorCode::CONNECTION_RESET), std::string("testError")); EXPECT_CALL( - *observer, - close( - transport, - folly::Optional>(testError))); + *observer, close(transport, folly::Optional(testError))); transport->close(testError); Mock::VerifyAndClearExpectations(observer.get()); InSequence s; diff --git a/quic/api/test/TestQuicTransport.h b/quic/api/test/TestQuicTransport.h index d697c176f..f45a13ea1 100644 --- a/quic/api/test/TestQuicTransport.h +++ b/quic/api/test/TestQuicTransport.h @@ -39,7 +39,7 @@ class TestQuicTransport // we need to call close in the derived class. resetConnectionCallbacks(); closeImpl( - std::make_pair( + QuicError( QuicErrorCode(LocalErrorCode::SHUTTING_DOWN), std::string("shutdown")), false); diff --git a/quic/client/QuicClientAsyncTransport.cpp b/quic/client/QuicClientAsyncTransport.cpp index f1d2f4495..755ea75c4 100644 --- a/quic/client/QuicClientAsyncTransport.cpp +++ b/quic/client/QuicClientAsyncTransport.cpp @@ -38,11 +38,10 @@ void QuicClientAsyncTransport::onConnectionEnd() noexcept { closeNowImpl(std::move(ex)); } -void QuicClientAsyncTransport::onConnectionError( - std::pair code) noexcept { +void QuicClientAsyncTransport::onConnectionError(QuicError error) noexcept { folly::AsyncSocketException ex( folly::AsyncSocketException::UNKNOWN, - folly::to("Quic connection error", code.second)); + folly::to("Quic connection error", error.message)); // TODO: closeNow inside this callback may actually trigger gracefull close closeNowImpl(std::move(ex)); } diff --git a/quic/client/QuicClientAsyncTransport.h b/quic/client/QuicClientAsyncTransport.h index 5e80c920f..2ed40836b 100644 --- a/quic/client/QuicClientAsyncTransport.h +++ b/quic/client/QuicClientAsyncTransport.h @@ -36,12 +36,10 @@ class QuicClientAsyncTransport : public QuicStreamAsyncTransport, void onNewUnidirectionalStream(StreamId id) noexcept override; void onStopSending(StreamId id, ApplicationErrorCode error) noexcept override; void onConnectionEnd() noexcept override; - void onConnectionSetupError( - std::pair code) noexcept override { + void onConnectionSetupError(QuicError code) noexcept override { onConnectionError(std::move(code)); } - void onConnectionError( - std::pair code) noexcept override; + void onConnectionError(QuicError code) noexcept override; void onTransportReady() noexcept override; }; } // namespace quic diff --git a/quic/client/QuicClientTransport.cpp b/quic/client/QuicClientTransport.cpp index ddb5a4c3b..4021c91e3 100644 --- a/quic/client/QuicClientTransport.cpp +++ b/quic/client/QuicClientTransport.cpp @@ -93,7 +93,7 @@ QuicClientTransport::~QuicClientTransport() { resetConnectionCallbacks(); // Close without draining. closeImpl( - std::make_pair( + QuicError( QuicErrorCode(LocalErrorCode::SHUTTING_DOWN), std::string("Closing from client destructor")), false); @@ -181,7 +181,7 @@ void QuicClientTransport::processPacketData( const auto& token = clientConn_->statelessResetToken; if (statelessReset->token == token) { VLOG(4) << "Received Stateless Reset " << *this; - conn_->peerConnectionError = std::make_pair( + conn_->peerConnectionError = QuicError( QuicErrorCode(LocalErrorCode::CONNECTION_RESET), toString(LocalErrorCode::CONNECTION_RESET).str()); throw QuicInternalException("Peer reset", LocalErrorCode::NO_ERROR); @@ -564,8 +564,8 @@ void QuicClientTransport::processPacketData( if (conn_->qLogger) { conn_->qLogger->addTransportStateUpdate(getPeerClose(errMsg)); } - conn_->peerConnectionError = std::make_pair( - QuicErrorCode(connFrame.errorCode), std::move(errMsg)); + conn_->peerConnectionError = + QuicError(QuicErrorCode(connFrame.errorCode), std::move(errMsg)); throw QuicTransportException( "Peer closed", TransportErrorCode::NO_ERROR); break; @@ -1051,8 +1051,8 @@ void QuicClientTransport::errMessage( if (!happyEyeballsState.shouldWriteToFirstSocket && !happyEyeballsState.shouldWriteToSecondSocket) { runOnEvbAsync([errString = std::move(errStr)](auto self) { - auto quicError = std::make_pair( - QuicErrorCode(LocalErrorCode::CONNECT_FAILED), errString); + auto quicError = + QuicError(QuicErrorCode(LocalErrorCode::CONNECT_FAILED), errString); auto clientPtr = static_cast(self.get()); clientPtr->closeImpl(std::move(quicError), false, false); }); @@ -1069,7 +1069,7 @@ void QuicClientTransport::onReadError( // draining the socket. runOnEvbAsync([ex](auto self) { auto clientPtr = static_cast(self.get()); - clientPtr->closeNow(std::make_pair( + clientPtr->closeNow(QuicError( QuicErrorCode(LocalErrorCode::CONNECTION_ABANDONED), std::string(ex.what()))); }); @@ -1546,20 +1546,20 @@ void QuicClientTransport::start( } catch (const QuicTransportException& ex) { runOnEvbAsync([ex](auto self) { auto clientPtr = static_cast(self.get()); - clientPtr->closeImpl(std::make_pair( - QuicErrorCode(ex.errorCode()), std::string(ex.what()))); + clientPtr->closeImpl( + QuicError(QuicErrorCode(ex.errorCode()), std::string(ex.what()))); }); } catch (const QuicInternalException& ex) { runOnEvbAsync([ex](auto self) { auto clientPtr = static_cast(self.get()); - clientPtr->closeImpl(std::make_pair( - QuicErrorCode(ex.errorCode()), std::string(ex.what()))); + clientPtr->closeImpl( + QuicError(QuicErrorCode(ex.errorCode()), std::string(ex.what()))); }); } catch (const std::exception& ex) { LOG(ERROR) << "Connect failed " << ex.what(); runOnEvbAsync([ex](auto self) { auto clientPtr = static_cast(self.get()); - clientPtr->closeImpl(std::make_pair( + clientPtr->closeImpl(QuicError( QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), std::string(ex.what()))); }); diff --git a/quic/client/connector/QuicConnector.cpp b/quic/client/connector/QuicConnector.cpp index beeeed98d..f440ec61d 100644 --- a/quic/client/connector/QuicConnector.cpp +++ b/quic/client/connector/QuicConnector.cpp @@ -19,8 +19,7 @@ namespace quic { QuicConnector::QuicConnector(Callback* cb) : cb_(CHECK_NOTNULL(cb)) {} -void QuicConnector::onConnectionSetupError( - std::pair code) noexcept { +void QuicConnector::onConnectionSetupError(QuicError code) noexcept { if (cb_) { cb_->onConnectError(std::move(code)); } @@ -108,7 +107,7 @@ void QuicConnector::cleanUp() { void QuicConnector::cleanUpAndCloseSocket() { if (quicClient_) { - auto error = std::make_pair( + auto error = QuicError( quic::QuicErrorCode(quic::LocalErrorCode::SHUTTING_DOWN), std::string("shutting down")); quicClient_->close(std::move(error)); @@ -122,7 +121,7 @@ std::chrono::milliseconds QuicConnector::timeElapsed() { } void QuicConnector::timeoutExpired() noexcept { - auto error = std::make_pair( + auto error = QuicError( quic::QuicErrorCode(quic::LocalErrorCode::CONNECT_FAILED), std::string("connect operation timed out")); if (quicClient_) { diff --git a/quic/client/connector/QuicConnector.h b/quic/client/connector/QuicConnector.h index c07f7036e..8538557e6 100644 --- a/quic/client/connector/QuicConnector.h +++ b/quic/client/connector/QuicConnector.h @@ -31,8 +31,7 @@ class QuicConnector : private quic::QuicSocket::ConnectionSetupCallback, class Callback { public: virtual ~Callback() = default; - virtual void onConnectError( - std::pair errorCode) = 0; + virtual void onConnectError(QuicError errorCode) = 0; virtual void onConnectSuccess() = 0; }; @@ -78,8 +77,7 @@ class QuicConnector : private quic::QuicSocket::ConnectionSetupCallback, // QuicSocket::ConnectionSetupCallback overrides. void onFirstPeerPacketProcessed() noexcept override {} - void onConnectionSetupError( - std::pair code) noexcept override; + void onConnectionSetupError(QuicError code) noexcept override; void onTransportReady() noexcept override {} void onReplaySafe() noexcept override; diff --git a/quic/client/test/Mocks.h b/quic/client/test/Mocks.h index d980cbad8..2897b1e1a 100644 --- a/quic/client/test/Mocks.h +++ b/quic/client/test/Mocks.h @@ -76,9 +76,7 @@ class MockClientHandshake : public ClientHandshake { class MockQuicConnectorCallback : public quic::QuicConnector::Callback { public: - MOCK_METHOD1( - onConnectError, - void(std::pair)); + MOCK_METHOD1(onConnectError, void(QuicError)); MOCK_METHOD0(onConnectSuccess, void()); }; @@ -99,7 +97,7 @@ class MockQuicClientTransport : public quic::QuicClientTransport { void start(ConnectionSetupCallback* connSetupCb, ConnectionCallbackNew*) override { - auto cancelCode = std::make_pair( + auto cancelCode = QuicError( QuicErrorCode(LocalErrorCode::NO_ERROR), toString(LocalErrorCode::NO_ERROR).str()); diff --git a/quic/client/test/QuicConnectorTest.cpp b/quic/client/test/QuicConnectorTest.cpp index 49e8a5065..d1fadfae3 100644 --- a/quic/client/test/QuicConnectorTest.cpp +++ b/quic/client/test/QuicConnectorTest.cpp @@ -63,9 +63,7 @@ TEST_F(QuicConnectorTest, TestConnectSuccess) { TEST_F(QuicConnectorTest, TestConnectFailure) { EXPECT_CALL(cb_, onConnectError(_)) .Times(1) - .WillOnce(Invoke([this](std::pair) { - eventBase_.terminateLoopSoon(); - })); + .WillOnce(Invoke([this](QuicError) { eventBase_.terminateLoopSoon(); })); executeMockConnect( MockQuicClientTransport::TestType::Failure, std::chrono::milliseconds(200)); @@ -75,9 +73,7 @@ TEST_F(QuicConnectorTest, TestConnectFailure) { TEST_F(QuicConnectorTest, TestConnectTimeout) { EXPECT_CALL(cb_, onConnectError(_)) .Times(1) - .WillOnce(Invoke([this](std::pair) { - eventBase_.terminateLoopSoon(); - })); + .WillOnce(Invoke([this](QuicError) { eventBase_.terminateLoopSoon(); })); executeMockConnect( MockQuicClientTransport::TestType::Timeout, std::chrono::milliseconds(1)); eventBase_.loopForever(); diff --git a/quic/common/test/TestUtils.cpp b/quic/common/test/TestUtils.cpp index 5617d4dab..4179e29da 100644 --- a/quic/common/test/TestUtils.cpp +++ b/quic/common/test/TestUtils.cpp @@ -706,39 +706,19 @@ std::vector getQLogEventIndices( return indices; } -bool matchError( - std::pair> errorCode, - LocalErrorCode error) { - return errorCode.first.type() == QuicErrorCode::Type::LocalErrorCode && - *errorCode.first.asLocalErrorCode() == error; +bool matchError(QuicError errorCode, LocalErrorCode error) { + return errorCode.code.type() == QuicErrorCode::Type::LocalErrorCode && + *errorCode.code.asLocalErrorCode() == error; } -bool matchError( - std::pair> errorCode, - TransportErrorCode error) { - return errorCode.first.type() == QuicErrorCode::Type::TransportErrorCode && - *errorCode.first.asTransportErrorCode() == error; +bool matchError(QuicError errorCode, TransportErrorCode error) { + return errorCode.code.type() == QuicErrorCode::Type::TransportErrorCode && + *errorCode.code.asTransportErrorCode() == error; } -bool matchError( - std::pair> errorCode, - ApplicationErrorCode error) { - return errorCode.first.type() == QuicErrorCode::Type::ApplicationErrorCode && - *errorCode.first.asApplicationErrorCode() == error; -} - -bool matchError( - std::pair errorCode, - ApplicationErrorCode error) { - return errorCode.first.type() == QuicErrorCode::Type::ApplicationErrorCode && - *errorCode.first.asApplicationErrorCode() == error; -} - -bool matchError( - std::pair errorCode, - TransportErrorCode error) { - return errorCode.first.type() == QuicErrorCode::Type::TransportErrorCode && - *errorCode.first.asTransportErrorCode() == error; +bool matchError(QuicError errorCode, ApplicationErrorCode error) { + return errorCode.code.type() == QuicErrorCode::Type::ApplicationErrorCode && + *errorCode.code.asApplicationErrorCode() == error; } CongestionController::AckEvent::AckPacket makeAckPacketFromOutstandingPacket( diff --git a/quic/common/test/TestUtils.h b/quic/common/test/TestUtils.h index 0ea9c3d22..7ba9f7230 100644 --- a/quic/common/test/TestUtils.h +++ b/quic/common/test/TestUtils.h @@ -174,25 +174,9 @@ uint64_t computeExpectedDelay( uint8_t ackDelayExponent); // match error functions -bool matchError( - std::pair> errorCode, - LocalErrorCode error); - -bool matchError( - std::pair> errorCode, - TransportErrorCode error); - -bool matchError( - std::pair> errorCode, - ApplicationErrorCode error); - -bool matchError( - std::pair errorCode, - ApplicationErrorCode error); - -bool matchError( - std::pair errorCode, - TransportErrorCode error); +bool matchError(QuicError errorCode, LocalErrorCode error); +bool matchError(QuicError errorCode, TransportErrorCode error); +bool matchError(QuicError errorCode, ApplicationErrorCode error); ConnectionId getTestConnectionId( uint32_t hostId = 0, diff --git a/quic/fizz/client/test/QuicClientTransportTest.cpp b/quic/fizz/client/test/QuicClientTransportTest.cpp index 24880c221..a28734356 100644 --- a/quic/fizz/client/test/QuicClientTransportTest.cpp +++ b/quic/fizz/client/test/QuicClientTransportTest.cpp @@ -266,9 +266,7 @@ class StreamData { explicit StreamData(StreamId id) : id(id) {} - void setException( - const std::pair>& - err) { + void setException(const QuicError& err) { promise.setException(std::runtime_error(toString(err))); delete this; } @@ -387,9 +385,9 @@ TEST_P(QuicClientTransportIntegrationTest, TLSAlert) { client->getNonConstConn().qLogger = qLogger; EXPECT_CALL(clientConnSetupCallback, onConnectionSetupError(_)) .WillOnce(Invoke([&](const auto& errorCode) { - LOG(ERROR) << "error: " << errorCode.second; + LOG(ERROR) << "error: " << errorCode.message; const TransportErrorCode* transportError = - errorCode.first.asTransportErrorCode(); + errorCode.code.asTransportErrorCode(); EXPECT_NE(transportError, nullptr); client->close(folly::none); this->checkTransportSummaryEvent(qLogger); @@ -413,8 +411,8 @@ TEST_P(QuicClientTransportIntegrationTest, BadServerTest) { client->setTransportSettings(tp); EXPECT_CALL(clientConnSetupCallback, onConnectionSetupError(_)) .WillOnce(Invoke([&](const auto& errorCode) { - LOG(ERROR) << "error: " << errorCode.second; - const LocalErrorCode* localError = errorCode.first.asLocalErrorCode(); + LOG(ERROR) << "error: " << errorCode.message; + const LocalErrorCode* localError = errorCode.code.asLocalErrorCode(); EXPECT_NE(localError, nullptr); this->checkTransportSummaryEvent(qLogger); })); @@ -1078,16 +1076,10 @@ TEST_F(QuicClientTransportTest, SocketClosedDuringOnTransportReady) { GMOCK_METHOD0_(, noexcept, , onTransportReadyMock, void()); GMOCK_METHOD0_(, noexcept, , onReplaySafe, void()); GMOCK_METHOD0_(, noexcept, , onConnectionEnd, void()); - void onConnectionSetupError( - std::pair error) noexcept override { + void onConnectionSetupError(QuicError error) noexcept override { onConnectionError(std::move(error)); } - GMOCK_METHOD1_( - , - noexcept, - , - onConnectionError, - void(std::pair)); + GMOCK_METHOD1_(, noexcept, , onConnectionError, void(QuicError)); private: std::shared_ptr socket_; @@ -3036,7 +3028,7 @@ TEST_P( socketWrites.clear(); EXPECT_CALL(readCb, readError(streamId, _)); if (GetParam()) { - client->close(std::make_pair( + client->close(QuicError( QuicErrorCode(GenericApplicationErrorCode::UNKNOWN), std::string("stopping"))); EXPECT_TRUE(verifyFramePresent( @@ -3203,7 +3195,7 @@ TEST_P(QuicClientTransportAfterStartTestClose, CloseConnectionWithError) { deliverData(packet->coalesce()); socketWrites.clear(); if (GetParam()) { - client->close(std::make_pair( + client->close(QuicError( QuicErrorCode(GenericApplicationErrorCode::UNKNOWN), std::string("stopping"))); EXPECT_TRUE(verifyFramePresent( @@ -3336,7 +3328,7 @@ TEST_P(QuicClientTransportAfterStartTestClose, TimeoutsNotSetAfterClose) { 0 /* largestAcked */)); if (GetParam()) { - client->close(std::make_pair( + client->close(QuicError( QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), std::string("how about no"))); } else { @@ -4359,7 +4351,7 @@ TEST_F(QuicClientTransportAfterStartTest, ReceiveConnectionClose) { // Now the transport should be closed EXPECT_EQ( QuicErrorCode(TransportErrorCode::NO_ERROR), - client->getConn().localConnectionError->first); + client->getConn().localConnectionError->code); EXPECT_TRUE(client->isClosed()); EXPECT_TRUE(verifyFramePresent( socketWrites, @@ -4391,7 +4383,7 @@ TEST_F(QuicClientTransportAfterStartTest, ReceiveApplicationClose) { // Now the transport should be closed EXPECT_EQ( QuicErrorCode(TransportErrorCode::NO_ERROR), - client->getConn().localConnectionError->first); + client->getConn().localConnectionError->code); EXPECT_TRUE(client->isClosed()); EXPECT_TRUE(verifyFramePresent( socketWrites, @@ -4428,7 +4420,7 @@ TEST_F(QuicClientTransportAfterStartTest, ReceiveApplicationCloseNoError) { // Now the transport should be closed EXPECT_EQ( QuicErrorCode(TransportErrorCode::NO_ERROR), - client->getConn().localConnectionError->first); + client->getConn().localConnectionError->code); EXPECT_TRUE(client->isClosed()); EXPECT_TRUE(verifyFramePresent( socketWrites, @@ -4472,7 +4464,7 @@ TEST_F(QuicClientTransportAfterStartTest, DestroyWhileDraining) { TEST_F(QuicClientTransportAfterStartTest, CloseNowWhileDraining) { // Drain first with no active streams - auto err = std::make_pair( + auto err = QuicError( QuicErrorCode(LocalErrorCode::INTERNAL_ERROR), toString(LocalErrorCode::INTERNAL_ERROR).str()); client->close(err); @@ -4484,7 +4476,7 @@ TEST_F(QuicClientTransportAfterStartTest, CloseNowWhileDraining) { } TEST_F(QuicClientTransportAfterStartTest, ExpiredDrainTimeout) { - auto err = std::make_pair( + auto err = QuicError( QuicErrorCode(LocalErrorCode::INTERNAL_ERROR), toString(LocalErrorCode::INTERNAL_ERROR).str()); client->close(err); @@ -4497,7 +4489,7 @@ TEST_F(QuicClientTransportAfterStartTest, ExpiredDrainTimeout) { TEST_F(QuicClientTransportAfterStartTest, WriteThrowsExceptionWhileDraining) { // Drain first with no active streams - auto err = std::make_pair( + auto err = QuicError( QuicErrorCode(LocalErrorCode::INTERNAL_ERROR), toString(LocalErrorCode::INTERNAL_ERROR).str()); EXPECT_CALL(*sock, write(_, _)).WillRepeatedly(SetErrnoAndReturn(EBADF, -1)); @@ -4729,7 +4721,7 @@ TEST_F(QuicClientTransportAfterStartTest, OneCloseFramePerRtt) { // Close the client transport. There could be multiple writes given how many // ciphers we have. EXPECT_CALL(*sock, write(_, _)).Times(AtLeast(1)).WillRepeatedly(Return(10)); - client->close(std::make_pair( + client->close(QuicError( QuicErrorCode(LocalErrorCode::INTERNAL_ERROR), toString(LocalErrorCode::INTERNAL_ERROR).str())); EXPECT_TRUE(conn.lastCloseSentTime.hasValue()); diff --git a/quic/fizz/client/test/QuicClientTransportTestUtil.h b/quic/fizz/client/test/QuicClientTransportTestUtil.h index 4be1de1a3..4551a5de4 100644 --- a/quic/fizz/client/test/QuicClientTransportTestUtil.h +++ b/quic/fizz/client/test/QuicClientTransportTestUtil.h @@ -716,13 +716,13 @@ class QuicClientTransportTestBase : public virtual testing::Test { if (client->getConn().localConnectionError) { bool idleTimeout = false; const LocalErrorCode* localError = - client->getConn().localConnectionError->first.asLocalErrorCode(); + client->getConn().localConnectionError->code.asLocalErrorCode(); if (localError) { idleTimeout = (*localError == LocalErrorCode::IDLE_TIMEOUT); } if (!idleTimeout) { throw std::runtime_error( - toString(client->getConn().localConnectionError->first)); + toString(client->getConn().localConnectionError->code)); } } } diff --git a/quic/samples/echo/EchoClient.h b/quic/samples/echo/EchoClient.h index 1efe1c3bd..d57d3c459 100644 --- a/quic/samples/echo/EchoClient.h +++ b/quic/samples/echo/EchoClient.h @@ -50,10 +50,7 @@ class EchoClient : public quic::QuicSocket::ConnectionSetupCallback, << " on stream=" << streamId; } - void readError( - quic::StreamId streamId, - std::pair> - error) noexcept override { + void readError(quic::StreamId streamId, QuicError error) noexcept override { LOG(ERROR) << "EchoClient failed read from stream=" << streamId << ", error=" << toString(error); // A read error only terminates the ingress portion of the stream state. @@ -81,15 +78,13 @@ class EchoClient : public quic::QuicSocket::ConnectionSetupCallback, LOG(INFO) << "EchoClient connection end"; } - void onConnectionSetupError( - std::pair error) noexcept override { + void onConnectionSetupError(QuicError error) noexcept override { onConnectionError(std::move(error)); } - void onConnectionError( - std::pair error) noexcept override { - LOG(ERROR) << "EchoClient error: " << toString(error.first) - << "; errStr=" << error.second; + void onConnectionError(QuicError error) noexcept override { + LOG(ERROR) << "EchoClient error: " << toString(error.code) + << "; errStr=" << error.message; startDone_.post(); } @@ -104,10 +99,8 @@ class EchoClient : public quic::QuicSocket::ConnectionSetupCallback, sendMessage(id, pendingOutput_[id]); } - void onStreamWriteError( - quic::StreamId id, - std::pair> - error) noexcept override { + void onStreamWriteError(quic::StreamId id, QuicError error) noexcept + override { LOG(ERROR) << "EchoClient write error with stream=" << id << " error=" << toString(error); } diff --git a/quic/samples/echo/EchoHandler.h b/quic/samples/echo/EchoHandler.h index d871bdde7..78d89142d 100644 --- a/quic/samples/echo/EchoHandler.h +++ b/quic/samples/echo/EchoHandler.h @@ -47,15 +47,13 @@ class EchoHandler : public quic::QuicSocket::ConnectionSetupCallback, LOG(INFO) << "Socket closed"; } - void onConnectionSetupError( - std::pair error) noexcept override { + void onConnectionSetupError(QuicError error) noexcept override { onConnectionError(std::move(error)); } - void onConnectionError( - std::pair error) noexcept override { - LOG(ERROR) << "Socket error=" << toString(error.first) << " " - << error.second; + void onConnectionError(QuicError error) noexcept override { + LOG(ERROR) << "Socket error=" << toString(error.code) << " " + << error.message; } void readAvailable(quic::StreamId id) noexcept override { @@ -84,10 +82,7 @@ class EchoHandler : public quic::QuicSocket::ConnectionSetupCallback, } } - void readError( - quic::StreamId id, - std::pair> - error) noexcept override { + void readError(quic::StreamId id, QuicError error) noexcept override { LOG(ERROR) << "Got read error on stream=" << id << " error=" << toString(error); // A read error only terminates the ingress portion of the stream state. @@ -117,10 +112,8 @@ class EchoHandler : public quic::QuicSocket::ConnectionSetupCallback, echo(id, input_[id]); } - void onStreamWriteError( - quic::StreamId id, - std::pair> - error) noexcept override { + void onStreamWriteError(quic::StreamId id, QuicError error) noexcept + override { LOG(ERROR) << "write error with stream=" << id << " error=" << toString(error); } diff --git a/quic/server/QuicServerTransport.cpp b/quic/server/QuicServerTransport.cpp index b0339bfd0..87f7593ee 100644 --- a/quic/server/QuicServerTransport.cpp +++ b/quic/server/QuicServerTransport.cpp @@ -67,7 +67,7 @@ QuicServerTransport::~QuicServerTransport() { // transport. resetConnectionCallbacks(); closeImpl( - std::make_pair( + QuicError( QuicErrorCode(LocalErrorCode::SHUTTING_DOWN), std::string("Closing from server destructor")), false); @@ -416,15 +416,13 @@ void QuicServerTransport::onCryptoEventAvailable() noexcept { maybeNotifyTransportReady(); } catch (const QuicTransportException& ex) { VLOG(4) << "onCryptoEventAvailable() error " << ex.what() << " " << *this; - closeImpl( - std::make_pair(QuicErrorCode(ex.errorCode()), std::string(ex.what()))); + closeImpl(QuicError(QuicErrorCode(ex.errorCode()), std::string(ex.what()))); } catch (const QuicInternalException& ex) { VLOG(4) << "onCryptoEventAvailable() error " << ex.what() << " " << *this; - closeImpl( - std::make_pair(QuicErrorCode(ex.errorCode()), std::string(ex.what()))); + closeImpl(QuicError(QuicErrorCode(ex.errorCode()), std::string(ex.what()))); } catch (const std::exception& ex) { VLOG(4) << "read() error " << ex.what() << " " << *this; - closeImpl(std::make_pair( + closeImpl(QuicError( QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), std::string(ex.what()))); } diff --git a/quic/server/QuicServerWorker.cpp b/quic/server/QuicServerWorker.cpp index 0e160033d..5473bd73a 100644 --- a/quic/server/QuicServerWorker.cpp +++ b/quic/server/QuicServerWorker.cpp @@ -1296,7 +1296,7 @@ void QuicServerWorker::shutdownAllConnections(LocalErrorCode error) { transport->setTransportStatsCallback(nullptr); transport->setHandshakeFinishedCallback(nullptr); transport->closeNow( - std::make_pair(QuicErrorCode(error), std::string("shutting down"))); + QuicError(QuicErrorCode(error), std::string("shutting down"))); } // Shut down all transports with bound connection ids. @@ -1306,7 +1306,7 @@ void QuicServerWorker::shutdownAllConnections(LocalErrorCode error) { t->setTransportStatsCallback(nullptr); t->setHandshakeFinishedCallback(nullptr); t->closeNow( - std::make_pair(QuicErrorCode(error), std::string("shutting down"))); + QuicError(QuicErrorCode(error), std::string("shutting down"))); QUIC_STATS(statsCallback_, onConnectionClose, QuicErrorCode(error)); } } diff --git a/quic/server/async_tran/QuicServerAsyncTransport.cpp b/quic/server/async_tran/QuicServerAsyncTransport.cpp index 253fbc45a..455061629 100644 --- a/quic/server/async_tran/QuicServerAsyncTransport.cpp +++ b/quic/server/async_tran/QuicServerAsyncTransport.cpp @@ -37,11 +37,10 @@ void QuicServerAsyncTransport::onConnectionEnd() noexcept { closeNowImpl(std::move(ex)); } -void QuicServerAsyncTransport::onConnectionError( - std::pair code) noexcept { +void QuicServerAsyncTransport::onConnectionError(QuicError code) noexcept { folly::AsyncSocketException ex( folly::AsyncSocketException::UNKNOWN, - folly::to("Quic connection error", code.second)); + folly::to("Quic connection error", code.message)); closeNowImpl(std::move(ex)); } diff --git a/quic/server/async_tran/QuicServerAsyncTransport.h b/quic/server/async_tran/QuicServerAsyncTransport.h index a3949e9ea..9ceb2e27e 100644 --- a/quic/server/async_tran/QuicServerAsyncTransport.h +++ b/quic/server/async_tran/QuicServerAsyncTransport.h @@ -31,12 +31,10 @@ class QuicServerAsyncTransport : public QuicStreamAsyncTransport, void onNewUnidirectionalStream(StreamId id) noexcept override; void onStopSending(StreamId id, ApplicationErrorCode error) noexcept override; void onConnectionEnd() noexcept override; - void onConnectionSetupError( - std::pair code) noexcept override { + void onConnectionSetupError(QuicError code) noexcept override { onConnectionError(std::move(code)); } - void onConnectionError( - std::pair code) noexcept override; + void onConnectionError(QuicError code) noexcept override; void onTransportReady() noexcept override; }; } // namespace quic diff --git a/quic/server/state/ServerStateMachine.cpp b/quic/server/state/ServerStateMachine.cpp index 5f500c817..8adcd1409 100644 --- a/quic/server/state/ServerStateMachine.cpp +++ b/quic/server/state/ServerStateMachine.cpp @@ -1134,8 +1134,8 @@ void onServerReadDataFromOpen( if (conn.qLogger) { conn.qLogger->addTransportStateUpdate(getPeerClose(errMsg)); } - conn.peerConnectionError = std::make_pair( - QuicErrorCode(connFrame.errorCode), std::move(errMsg)); + conn.peerConnectionError = + QuicError(QuicErrorCode(connFrame.errorCode), std::move(errMsg)); if (getSendConnFlowControlBytesWire(conn) == 0 && conn.flowControlState.sumCurStreamBufferLen) { VLOG(2) << "Client gives up a flow control blocked connection"; @@ -1384,8 +1384,8 @@ void onServerReadDataFromClosed( } // we want to deliver app callbacks with the peer supplied error, // but send a NO_ERROR to the peer. - conn.peerConnectionError = std::make_pair( - QuicErrorCode(connFrame.errorCode), std::move(errMsg)); + conn.peerConnectionError = + QuicError(QuicErrorCode(connFrame.errorCode), std::move(errMsg)); break; } default: diff --git a/quic/server/test/QuicServerTransportTest.cpp b/quic/server/test/QuicServerTransportTest.cpp index 98ae1d771..cb2326d9e 100644 --- a/quic/server/test/QuicServerTransportTest.cpp +++ b/quic/server/test/QuicServerTransportTest.cpp @@ -245,7 +245,7 @@ TEST_F(QuicServerTransportTest, TimeoutsNotSetAfterClose) { *expected, 0 /* cipherOverhead */, 0 /* largestAcked */)); - server->close(std::make_pair( + server->close(QuicError( QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), std::string("how about no"))); server->idleTimeout().cancelTimeout(); @@ -270,7 +270,7 @@ TEST_F(QuicServerTransportTest, InvalidMigrationNoDrain) { *expected, 0 /* cipherOverhead */, 0 /* largestAcked */)); - server->close(std::make_pair( + server->close(QuicError( QuicErrorCode(TransportErrorCode::INVALID_MIGRATION), std::string("migration disabled"))); server->idleTimeout().cancelTimeout(); @@ -314,7 +314,7 @@ TEST_F(QuicServerTransportTest, RecvDataAfterIdleTimeout) { } TEST_F(QuicServerTransportTest, TestCloseConnectionWithError) { - server->close(std::make_pair( + server->close(QuicError( QuicErrorCode(GenericApplicationErrorCode::UNKNOWN), std::string("stopping"))); EXPECT_TRUE(verifyFramePresent( @@ -324,7 +324,7 @@ TEST_F(QuicServerTransportTest, TestCloseConnectionWithError) { } TEST_F(QuicServerTransportTest, TestCloseConnectionWithNoError) { - server->close(std::make_pair( + server->close(QuicError( QuicErrorCode(GenericApplicationErrorCode::UNKNOWN), std::string("stopping"))); EXPECT_TRUE(verifyFramePresent( @@ -377,7 +377,7 @@ TEST_F(QuicServerTransportTest, TestCloseConnectionWithNoErrorPendingStreams) { ++clientNextAppDataPacketNum, acks, PacketNumberSpace::AppData))); - server->close(std::make_pair( + server->close(QuicError( QuicErrorCode(GenericApplicationErrorCode::UNKNOWN), std::string("stopping"))); @@ -534,7 +534,7 @@ TEST_F(QuicServerTransportTest, NoDataExceptCloseProcessedAfterClosing) { auto packet = std::move(builder).buildPacket(); - server->close(std::make_pair( + server->close(QuicError( QuicErrorCode(GenericApplicationErrorCode::UNKNOWN), std::string("hello"))); EXPECT_TRUE(verifyFramePresent( @@ -1288,14 +1288,14 @@ TEST_F(QuicServerTransportTest, ReceiveConnectionClose) { deliverDataWithoutErrorCheck(packetToBuf(packet)); // Now the transport should be closed EXPECT_EQ( - server->getConn().localConnectionError->first, + server->getConn().localConnectionError->code, QuicErrorCode(TransportErrorCode::NO_ERROR)); EXPECT_EQ( - server->getConn().peerConnectionError->first, + server->getConn().peerConnectionError->code, QuicErrorCode(TransportErrorCode::NO_ERROR)); auto closedMsg = folly::to("Server closed by peer reason=", errMsg); - EXPECT_EQ(server->getConn().peerConnectionError->second, closedMsg); + EXPECT_EQ(server->getConn().peerConnectionError->message, closedMsg); EXPECT_TRUE(server->isClosed()); EXPECT_TRUE(verifyFramePresent( serverWrites, @@ -1331,13 +1331,13 @@ TEST_F(QuicServerTransportTest, ReceiveApplicationClose) { // Now the transport should be closed EXPECT_EQ( QuicErrorCode(TransportErrorCode::NO_ERROR), - server->getConn().localConnectionError->first); + server->getConn().localConnectionError->code); EXPECT_EQ( - server->getConn().peerConnectionError->first, + server->getConn().peerConnectionError->code, QuicErrorCode(GenericApplicationErrorCode::UNKNOWN)); auto closedMsg = folly::to("Server closed by peer reason=", errMsg); - EXPECT_EQ(server->getConn().peerConnectionError->second, closedMsg); + EXPECT_EQ(server->getConn().peerConnectionError->message, closedMsg); EXPECT_TRUE(server->isClosed()); EXPECT_TRUE(verifyFramePresent( serverWrites, @@ -1368,13 +1368,13 @@ TEST_F(QuicServerTransportTest, ReceiveConnectionCloseTwice) { // Now the transport should be closed EXPECT_EQ( QuicErrorCode(TransportErrorCode::NO_ERROR), - server->getConn().localConnectionError->first); + server->getConn().localConnectionError->code); EXPECT_EQ( - server->getConn().peerConnectionError->first, + server->getConn().peerConnectionError->code, QuicErrorCode(TransportErrorCode::NO_ERROR)); auto closedMsg = folly::to("Server closed by peer reason=", errMsg); - EXPECT_EQ(server->getConn().peerConnectionError->second, closedMsg); + EXPECT_EQ(server->getConn().peerConnectionError->message, closedMsg); EXPECT_TRUE(server->isClosed()); EXPECT_TRUE(verifyFramePresent( serverWrites, @@ -1533,7 +1533,7 @@ TEST_F( } EXPECT_TRUE(server->getConn().localConnectionError); EXPECT_EQ( - server->getConn().localConnectionError->second, "Migration disabled"); + server->getConn().localConnectionError->message, "Migration disabled"); EXPECT_EQ(server->getConn().streamManager->streamCount(), 0); } @@ -1650,7 +1650,7 @@ TEST_F(QuicServerTransportTest, ShortHeaderPacketWithNoFramesAfterClose) { })); // Close the connection - server->close(std::make_pair( + server->close(QuicError( QuicErrorCode(TransportErrorCode::INTERNAL_ERROR), std::string("test close"))); server->idleTimeout().cancelTimeout(); @@ -1746,7 +1746,7 @@ TEST_P( } EXPECT_TRUE(server->getConn().localConnectionError); EXPECT_EQ( - server->getConn().localConnectionError->second, + server->getConn().localConnectionError->message, "Probing not supported yet"); std::vector indices = @@ -2068,7 +2068,7 @@ TEST_P( EXPECT_TRUE(server->getConn().localConnectionError); EXPECT_EQ( - server->getConn().localConnectionError->second, + server->getConn().localConnectionError->message, "Probing not supported yet"); } @@ -2100,7 +2100,7 @@ TEST_F(QuicServerTransportTest, TooManyMigrations) { } EXPECT_TRUE(server->getConn().localConnectionError); EXPECT_EQ( - server->getConn().localConnectionError->second, "Too many migrations"); + server->getConn().localConnectionError->message, "Too many migrations"); EXPECT_TRUE(server->isClosed()); std::vector indices = getQLogEventIndices(QLogEventType::PacketDrop, qLogger); @@ -3079,7 +3079,7 @@ TEST_F( } EXPECT_TRUE(server->getConn().localConnectionError); EXPECT_EQ( - server->getConn().localConnectionError->second, + server->getConn().localConnectionError->message, "Migration not allowed during handshake"); EXPECT_TRUE(server->isClosed()); @@ -3113,7 +3113,7 @@ TEST_F( } EXPECT_TRUE(server->getConn().localConnectionError); EXPECT_EQ( - server->getConn().localConnectionError->second, + server->getConn().localConnectionError->message, "Migration not allowed during handshake"); } @@ -3146,7 +3146,7 @@ TEST_F( } EXPECT_TRUE(server->getConn().localConnectionError); EXPECT_EQ( - server->getConn().localConnectionError->second, + server->getConn().localConnectionError->message, "Migration not allowed during handshake"); } @@ -3177,7 +3177,7 @@ TEST_F( } EXPECT_TRUE(server->getConn().localConnectionError); EXPECT_EQ( - server->getConn().localConnectionError->second, "Migration disabled"); + server->getConn().localConnectionError->message, "Migration disabled"); EXPECT_EQ(server->getConn().streamManager->streamCount(), 0); EXPECT_EQ(server->getConn().pendingZeroRttData, nullptr); EXPECT_EQ(server->getConn().pendingOneRttData, nullptr); @@ -3569,7 +3569,7 @@ TEST_F(QuicUnencryptedServerTransportTest, TestCloseWhileAsyncPending) { EXPECT_CALL(handshakeFinishedCallback, onHandshakeUnfinished()); recvClientFinished(); - server->close(std::make_pair( + server->close(QuicError( QuicErrorCode(GenericApplicationErrorCode::UNKNOWN), std::string("hello"))); EXPECT_TRUE(server->isClosed()); diff --git a/quic/server/test/QuicServerTransportTestUtil.h b/quic/server/test/QuicServerTransportTestUtil.h index 54522b0c2..537e8bcf0 100644 --- a/quic/server/test/QuicServerTransportTestUtil.h +++ b/quic/server/test/QuicServerTransportTestUtil.h @@ -477,13 +477,13 @@ class QuicServerTransportTestBase : public virtual testing::Test { if (server->getConn().localConnectionError) { bool idleTimeout = false; const LocalErrorCode* localError = - server->getConn().localConnectionError->first.asLocalErrorCode(); + server->getConn().localConnectionError->code.asLocalErrorCode(); if (localError) { idleTimeout = (*localError == LocalErrorCode::IDLE_TIMEOUT); } if (!idleTimeout) { throw std::runtime_error( - toString(server->getConn().localConnectionError->first)); + toString(server->getConn().localConnectionError->code)); } } } diff --git a/quic/state/StateData.h b/quic/state/StateData.h index 88e2754b7..1d32e6ef0 100644 --- a/quic/state/StateData.h +++ b/quic/state/StateData.h @@ -519,10 +519,10 @@ struct QuicConnectionStateBase : public folly::DelayedDestruction { folly::Optional localAddress; // Local error on the connection. - folly::Optional> localConnectionError; + folly::Optional localConnectionError; // Error sent on the connection by the peer. - folly::Optional> peerConnectionError; + folly::Optional peerConnectionError; // Supported versions in order of preference. Only meaningful to clients. // TODO: move to client only conn state. diff --git a/quic/tools/tperf/tperf.cpp b/quic/tools/tperf/tperf.cpp index c348d6914..a0032fb7c 100644 --- a/quic/tools/tperf/tperf.cpp +++ b/quic/tools/tperf/tperf.cpp @@ -275,15 +275,13 @@ class ServerStreamHandler : public quic::QuicSocket::ConnectionSetupCallback, sock_.reset(); } - void onConnectionSetupError( - std::pair error) noexcept override { + void onConnectionSetupError(QuicError error) noexcept override { onConnectionError(std::move(error)); } - void onConnectionError( - std::pair error) noexcept override { - LOG(ERROR) << "Conn errorCoded=" << toString(error.first) - << ", errorMsg=" << error.second; + void onConnectionError(QuicError error) noexcept override { + LOG(ERROR) << "Conn errorCoded=" << toString(error.code) + << ", errorMsg=" << error.message; } void onTransportReady() noexcept override { @@ -325,10 +323,7 @@ class ServerStreamHandler : public quic::QuicSocket::ConnectionSetupCallback, LOG(INFO) << "read available for stream id=" << id; } - void readError( - quic::StreamId id, - std::pair> - error) noexcept override { + void readError(quic::StreamId id, QuicError error) noexcept override { LOG(ERROR) << "Got read error on stream=" << id << " error=" << toString(error); // A read error only terminates the ingress portion of the stream state. @@ -361,10 +356,8 @@ class ServerStreamHandler : public quic::QuicSocket::ConnectionSetupCallback, } } - void onStreamWriteError( - quic::StreamId id, - std::pair> - error) noexcept override { + void onStreamWriteError(quic::StreamId id, QuicError error) noexcept + override { LOG(ERROR) << "write error with stream=" << id << " error=" << toString(error); } @@ -648,7 +641,7 @@ class TPerfClient : public quic::QuicSocket::ConnectionSetupCallback, void readError( quic::StreamId /*streamId*/, - std::pair> + QuicError /*error*/) noexcept override { // A read error only terminates the ingress portion of the stream state. // Your application should probably terminate the egress portion via @@ -686,14 +679,12 @@ class TPerfClient : public quic::QuicSocket::ConnectionSetupCallback, eventBase_.terminateLoopSoon(); } - void onConnectionSetupError( - std::pair error) noexcept override { + void onConnectionSetupError(QuicError error) noexcept override { onConnectionError(std::move(error)); } - void onConnectionError( - std::pair error) noexcept override { - LOG(ERROR) << "TPerfClient error: " << toString(error.first); + void onConnectionError(QuicError error) noexcept override { + LOG(ERROR) << "TPerfClient error: " << toString(error.code); eventBase_.terminateLoopSoon(); } @@ -703,10 +694,8 @@ class TPerfClient : public quic::QuicSocket::ConnectionSetupCallback, << " is write ready with maxToSend=" << maxToSend; } - void onStreamWriteError( - quic::StreamId id, - std::pair> - error) noexcept override { + void onStreamWriteError(quic::StreamId id, QuicError error) noexcept + override { LOG(ERROR) << "TPerfClient write error with stream=" << id << " error=" << toString(error); }