Add checking for timeout outside 'peek'

BedrockCore.cpp

@@ -53,6 +53,18 @@ uint64_t BedrockCore::_getRemainingTime(const BedrockCommand& command) {
     return min(processTimeout, adjustedTimeout);
 }
 
+bool BedrockCore::isTimedOut(BedrockCommand& command) {
+    try {
+        _getRemainingTime(command);
+    } catch (const SException& e) {
+        // Yep, timed out.
+        _handleCommandException(command, e);
+        command.complete = true;
+        return true;
+    }
+    return false;
+}
+
 bool BedrockCore::peekCommand(BedrockCommand& command) {
     AutoTimer timer(command, BedrockCommand::PEEK);
     // Convenience references to commonly used properties.

BedrockCore.h

@@ -21,6 +21,11 @@ class BedrockCore : public SQLiteCore {
         uint64_t _start;
     };
 
+    // Checks if a command has already timed out. Like `peekCommand` without doing any work. Returns `true` and sets
+    // the same command state as `peekCommand` would if the command has timed out. Returns `false` and does nothing if
+    // the command hasn't timed out.
+    bool isTimedOut(BedrockCommand& command);
+
     // Peek lets you pre-process a command. It will be called on each command before `process` is called on the same
     // command, and it *may be called multiple times*. Preventing duplicate actions on calling peek multiple times is
     // up to the implementer, and may happen *across multiple servers*. I.e., a follower server may call `peek`, and on

BedrockServer.cpp

@@ -703,6 +703,22 @@ void BedrockServer::worker(SData& args,
                 continue;
             }
 
+            // If the command has already timed out when we get it, we can return early here without peeking it.
+            // We'd also catch that the command timed out in `peek`, but this can cause some weird side-effects. For
+            // instance, we saw QUORUM commands that make HTTPS requests time out in the sync thread, which caused them
+            // to be returned to the main queue, where they would have timed out in `peek`, but it was never called
+            // because the commands already had a HTTPS request attached, and then they were immediately re-sent to the
+            // sync queue, because of the QUORUM consistency requirement, resulting in an endless loop.
+            if (core.isTimedOut(command)) {
+                if (command.initiatingPeerID) {
+                    // Escalated command. Give it back to the sync thread to respond.
+                    syncNodeCompletedCommands.push(move(command));
+                } else {
+                    server._reply(command);
+                }
+                continue;
+            }
+
             // Check if this command would be likely to cause a crash
             if (server._wouldCrash(command)) {
                 // If so, make a lot of noise, and respond 500 without processing it.