search

mrtazz / restclient-cpp

C++ client for making HTTP/REST requests
http://code.mrtazz.com/restclient-cpp/
MIT License
1.57k stars 377 forks source link

Scarce error information. Failed to query. Status code -1 #153

Closed et-nik closed 4 years ago

et-nik commented 4 years ago

I think need to add more details when an error occurs. If restclient couldn't connect to the host or resolve the domain, then there is almost no information. There is only an uninformative error 'Failed to query' and -1 status code.

Curl library has good functionality about many errors, but it is not used.

https://github.com/mrtazz/restclient-cpp/blob/master/source/connection.cc#L444

Maybe add more curl errors?

switch (res) {
  case CURLE_OPERATION_TIMEDOUT:
    ret.code = res;
    ret.body = "Operation Timeout.";
    break;
  case CURLE_SSL_CERTPROBLEM:
  case CURLE_FAILED_INIT:
  case CURLE_URL_MALFORMAT:
  case CURLE_NOT_BUILT_IN:
  case CURLE_COULDNT_RESOLVE_PROXY:
  case CURLE_COULDNT_RESOLVE_HOST:
  case CURLE_COULDNT_CONNECT:
  // ...
    ret.code = res;
    ret.body = curl_easy_strerror(res);
    break;
  default:
    ret.body = "Failed to query.";
    ret.code = -1;
}

List of all curl error here https://curl.haxx.se/libcurl/c/libcurl-errors.html

If you want, I can create PR.

mrtazz commented 4 years ago

Oh this is a great idea. I think it makes sense to maintain an error buffer (https://curl.haxx.se/libcurl/c/CURLOPT_ERRORBUFFER.html) on the connection object that curl can use. And then for any error message prefer the contents of that over strerror of possible and use strerror as the fallback as described in the man page as well. Happy to review a PR for this!

On 3. Apr 2020, at 17:00, Nikita notifications@github.com wrote:

 I think need to add more details when an error occurs. If restclient couldn't connect to the host or resolve the domain, then there is almost no information. There is only an uninformative error 'Failed to query' and -1 status code.

Curl library has good functionality about many errors, but it is not used.

https://github.com/mrtazz/restclient-cpp/blob/master/source/connection.cc#L444

Maybe add more curl errors?

switch (res) { case CURLE_OPERATION_TIMEDOUT: ret.code = res; ret.body = "Operation Timeout."; break; case CURLE_SSL_CERTPROBLEM: case CURLE_FAILED_INIT: case CURLE_URL_MALFORMAT: case CURLE_NOT_BUILT_IN: case CURLE_COULDNT_RESOLVE_PROXY: case CURLE_COULDNT_RESOLVE_HOST: case CURLE_COULDNT_CONNECT: // ... ret.code = res; ret.body = curl_easy_strerror(res); break; default: ret.body = "Failed to query."; ret.code = -1; } List of all curl error here https://curl.haxx.se/libcurl/c/libcurl-errors.html

If you want, I can create PR.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

Spencatro commented 4 years ago

Yeesh, sorry for the spam. (That's what I get for being neurotic about squashing my commits I guess.) Anyways, I've opened a PR to attempt to address this! :octocat:

montdidier commented 2 years ago

I think this would be valuable earlier than the v0.6.0 milestone given the rate at which this milestone is progressing. Perhaps an interim release?