Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--retry-all-errors doesn't retry 400 status code #6712

Closed
lawrencegripper opened this issue Mar 9, 2021 · 4 comments
Closed

--retry-all-errors doesn't retry 400 status code #6712

lawrencegripper opened this issue Mar 9, 2021 · 4 comments

Comments

@lawrencegripper
Copy link

@lawrencegripper lawrencegripper commented Mar 9, 2021

First up, thanks for curl it's awesome!

I did this

curl "https://ense241pboy9vso.m.pipedream.net" --retry 5 --retry-all-errors -X POST -v

The endpoint returns a 400 status code with a body of hello world.

curl doesn't retry the request and returns the body.

After reading through the code I discovered that by adding --fail to the command I could get the behaviour that I wanted. curl "https://ense241pboy9vso.m.pipedream.net" --retry 5 --retry-all-errors -X POST --fail will retry the 400 response.

I expected the following

I expected the 400 response to be retried.

Given the --retry-all-errors docs say

Retry on any error. This option is used together with --retry.

I think either a note in the docs for retry-all-errors to highlight that --fail is needed to retry http error codes or tweaks to the source so any http error code is retried when retry-all-errors is set.

I think this is because --fail is causing this if to hit and the 400 to be counted as an error. Reading the code again I think this was way off.

New theory, I think to retry 400 response codes with retry-all-errors the following lines need tweaking (never played with C so apologies if I'm way off again).

curl/src/tool_operate.c

Lines 373 to 384 in 40f3c18

else if(config->failwithbody) {
/* if HTTP response >= 400, return error */
long code = 0;
curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &code);
if(code >= 400) {
if(global->showerror)
fprintf(global->errors,
"curl: (%d) The requested URL returned error: %ld\n",
CURLE_HTTP_RETURNED_ERROR, code);
result = CURLE_HTTP_RETURNED_ERROR;
}
}

Maybe to be else if(config->failwithbody || config->retry_all_errors ) {

I've never played in C but happy to have a go at this tweak and adding a test case for it.

curl/libcurl version

root@bd78ab9fafbc:/# curl -V       
curl 7.74.0 (x86_64-pc-linux-gnu) libcurl/7.74.0 OpenSSL/1.1.1i zlib/1.2.11 brotli/1.0.9 libidn2/2.3.0 libpsl/0.21.0 (+libidn2/2.3.0) libssh2/1.9.0 nghttp2/1.43.0 librtmp/2.3
Release-Date: 2020-12-09
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps mqtt pop3 pop3s rtmp rtsp scp sftp smb smbs smtp smtps telnet tftp 
Features: alt-svc AsynchDNS brotli GSS-API HTTP2 HTTPS-proxy IDN IPv6 Kerberos Largefile libz NTLM NTLM_WB PSL SPNEGO SSL TLS-SRP UnixSockets

operating system

Tested in debian sid docker container.

root@bd78ab9fafbc:/# uname -a
Linux bd78ab9fafbc 5.8.0-7642-generic #47~1614007149~20.04~82fb226-Ubuntu SMP Tue Feb 23 02:56:27 UTC  x86_64 GNU/Linux
@jay
Copy link
Member

@jay jay commented Mar 10, 2021

HTTP status code errors (4xx,5xx) are not transfer errors so curl won't error unless you use --fail. That may need more presence in the documentation but I don't know where.

4.4 Why do I get downloaded data even though the web page doesn't exist?
4.20 curl doesn't return error for HTTP non-200 responses!

@lawrencegripper
Copy link
Author

@lawrencegripper lawrencegripper commented Mar 10, 2021

Makes sense to me, could be good to call this out in the man page under --retry-all-errors. The wording around any error in the existing doc is what has threw me.

I've put some possible changes in bold.

Retry on any transfer error. This option is used together with --retry.

This option is the "sledgehammer" of retrying. Do not use this option by default (eg in curlrc), there may be unintended consequences such as sending or receiving duplicate data. Do not use with redirected input or output. You'd be much better off handling your unique problems in shell script. Note that HTTP Status code errors are not retried by default add --fail to ensure these are retried as well. Please read the example below.

jay added a commit to jay/curl that referenced this issue Mar 11, 2021
- Add a paragraph explaining that curl does not consider HTTP response
  errors as curl errors, and how that behavior can be modified by using
  --retry and --fail.

The --retry-all-errors doc says "Retry on any error" which some users
may find misleading without the added explanation.

Ref: https://curl.se/docs/faq.html#Why_do_I_get_downloaded_data_eve
Ref: https://curl.se/docs/faq.html#curl_doesn_t_return_error_for_HT

Reported-by: Lawrence Gripper

Fixes curl#6712
Closes #xxxx
@jay
Copy link
Member

@jay jay commented Mar 11, 2021

I've proposed #6720 to fix this, please take a look. I've made a distinction between --retry (which will retry some HTTP response codes that indicate transient HTTP errors) and --fail (which does almost all 4xx and 5xx).

@lawrencegripper
Copy link
Author

@lawrencegripper lawrencegripper commented Mar 11, 2021

Looks great, thanks!

@jay jay closed this in cf9d16b Mar 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants