Improve remote shell loop error handling

[pnoltes]

Improve remote shell loop by recv return and then logging result and exiting loop if needed.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 63.88889% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.72%. Comparing base (270c784) to head (0b909e6).
⚠️ Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
bundles/http_admin/http_admin/src/http_admin.c	55.00%	18 Missing ⚠️
...e_service_admin_dfi/src/remote_service_admin_dfi.c	75.00%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #851      +/-   ##
==========================================
+ Coverage   91.54%   91.72%   +0.17%     
==========================================
  Files         235      235              
  Lines       28787    28928     +141     
==========================================
+ Hits        26353    26533     +180     
+ Misses       2434     2395      -39     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[PengZheng]

LGTM

Though networking bundles are not robust enough to be used in production environment, they are still instructive to help programmers learn how to integrate Celix. It's OK that we add some basic protections first before a refactoring.

[PengZheng]

Strictly speaking, we cannot assume a single TCP read returns the whole request body. This remark also applies to

celix/bundles/remote_services/remote_service_admin_dfi/src/remote_service_admin_dfi.c

Lines 529 to 532 in 0789aaa

	uint64_t datalength = request_info->content_length;
	char* data = malloc(datalength + 1);
	mg_read(conn, data, datalength);
	data[datalength] = '\0';

The correct way is illustrated by the following official example

#define RECV_BUF_SIZE 1 << 20

size_t read_data(struct mg_connection* conn,
                        uint8_t* buff,
                        size_t buff_len) {
  size_t read_len = 0;
  while (read_len < buff_len) {
    size_t sz_to_read = std::min<size_t>(RECV_BUF_SIZE, buff_len - read_len);
    int this_read = mg_read(conn, buff + read_len, sz_to_read);
    if (this_read < 0) {
      std::cerr << "[error] Failed to read data" << std::endl;
      break;
    } else {
      read_len += size_t(this_read);
      if (this_read > 0) {
        std::cout << "[debug] Received " << this_read << " more bytes" << std::endl;
      }
    }
  }

  return read_len;
}

[pnoltes]

updated the read for remote shell and applied to same pattern to rsa and http admin

[PengZheng]

Nice improvement for our networking code.
The HTTP code is good enough except for its zero return handling.
The TCP part is more difficult to deal with raw socket API.

[PengZheng]

read == 0 usually means EOF, rather than an error.

The function mg_read() receives data over an existing connection. The data is handled as binary and is stored in a buffer whose address has been provided as a parameter. The function returns the number of read bytes when successful, the value 0 when the connection has been closed by peer and a negative value when no more data could be read from the connection.

[PengZheng]

According to official documentation:

|content_length|long long| The content length of the request body. This value can be -1 if no content length was provided. The request may still have body data, but the server cannot determine the length until all data has arrived (e.g. when the client closes the connection, or the final chunk of a chunked request has been received). |

If every request body is required to have content_length, then we need to check against -1.
Do we need to check against a reasonable upper bound to avoid allocating too much memory?

[PengZheng]

This is still not robust enough. A common error breaking this is EINTR, which requires retry.
Note EINTR is not the only such status code, EAGAIN/EWOULDBLOCK to name a few.
We also need to take platform difference (macOS and Linux) into account.