Alternative Improve _uploadReadByte (#2656)

* add opportunity for more than one retry to _uploadReadByte

* an alternative timeout-based method to making _uploadReadByte more resilient

* move timing variables in the correct scope

* implement and use client.getTimeout instead of hard-coded timeout in _uploadReadByte

* add missing return

* some refactoring to address respecting the timeout in a potentially deadlocked connection

* fix spelling in comment

* address review comments; move impl to cpp file for getTimeout, and remove local variable for currentMillis

* remove redundant cast

* need to check for timeout outside the inner while as well

* update WebUpdate example to print something in unexpected callback condition

* update log_e messages per review comments

libraries/WebServer/src/Parsing.cpp

@@ -304,11 +304,41 @@ void WebServer::_uploadWriteByte(uint8_t b){
 
 int WebServer::_uploadReadByte(WiFiClient& client){
   int res = client.read();
-  if(res == -1){
-    while(!client.available() && client.connected())
-      delay(2);
-    res = client.read();
+  if(res < 0) {
+    // keep trying until you either read a valid byte or timeout
+    unsigned long startMillis = millis();
+    long timeoutIntervalMillis = client.getTimeout();
+    boolean timedOut = false;
+    for(;;) {
+      // loosely modeled after blinkWithoutDelay pattern
+      while(!timedOut && !client.available() && client.connected()){
+        delay(2);
+        timedOut = millis() - startMillis >= timeoutIntervalMillis;
+      }
+
+      res = client.read();
+      if(res >= 0) {
+        return res; // exit on a valid read
+      }
+      // NOTE: it is possible to get here and have all of the following
+      //       assertions hold true
+      //
+      //       -- client.available() > 0
+      //       -- client.connected == true
+      //       -- res == -1
+      //
+      //       a simple retry strategy overcomes this which is to say the
+      //       assertion is not permanent, but the reason that this works
+      //       is elusive, and possibly indicative of a more subtle underlying
+      //       issue
+
+      timedOut = millis() - startMillis >= timeoutIntervalMillis;
+      if(timedOut) {
+        return res; // exit on a timeout
+      }
+    }
   }
+
   return res;
 }