Possible cause of incomplete server response and glued together responses?

I have done this for the past couple of years without problem: Log data on fixed interval, send HTTP POST requests to a server and wait for it to respond. I extract one thing out of it and move on to log data.

But lately (I've not upgraded Arduino IDE or hardware or firmware), I have been getting partial server responses like:

HTTP/1.1 500 Internal Server Error
Date: Tue, 08 Jul 2014 20:34:22 GMT

The connection times out after about 10-18 seconds, less than my software time out of 30s in a while loop.

Then after next connection is established, I get this:

Server: Apache/2.2.22 (Ubuntu)
Vary: Accept-Encoding
Content-Length: 619
Connection: close
Content-Type: text/html; charset=iso-8859-1

<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>500 Internal Server Error</title>
</head><body>
<h1>Internal Server Error</h1>
<p>The server encountered an internal error or
misconfiguration and was unable to complete
your request.</p>
<p>Please contact the server administrator,
 [no address given] and inform them of the time the error occurred,
and anything you might have done that may have
caused the error.</p>
<p>More information about this error may be available
in the server error log.</p>
<hr>
<address>Apache/2.2.22 (Ubuntu) Server at www.-------.com Port 80</address>
</body></html>
HTTP/1.1 200 OK
Date: Tue, 08 Jul 2014 20:34:35 GMT
Server: Apache/2.2.22 (Ubuntu)
Vary: Accept-Encoding
Keep-Alive: timeout=5, max=100

Apparently only the first 2 lines of the 500 error response was received during the first connection. Then minutes later, next upload gets the rest of the first response and with the second response (not complete either).

I wonder if I am to be blamed. I use:

if (!client.connected()) break;

This breaks out of a while loop that waits for server response.

What if server breaks connection when there is still unread message in the wifi shield? This will just make arduino quit reading the unread message and it gets left over for the next connection. This explanation contrasts my understand:

I am not using the "LATEST" version of the library which this reference is about though.

Any simple explanations? I'll try to compile in arduino ide 1.0.5 r2 to see what happens.

You should post your code. But just as a warning, the wifi shield has some bugs in the firmware, even the newest firmware version 1.1.0.

I'm not able to upgrade any wifi shield firmware since they are all deployed in different fields.

My code that waits for server response on a connection:

//Receive server reply and log it to Debug(serial), debugfile. Returns the reply code.
int receive_server_reply(WiFiClient* my_in)
{
	boolean catch_next_byte=false; // This indicates the next byte from buswave server should be captured as reply value. This is turned on only when cell_send_response_indicator character is detected (usually '*').
	char HTTP_response_code[]="   "; // This stores the HTTP response code such as 200 or 500, for later recognition.
	int reply=cell_send_response_network_reg_err;
	unsigned int received_byte_cnt=0; // This counts received bytes from server. It is used to determine where the HTTP respond code is located.
	unsigned long connect_time;
	// Ready the debug file 
	if (!debugfile.open(debug_file_name, O_WRITE | O_CREAT | O_AT_END)) // Send data to debugfile
	{
		Debug->print(millis());
		Debug->println(F(":Can't open debug file"));
		my_in->stop();
		while(1){}
	}

	// Wait on server response
	connect_time=millis();
	Debug->print(millis());
	Debug->println(F(":Waiting for server response"));
	while (millis()-connect_time<server_response_time) 
	{
		char c;
		if (my_in->available())
		{
			c= my_in->read();
			Debug->write(c);
			debugfile.write(c);
			if ((received_byte_cnt>8)&&(received_byte_cnt<12)) HTTP_response_code[received_byte_cnt-9]=c; // Store the HTTP response code
			if (received_byte_cnt==11) // Got the response code
			{
				if (strcmp("200",HTTP_response_code)!=0) // Cache data if response is not 200.
				{
					while (millis()-connect_time<server_response_time) // Still receive the rest of the response until there is none.
					{
						if (my_in->available())
						{
							c= my_in->read();
							Debug->write(c);
							debugfile.write(c);
						}
						//if (!my_in->connected()) break;
					}
					debugfile.println(F("Server not responding 200"));
					Debug->println(F("Server not responding 200"));
					reply=cell_send_response_network_reg_err;
					break;
				}
			}
			received_byte_cnt++;
		}
		if (catch_next_byte)
		{
			reply=c;
			catch_next_byte=false;
		}
		if (c==cell_send_response_indicator) catch_next_byte=true;
		//if (!my_in->connected()) break;
	}

	Debug->println();
	Debug->print(millis());
	Debug->println(F(":Disconnecting from server."));

	debugfile.println();
	debugfile.close();

	PGM_RAM(network_msg_02,formatting_buffer);
	sprintf(buffer,formatting_buffer,(int)reply);
	Debug->println(buffer);
	debug_log(buffer);
	
	return reply;
}

After I commented out the if (!my_in->connected()) break; I have not since received an incomplete response but it's some hard waiting time. I'll report back with arduino ide 1.0.5 r2 compile result.

WiFiClient::connected() my version:

uint8_t WiFiClient::connected() {

  if (_sock == 255) {
    return 0;
  } else {
    uint8_t s = status();

    return !(s == LISTEN || s == CLOSED || s == FIN_WAIT_1 ||
    		s == FIN_WAIT_2 || s == TIME_WAIT ||
    		s == SYN_SENT || s== SYN_RCVD ||
             (s == CLOSE_WAIT && !available()));
  }
}

Version 1.0.5 r2

uint8_t WiFiClient::connected() {

  if (_sock == 255) {
    return 0;
  } else {
    uint8_t s = status();

    return !(s == LISTEN || s == CLOSED || s == FIN_WAIT_1 ||
    		s == FIN_WAIT_2 || s == TIME_WAIT ||
    		s == SYN_SENT || s== SYN_RCVD ||
    		(s == CLOSE_WAIT));
  }
}

The available is removed. Of course the status() may be defined differently between versions.
My version:

uint8_t WiFiClient::status() {
    if (_sock == 255) {
    return CLOSED;
  } else {
    return ServerDrv::getClientState(_sock);
  }
}

1.0.5 r2

uint8_t WiFiClient::status() {
    if (_sock == 255) {
    return CLOSED;
  } else {
    return ServerDrv::getClientState(_sock);
  }
}

I don't understand the subtle difference of having available() and not having it in the connected(). Sounds like my older version is by the reference and the newest stuff is no longer checking the available() as it says it does on the ref.

The available is removed.

What do you mean by that? The available() function is still there in v1.0.5.

This is the code I use to read a server response. It is the "Perfect World" version. It has no timeout feature, but it should read the entire response if the server doesn't stall and the connection doesn't fail.

while(client.connected()) {
  while(client.available()) {
    char ch = client.read();
    Serial.write(ch);
  }
}
client.stop();

The available() that is in the return value in an earlier version of connected() is removed in the latest version of connected()

(s == CLOSE_WAIT && !available())

vs.

(s == CLOSE_WAIT );

No the function itself is still there.

You are right, your code is too simple. It would not work with my project's extended time scale (weeks, and prefer months without on-site service). I use timeout and connected() as a shortcut but connected() seems to be the problem now.

I did switch from connection-type: keep-alive to close. Does that screw me up? In the keep-alive days, I never had a problem. I am guessing the server wonders why this little brat just won't send more requests and waits online. Now with close, the server closes connection immediately and if connected() doesn't detect there is still available bytes, then I don't get these bytes.

This is my "Real World" code for reading a server response. It does have a timeout.

  // connectLoop controls the hardware fail timeout
  int connectLoop = 0;

  while(client.connected())
  {
    while(client.available())
    {
      char ch = client.read();
      Serial.write(ch);
      // set connectLoop to zero if a packet arrives
      connectLoop = 0;
    }

    connectLoop++;

    // if more than 10000 milliseconds since the last packet
    if(connectLoop > 10000)
    {
      // then close the connection from this end.
      Serial.println();
      Serial.println(F("Timeout"));
      client.stop();
    }
    // this is a delay for the connectLoop timing
    delay(1);
  }

  Serial.println();

  Serial.println(F("disconnecting."));
  // close client end
  client.stop();

How do you know it has been 1000 milliseconds? delay(1) is incredible inaccurate. I would try delayMicroseconds(1000); I just save millis() when I start waiting and check it in every loop.

Both delay and millis is not exactly accurate, but in this application, it is close enough.

You don't have to use it. I posted it because it works well for me. Unlike your code, it doesn't download just part of the response. It gets it all.

Thanks Tim. My code downloads all if I comment out the if (!client.connected()) break; and just wait until timeout. I'm trying to find a grace way to detect server disconnection. I tried to use arduino IDE 1.0.5 r2 but can't get connection. I guess I need to update my firmware, which bothers me (dozens in remote locations and firmware update is difficult). I ended up keeping my wifi library for 1.0.5 r2. I found out if I have wifi in sketchbooks/libraries and in arduino/libraries, it only uses the one in sketchbooks folder.

Tim, I think I found the problem. I upgraded firmware and used the 1.0.5 r2 wifi library.

Arduino sample code inside of loop: A while loop is used. I got every character. I don't understand. Why should I use while instead of using if?

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

  // if the server's disconnected, stop the client:
  if (!client.connected()) {
    Serial.println();
    Serial.println("disconnecting from server.");
    client.stop();

    // do nothing forevermore:
    while(true);
  }

I changed while into if

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

  // if the server's disconnected, stop the client:
  if (!client.connected()) {
    Serial.println();
    Serial.println("disconnecting from server.");
    client.stop();

    // do nothing forevermore:
    while(true);
  }

I ended up with only one character. This only means that connected() will return false if connection is broken but there are still characters in the buffer. This IS OPPOSITE of what arduino reference says. Just another classic Arduino quality issue.

I just don't trust arduino that much anymore. I might use while but there is really no excuse with a broken promise the connected will return true if there is still message left.

I just don't trust arduino that much anymore. I might use while but there is really no excuse with a broken promise the connected will return true if there is still message left.

The client.connected() function will return true if the client believes it is still connected to the server, considering the server has not stalled or the connection has broken (failed). It does not report any characters in the rx buffer. You must use client.available() for that.

I don't know what you mean by "broken promise".

edit: Maybe you were confused by this statement in the reference:

Description

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.

This is true. If you attempt to close the connection from the client end, and there are characters in the rx buffer, the connection will not close. You must empty the rx buffer before the connection will close.

Tim,

Do you think that refers to data that the client sends not receives? I thought it would be receives but you could be equally correct. Yes, that was the broken promise. You can see that clearly the earlier version of connected() has available() in the return value:

return !(s == LISTEN || s == CLOSED || s == FIN_WAIT_1 ||
    		s == FIN_WAIT_2 || s == TIME_WAIT ||
    		s == SYN_SENT || s== SYN_RCVD ||
             (s == CLOSE_WAIT && !available()));

I don't understand all the other states as there is zero explanation in the code.

But somehow it doesn't work, not with original firmware on the shield. I tried it. They must have tried to make it work, and posted the description that it considers connected if the client receive is not empty (not send).

The sample code on connected() ref page also indicates that connected() helps tell whether server has disconnected from client, not the other way around. That sample code, which I tested with arduino 1.0.5 r2 and latest firmware on the shield, just fails to run. The "if" statement makes the execution visit connected() line too soon and I only get 1-2 characters. I can only get everything when I change the "if" to "while", trapping execution at the client.read.

All I did was I used my ssid, pass, server address, nothing else. If a sample code doesn't work, I don't trust the library very much. There is no comment capability on that ref page either. They say if you have questions, post on software dev board. I did. We'll see.

Do what you want. My client code works. I don't use the library examples. They do not have the fault tolerance my code requires.

If there are characters in the client socket rx buffer, the connection will not close, no matter which end attempts to close the connection.

The socket will not send an ACK to the server for that packet until the rx buffer has been emptied. The server will not send a close message until it gets an ACK for the last packet. This is a tcp protocol feature.

The socket will not close the connection if the server stalls or the connection breaks (fails), the socket rx buffer has characters in it, and your client code calls client.stop(). You lose that socket. This is a tcp protocol feature.

Tim,

I am sorry if I bothered you too much. I am "asking too much" from arduino again. I did a simple test. If I just do

if ((!client.connected())&&(!client.available())) break;//just like their code before

Then it is safe to break loop and stop client and receive all characters. I'm not talking about tcp protocols. I am thinking reliability of arduino wifi library. I will just have to take for granted that any ref page information can be very old or inaccurate and I have to test every bit of code to make sure it works as arduino advertised it would. I wish the arduino team can just remove the description that conflicts with reality on that ref page or fix the function.

That still does not have fault-tolerance. No timeout.

Yes, but I have timeout already. This part just makes sure I'm not sitting on my thumbs until time out. Right now I have 30s time out starting from server connection. I could do a smarter time out to start and restart every time I receive a byte but shorten the total time to say 15s. But this (!(connected())&&(!available())) has successfully breaks out of the loop that will otherwise only break at time out.

But this (!(connected())&&(!available())) has successfully breaks out of the loop that will otherwise only break at time out.

You should explain this better. I don not see this in your original code. I have seen something like this used before, and it works only on a localnet with a dedicated server.

My original code is posted but without this feature so here is current code. It only receives server reply after HTTP request header and data are both sent. The feature appears twice in the code.

I'd love some critics. The server_response_time is 30,000 milliseconds.

//Receive server reply and log it to Debug(serial), debugfile. Returns the reply code.
int receive_server_reply(Client* my_in)
{
	boolean catch_next_byte=false; // This indicates the next byte from buswave server should be captured as reply value. This is turned on only when cell_send_response_indicator character is detected (usually '*').
	char HTTP_response_code[]="   "; // This stores the HTTP response code such as 200 or 500, for later recognition.
	int reply=cell_send_response_network_reg_err;
	unsigned int received_byte_cnt=0; // This counts received bytes from server. It is used to determine where the HTTP respond code is located.
	unsigned long connect_time;
	// Ready the debug file 
	if (!debugfile.open(debug_file_name, O_WRITE | O_CREAT | O_AT_END)) // Send data to debugfile
	{
		Debug->print(millis());
		Debug->println(F(":Can't open debug file"));
		my_in->stop();
		while(1){}
	}

	// Wait on server response
	connect_time=millis();
	Debug->print(millis());
	Debug->println(F(":Waiting for server response"));
	while (millis()-connect_time<server_response_time) 
	{
		char c;
		if (my_in->available())
		{
			c= my_in->read();
			Debug->write(c);
			debugfile.write(c);
			if ((received_byte_cnt>8)&&(received_byte_cnt<12)) HTTP_response_code[received_byte_cnt-9]=c; // Store the HTTP response code
			if (received_byte_cnt==11) // Got the response code
			{
				if (strcmp("200",HTTP_response_code)!=0) // Cache data if response is not 200.
				{
					while (millis()-connect_time<server_response_time) // Still receive the rest of the response until there is none.
					{
						if (my_in->available())
						{
							c= my_in->read();
							Debug->write(c);
							debugfile.write(c);
						}
						if ((!my_in->connected())&&(!my_in->available())) break; // As of Arduino IDE 1.0.5r2, the available() was removed from the connected() code so despite the ref page saying that connected() returns true when connection broke and there is still characters available, it would not return true.. You have to test available() before skip out of loop.
					}
					debugfile.println(F("Server not responding 200"));
					Debug->println(F("Server not responding 200"));
					reply=cell_send_response_network_reg_err;
					break;
				}
			}
			received_byte_cnt++;
		}
		if (catch_next_byte)
		{
			reply=c;
			catch_next_byte=false;
		}
		if (c==cell_send_response_indicator) catch_next_byte=true;
		if ((!my_in->connected())&&(!my_in->available())) break; // As of Arduino IDE 1.0.5r2, the available() was removed from the connected() code so despite the ref page saying that connected() returns true when connection broke and there is still characters available, it would not return true.. You have to test available() before skip out of loop.
	}

	Debug->println();
	Debug->print(millis());
	Debug->println(F(":Disconnecting from server."));

	debugfile.println();
	debugfile.close();

	PGM_RAM(network_msg_02,formatting_buffer);
	sprintf(buffer,formatting_buffer,(int)reply);
	Debug->println(buffer);
	debug_log(buffer);
	
	return reply;
}

Here is a sample server response. I am trying to catch the HTTP/1.1 200 to confirm success, and the response number after the *, such as *0 towards the end. This tells arduino whether it should do something, say disable a device or restart etc.

15:46:32.993> HTTP/1.1 200 OK
15:46:32.993> Date: Wed, 09 Jul 2014 20:46:09 GMT
15:46:33.055> Server: Apache/2.2.22 (Ubuntu)
15:46:33.055> Vary: Accept-Encoding
15:46:33.055> Connection: close
15:46:33.118> Transfer-Encoding: chunked
15:46:33.118> Content-Type: text/html; charset=utf-8
15:46:33.118> 
15:46:33.180> 2
15:46:33.180> *0
15:46:33.180> 0
15:46:33.180> 
15:46:33.180>

I am using Terminal by Br@y++ so I enabled time stamp :slight_smile: