WiFiClient.connected() returns false while data are still available in receive buffer

[JAndrassy]

Basic Infos

• [x ] This issue complies with the issue POLICY doc.
• [x ] I have read the documentation at readthedocs and the issue is not addressed there.
• [x ] I have tested that the issue is present in current master branch (aka latest git).
• [x ] I have searched the issue tracker for a similar issue.
• [x ] If there is a stack dump, I have decoded it.
• [ x] I have filled out all fields below.

Platform

• Hardware: [ESP-12|ESP-01|ESP-07|ESP8285 device|other]
• Core Version: [since 2.4.2]
• Development Env: [Arduino IDE|Platformio|Make|other]
• Operating System: [Windows|Ubuntu|MacOS]

Settings in IDE

• Module: [Generic ESP8266 Module|Wemos D1 mini r2|Nodemcu|other]
• Flash Mode: [qio|dio|other]
• Flash Size: [4MB/1MB]
• lwip Variant: [v1.4|v2 Lower Memory|Higher Bandwidth]
• Reset Method: [ck|nodemcu]
• Flash Frequency: [40Mhz]
• CPU Frequency: [80Mhz|160MHz]
• Upload Using: [OTA|SERIAL]
• Upload Speed: [115200|other] (serial upload only)

Problem Description

WiFiClient.connected() and WiFiClient bool() operator should return true while data are available in receive buffer. Commit b08d282#diff-a5fd274181d082a3211a59cb42a7e0d8 released in 2.4.2 changed this.

The SSL Clients have it right.

MCVE Sketch

#include <Arduino.h>

#include <ESP8266WiFi.h>

#ifndef STASSID
#define STASSID "your-ssid"
#define STAPSK  "your-password"
#endif

const char* ssid = STASSID;
const char* pass = STAPSK;

const char* server = "arduino.cc";

WiFiClient client;

void setup() {
  Serial.begin(115200);
  delay(10);

  Serial.println();
  Serial.print("Connecting to ");
  Serial.println(ssid);

  WiFi.mode(WIFI_STA);
  WiFi.begin(ssid, pass);

  while (WiFi.status() != WL_CONNECTED) {
    delay(500);
    Serial.print(".");
  }
  Serial.println("");
  Serial.println("WiFi connected");

  Serial.println("Starting connection to server...");
  if (client.connect(server, 80)) {
    Serial.println("connected to server");
    client.println("GET /asciilogo.txt HTTP/1.1");
    client.print("Host: ");
    client.println(server);
    client.println("Connection: close");
    client.println();
  }
}

void loop() {

  if (client.available()) {
    char c = client.read();
    Serial.write(c);
  }

  if (!client.connected()) {
    Serial.println();
    Serial.println("disconnecting from server.");
    client.stop();

    while (true) {
      delay(100);
    }
  }
}

Debug Messages

Connecting to ---
.........
WiFi connected
Starting connection to server...
connected to server
HTTP/1.1 200 OK
Server: nginx
Date: Sat, 02 Nov 2019 05:32:18 GMT
Content-Type: text/plain
Content-Length: 2263
Last-Modified: Wed, 02 Oct 2013 13:46:47 GMT
Connection: close
Vary: Accept-Encoding
ETag: "524c23c7-8d7"
Accept-Ranges: bytes


           `:;;;,`                      .:;;:.           
        .;;;;;;;;;;;`                :;;;;;;;;;;:     TM 
      `;;;;;;;;;;;;;;;`            :;;;;;;;;;;;;;;;      
     :;;;;;;;;;;;;;;;;;;         `;;;;;;;;;;;;;;;;;;     
    ;;;;;;;;;;;;;;;;;;;;;       .;;;;;;;;;;;;;;;;;;;;    
   ;;;;;;;;:`   `;;;;;;;;;     ,;;;;;;;;.`   .;;;;;;;;   
  .;;;;;;,         :;;;;;;;   .;;;;;;;          ;;;;;;;  
  ;;;;;;             ;;;;;;;  ;;;;;;,            ;;;;;;. 
 ,;;;;;               ;;;;;;.;;;;;;`              ;;;;;; 
 ;;;;;.                ;;;;;;;;;;;`      ```       ;;;;;`
 ;;;;;                  ;;;;;;;;;,       ;;;       .;;;;;
`;;;;:                  `;;;;;;;;        ;;;        ;;;;;
,;;;;`    `,,,,,,,,      ;;;;;;;      .,,;;;,,,     ;;;;;
:;;;;`    .;;;;;;;;       ;;;;;,      :;;;;;;;;     ;;;;;
:;;;;`    .;;;;;;;;      `;;;;;;      :;;;;;;;;     ;;;;;
.;;;;.                   ;;;;;;;.        ;;;        ;;;;;
 ;;;;;                  ;;;;;;;;;        ;;;        ;;;;;
 ;;;;;                 .;;;;;;;;;;       ;;;       ;;;;;,
 ;;;;;;               `;;;;;;;;;;;;                ;;;;; 
 `;;;;;,             .;;;;;; ;;;;;;;           
disconnecting from server.

the HTTP response is cut, because immediately after last TCP packet the TCP connection is closed by HTTP server and connected() returns false

[devyte]

WiFiClient.connected() and WiFiClient bool() operator should return true while data are available in receive buffer

No, they shouldn't. Connected means connected, not data available. Data availability is obtained from available() method.

The logic in your loop is flawed. You read one byte, then check connection, and bail out if the connection is gone. You aren't taking into account buffering, which means that the server could be done sending and close the connection, but on your end you're not yet done receiving the buffered bytes from the client.

There were bugs prior to 2.4.2 in the socket logic inside WiFiClient, where some cases of the TCP logic flow were not handled. It caused anything from stuck forever reading to dropped bytes to lost acks. The buggy behavior was made worse due to Frankenstein logic in user code. The current behavior of WiFiClient is on purpose and the product of extensive work done by @d-a-v and reviewed by me, as a result of a long number of reported issues.

The ssl client was implemented by @earlephilhower on top of the normal client. Due to encryption, there was a concession made there, but the same logic applies as for WiFiClient.

There are examples that show how what you're trying to do should be done, showcasing the code logic to use.

Closing due to on purpose.

[JAndrassy]

https://www.arduino.cc/en/Reference/WiFiClientConnected

Whether or not the client is connected. Note that a client is considered connected if the connection has been closed but there is still unread data.

all other libraries have it like this
and it was implemented by @igrr this way until 2.4.0

uint8_t WiFiClient::connected()
{
    if (!_client)
        return 0;

    return _client->state() == ESTABLISHED || available();
}

the logic of the MCVE is to show the error

[devyte]

Correct, and the reference is wrong. One case is: the other side dropped the connection, there is still data available => connected() would return true. You want to send something, check that connected() is true, and write to the client => fails. So Issues would get opened again and again: writing to client fails despite connected() true.
As I said, the current behavior is on purpose and the result of extensive work and investigation. It has even undergone stress testing.

[JAndrassy]

there is always status() to test the true state

and it doesn't match the reference
https://arduino-esp8266.readthedocs.io/en/latest/esp8266wifi/client-class.html

[d-a-v]

A bit of history:
#4626 (what you want to remove)
#5257
#5355

Here is the documentation:
https://arduino-esp8266.readthedocs.io/en/latest/esp8266wifi/client-examples.html?#general-client-loop

esp8266 must have been more heavily using networking after Arduino made up this API (what were the network chips at that time, beside the Arduino's official Ethernet shield and library?)
Their ::connected() and ::available() methods, as they are, sometimes lead to inconsistencies (especially the border cases, as described in above-mentioned issues and PRs, also those which reference them).

What @devyte says is right:

• connected() == true means one can send data
  and data may be received in the future
• available() > 0 means there are data already received, buffered, available for read
  • even if peer has already closed the connection
    in that case connected() is false, which also means we cannot send data anymore and no more data will be received

Also,

• Arduino: <<Note that a client is considered connected if the connection has been closed but there is still unread data.>>
  This is leading to lots of issues if it is about connected(). The true reality is that a TCP machine state can be totally disconnected but user have yet data to read. We need a consistent API letting us know in which exact state we are.
• status() is not related, it reflects the phy connectivity (WiFiClient is all about TCP)

Please elaborate if anything is still not clear or documentation must be improved.

[JAndrassy]

I meant WiFiClient.status(), not WiFi.status(). It is not required by the base class Client, but all libraries have it.

Arduino had WiFi and WiFi101 library in the time ESP8266WiFi library was created based on the Arduino WiFi library.

The ESP8266WiFi complied to the API as it is defined for the connected() function (status() == ESTABLISHED || available()). Now, from version 2.4.2, it doesn't comply. This breaks existing code and libraries in undetermined way. Sometimes everything is received, sometimes not. It is not only about connected(). This affected the bool operator too, since it is implemented as return connected().

[devyte]

@JAndrassy once again:

• the decision was a conscious one to make the current behavior what it is, it was done on purpose, after extensive discussion and overview by two of the current maintaining developers, and the final code was thoroughly tested and its impact assessed.
• we are aware of the implications about libraries. Please note that our lib is called ESP8266WiFi, and not WiFi. That is meant to indicate that it is specific to the ESP8266.
• we aim for compatibility with the reference, but we will not replicate buggy behavior from the reference. The code you cite is precisely what I meant by Frankenstein code on the user side: a series of workarounds and patches and conditions imposed on the user, because the reference lib code doesn't fully take into account the nature of the TCP state machine.

[JAndrassy]

the secure Clients of the ESP8266WiFi library return true from connected() if the underlying TCP connection was closed but data are available. and the reason is same as it always was for connected()

the ESP8266WiFi has from the beginning some different function results as the Arduino Ethernet or WiFi library, but this one WiFiClient.connected() was the same until 2.4.2.

[JAndrassy]

there are 3 reasons to revert it back

• Arduino API compatibility
• ESP8266WiFi backward compatibility
• 'symmetry' with ESP8266WiFi secure Clients

for TCP state WiFiClient.status() should be used

[d-a-v]

If it is reverted, there is no way to know when peer has closed the connection.
edit it is wrong to have connected()==true when tcp stack knows peer has closed the connection.

I think the operator bool() however can be changed to be connected() || (available() > 0).
edit that is to preserve as much as we can arduino API compatibility

I don't understand what you mean about WiFiClient.status(), can you elaborate ?

And it is true WiFiClientSecureBearSSL does not follow WiFiClient.
A fix is needed fixed in #7680

[JAndrassy]

I don't understand what you mean about WiFiClient.status(), can you elaborate ?

Arduino/libraries/ESP8266WiFi/src/WiFiClient.cpp

Line 340 in 692e542

uint8_t WiFiClient::status()

If it is reverted, there is no way to know when peer has closed the connection.

if (client.status() == CLOSED)