Commit 0ea1868e authored by Jason Rahman's avatar Jason Rahman Committed by Facebook Github Bot

RFC: Include timeout duration in exception message

Summary: For debugging purposes, it's often extremely useful to have the duration of the expired timeout in the exception message (in case the timeout was excessively low, or to understand exactly how long the operation ran before failing due to timeout). Here we make a best effort to include the timeout when possible.

Reviewed By: yfeldblum

Differential Revision: D4476038

fbshipit-source-id: f2d0aa5e1f6367a3cfe20579c27577ab91fd8d9c
parent dc3979b0
...@@ -106,8 +106,9 @@ class AsyncSSLSocketConnector: public AsyncSocket::ConnectCallback, ...@@ -106,8 +106,9 @@ class AsyncSSLSocketConnector: public AsyncSocket::ConnectCallback,
timeoutLeft = timeout_ - (curTime - startTime_); timeoutLeft = timeout_ - (curTime - startTime_);
if (timeoutLeft <= 0) { if (timeoutLeft <= 0) {
AsyncSocketException ex(AsyncSocketException::TIMED_OUT, AsyncSocketException ex(
"SSL connect timed out"); AsyncSocketException::TIMED_OUT,
folly::sformat("SSL connect timed out after {}ms", timeout_));
fail(ex); fail(ex);
delete this; delete this;
return; return;
...@@ -561,10 +562,10 @@ void AsyncSSLSocket::setServerName(std::string serverName) noexcept { ...@@ -561,10 +562,10 @@ void AsyncSSLSocket::setServerName(std::string serverName) noexcept {
#endif // FOLLY_OPENSSL_HAS_SNI #endif // FOLLY_OPENSSL_HAS_SNI
void AsyncSSLSocket::timeoutExpired() noexcept { void AsyncSSLSocket::timeoutExpired(
std::chrono::milliseconds timeout) noexcept {
if (state_ == StateEnum::ESTABLISHED && if (state_ == StateEnum::ESTABLISHED &&
(sslState_ == STATE_CACHE_LOOKUP || (sslState_ == STATE_CACHE_LOOKUP || sslState_ == STATE_ASYNC_PENDING)) {
sslState_ == STATE_ASYNC_PENDING)) {
sslState_ = STATE_ERROR; sslState_ = STATE_ERROR;
// We are expecting a callback in restartSSLAccept. The cache lookup // We are expecting a callback in restartSSLAccept. The cache lookup
// and rsa-call necessarily have pointers to this ssl socket, so delay // and rsa-call necessarily have pointers to this ssl socket, so delay
...@@ -579,9 +580,12 @@ void AsyncSSLSocket::timeoutExpired() noexcept { ...@@ -579,9 +580,12 @@ void AsyncSSLSocket::timeoutExpired() noexcept {
assert(state_ == StateEnum::ESTABLISHED && assert(state_ == StateEnum::ESTABLISHED &&
(sslState_ == STATE_CONNECTING || sslState_ == STATE_ACCEPTING)); (sslState_ == STATE_CONNECTING || sslState_ == STATE_ACCEPTING));
DestructorGuard dg(this); DestructorGuard dg(this);
AsyncSocketException ex(AsyncSocketException::TIMED_OUT, AsyncSocketException ex(
(sslState_ == STATE_CONNECTING) ? AsyncSocketException::TIMED_OUT,
"SSL connect timed out" : "SSL accept timed out"); folly::sformat(
"SSL {} timed out after {}ms",
(sslState_ == STATE_CONNECTING) ? "connect" : "accept",
timeout.count()));
failHandshake(__func__, ex); failHandshake(__func__, ex);
} }
} }
...@@ -990,8 +994,8 @@ AsyncSSLSocket::restartSSLAccept() ...@@ -990,8 +994,8 @@ AsyncSSLSocket::restartSSLAccept()
} }
if (sslState_ == STATE_ERROR) { if (sslState_ == STATE_ERROR) {
// go straight to fail if timeout expired during lookup // go straight to fail if timeout expired during lookup
AsyncSocketException ex(AsyncSocketException::TIMED_OUT, AsyncSocketException ex(
"SSL accept timed out"); AsyncSocketException::TIMED_OUT, "SSL accept timed out");
failHandshake(__func__, ex); failHandshake(__func__, ex);
return; return;
} }
......
...@@ -123,32 +123,31 @@ class AsyncSSLSocket : public virtual AsyncSocket { ...@@ -123,32 +123,31 @@ class AsyncSSLSocket : public virtual AsyncSocket {
noexcept = 0; noexcept = 0;
}; };
class HandshakeTimeout : public AsyncTimeout { class Timeout : public AsyncTimeout {
public: public:
HandshakeTimeout(AsyncSSLSocket* sslSocket, EventBase* eventBase) Timeout(AsyncSSLSocket* sslSocket, EventBase* eventBase)
: AsyncTimeout(eventBase) : AsyncTimeout(eventBase), sslSocket_(sslSocket) {}
, sslSocket_(sslSocket) {}
virtual void timeoutExpired() noexcept { bool scheduleTimeout(TimeoutManager::timeout_type timeout) {
sslSocket_->timeoutExpired(); timeout_ = timeout;
return AsyncTimeout::scheduleTimeout(timeout);
} }
private: bool scheduleTimeout(uint32_t timeoutMs) {
AsyncSSLSocket* sslSocket_; return scheduleTimeout(std::chrono::milliseconds{timeoutMs});
}; }
// Timer for if we fallback from SSL connects to TCP connects TimeoutManager::timeout_type getTimeout() {
class ConnectionTimeout : public AsyncTimeout { return timeout_;
public: }
ConnectionTimeout(AsyncSSLSocket* sslSocket, EventBase* eventBase)
: AsyncTimeout(eventBase), sslSocket_(sslSocket) {}
virtual void timeoutExpired() noexcept override { virtual void timeoutExpired() noexcept override {
sslSocket_->timeoutExpired(); sslSocket_->timeoutExpired(timeout_);
} }
private: private:
AsyncSSLSocket* sslSocket_; AsyncSSLSocket* sslSocket_;
TimeoutManager::timeout_type timeout_;
}; };
/** /**
...@@ -545,7 +544,7 @@ class AsyncSSLSocket : public virtual AsyncSocket { ...@@ -545,7 +544,7 @@ class AsyncSSLSocket : public virtual AsyncSocket {
void setServerName(std::string serverName) noexcept; void setServerName(std::string serverName) noexcept;
#endif // FOLLY_OPENSSL_HAS_SNI #endif // FOLLY_OPENSSL_HAS_SNI
void timeoutExpired() noexcept; void timeoutExpired(std::chrono::milliseconds timeout) noexcept;
/** /**
* Get the list of supported ciphers sent by the client in the client's * Get the list of supported ciphers sent by the client in the client's
...@@ -781,8 +780,8 @@ class AsyncSSLSocket : public virtual AsyncSocket { ...@@ -781,8 +780,8 @@ class AsyncSSLSocket : public virtual AsyncSocket {
HandshakeCB* handshakeCallback_{nullptr}; HandshakeCB* handshakeCallback_{nullptr};
SSL* ssl_{nullptr}; SSL* ssl_{nullptr};
SSL_SESSION *sslSession_{nullptr}; SSL_SESSION *sslSession_{nullptr};
HandshakeTimeout handshakeTimeout_; Timeout handshakeTimeout_;
ConnectionTimeout connectionTimeout_; Timeout connectionTimeout_;
// whether the SSL session was resumed using session ID or not // whether the SSL session was resumed using session ID or not
bool sessionIDResumed_{false}; bool sessionIDResumed_{false};
......
...@@ -1771,7 +1771,9 @@ void AsyncSocket::timeoutExpired() noexcept { ...@@ -1771,7 +1771,9 @@ void AsyncSocket::timeoutExpired() noexcept {
// Unregister for I/O events. // Unregister for I/O events.
if (connectCallback_) { if (connectCallback_) {
AsyncSocketException ex( AsyncSocketException ex(
AsyncSocketException::TIMED_OUT, "connect timed out"); AsyncSocketException::TIMED_OUT,
folly::sformat(
"connect timed out after {}ms", connectTimeout_.count()));
failConnect(__func__, ex); failConnect(__func__, ex);
} else { } else {
// we faced a connect error without a connect callback, which could // we faced a connect error without a connect callback, which could
...@@ -1782,7 +1784,9 @@ void AsyncSocket::timeoutExpired() noexcept { ...@@ -1782,7 +1784,9 @@ void AsyncSocket::timeoutExpired() noexcept {
} }
} else { } else {
// a normal write operation timed out // a normal write operation timed out
AsyncSocketException ex(AsyncSocketException::TIMED_OUT, "write timed out"); AsyncSocketException ex(
AsyncSocketException::TIMED_OUT,
folly::sformat("write timed out after {}ms", sendTimeout_));
failWrite(__func__, ex); failWrite(__func__, ex);
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment