Adjust error handling around socket closure for debugger

I had previously made the error handling in DebuggerCommand::Receive() too strict, and we'd fail to read available data from the socket when it was closed right after the data was sent. Loosened it, so if there appears to be data present we'll let the thrift layer have a chance to read the data.

I also moved the remaining log messages in that function to traces. There are cases where it is nice to see such a log message printed, but those are very rare and have not happened in the wild in the past many months. Log messages here particularly from the signal polling thread make our test output non-deterministic, and there's higher value in having stable tests :)

hphp/runtime/debugger/debugger_command.cpp

@@ -87,8 +87,10 @@ bool DebuggerCommand::Receive(DebuggerThriftBuffer &thrift,
     TRACE_RB(1, "DebuggerCommand::Receive: error %d\n", errorNumber);
     return errorNumber != EINTR; // Treat signals as timeouts
   }
-  // Any error bits set indicate that we have nothing to read, so fail.
-  if (fds[0].revents != POLLIN) {
+  // If we don't have any data to read (POLLIN) then we're done. If we
+  // do have data we'll attempt to read and decode it below, even if
+  // there are other error bits set.
+  if (!(fds[0].revents & POLLIN)) {
     TRACE_RB(1, "DebuggerCommand::Receive: revents %d\n", fds[0].revents);
     return true;
   }
@@ -103,7 +105,7 @@ bool DebuggerCommand::Receive(DebuggerThriftBuffer &thrift,
     // Note: this error case is difficult to test. But, it's exactly the same
     // as the error noted below. Make sure to keep handling of both of these
     // errors in sync.
-    Logger::Error("%s: socket error receiving command", caller);
+    TRACE_RB(1, "%s: socket error receiving command", caller);
     return true;
   }
 
@@ -147,7 +149,7 @@ bool DebuggerCommand::Receive(DebuggerThriftBuffer &thrift,
     }
 
     default:
-      Logger::Error("%s: received bad cmd type: %d", caller, type);
+      TRACE_RB(1, "%s: received bad cmd type: %d", caller, type);
       cmd.reset();
       return true;
   }
@@ -155,7 +157,7 @@ bool DebuggerCommand::Receive(DebuggerThriftBuffer &thrift,
     // Note: this error case is easily tested, and we have a test for it. But
     // the error case noted above is quite difficult to test. Keep these two
     // in sync.
-    Logger::Error("%s: socket error receiving command", caller);
+    TRACE_RB(1, "%s: socket error receiving command", caller);
     cmd.reset();
   }
   return true;

hphp/test/quick/debugger/error_bad_cmd_type_receive.php.expectf

@@ -10,6 +10,5 @@ Breakpoint 1 reached on line 3 of %s/error_bad_cmd_type_receive.php
 
 internaltesting badcmdtypereceive
 Executing internal test...
-Receive for command: received bad cmd type: 20001
 Unable to communicate with server. Server's down?
 Unable to reconnect to server, exiting.

hphp/test/quick/debugger/error_bad_cmd_type_send.php.expectf

@@ -10,4 +10,3 @@ Breakpoint 1 reached on line 3 of %s/error_bad_cmd_type_send.php
 
 internaltesting badcmdtypesend
 Executing internal test...
-DebuggerProxy::processInterrupt(): received bad cmd type: 20001

hphp/test/quick/debugger/error_short_cmd_receive.php.expectf

@@ -11,6 +11,5 @@ Breakpoint 1 reached on line 3 of %s/error_short_cmd_receive.php
 internaltesting shortcmdreceive
 Executing internal test...
 DebuggerCommand::recv(): a socket error has happened
-Receive for command: socket error receiving command
 Unable to communicate with server. Server's down?
 Unable to reconnect to server, exiting.

hphp/test/quick/debugger/error_short_cmd_send.php.expectf

@@ -11,4 +11,3 @@ Breakpoint 1 reached on line 3 of %s/error_short_cmd_send.php
 internaltesting shortcmdsend
 Executing internal test...
 DebuggerCommand::recv(): a socket error has happened
-DebuggerProxy::processInterrupt(): socket error receiving command