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

Jump to bottom

--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
Labels
cmdline tool documentation not-a-bug This is not a bug in curl

Comments

@lawrencegripper
Copy link

lawrencegripper commented Mar 9, 2021 •
edited

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 jay added cmdline tool not-a-bug This is not a bug in curl labels Mar 10, 2021
@jay
Copy link
Member

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 commented Mar 10, 2021 •
edited

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
@jay
retry-all-errors.d: Explain curl errors versus HTTP response errors
d780b4b 
- 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 jay mentioned this issue Mar 11, 2021
Closed
@jay
Copy link
Member

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

Looks great, thanks!

@jay jay closed this as completed in cf9d16b Mar 12, 2021
@KSerrania KSerrania mentioned this issue May 10, 2022
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
cmdline tool documentation not-a-bug This is not a bug in curl
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

retry-all-errors.d: Explain curl errors versus HTTP response errors jay/curl
2 participants
@jay @lawrencegripper