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.

Sign up for GitHub

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 only idempotent requests #76

Closed
jsha opened this issue Jun 21, 2020 · 2 comments · Fixed by #80
Closed

Retry only idempotent requests #76

jsha opened this issue Jun 21, 2020 · 2 comments · Fixed by #80

Comments

@jsha
Copy link
Collaborator

jsha commented Jun 21, 2020

At https://github.com/algesten/ureq/blob/master/src/unit.rs#L167, requests that fail on bad_status_read are retried, but only if no body bytes were sent. The HTTP RFC says:

A user agent MUST NOT automatically retry a request with a non-
idempotent method unless it has some means to know that the request
semantics are actually idempotent, regardless of the method, or some
means to detect that the original request was never applied.

It's possible to have a POST request with an empty body; that would be non-idempotent, but would also have zero body bytes sent.

Relatedly, the comments in unit.rs discuss "body bytes sent" which suggests this code could be run if a request with a body was made, but the error happened before any bytes of the body were sent. However, body_bytes_sent is only set if the whole body is successfully sent. I think it would be clearer to run the retry only if body's size is set and is zero.
@algesten
Copy link
Owner

Interesting! Didn't realise the spec allowed for other mechanisms than just looking at the method.

I guess the simplest would be to just lock down any retries depending on method?

GET | HEAD | OPTIONS | TRACE | PUT | DELETE?

@jsha
Copy link
Collaborator Author

jsha commented Jun 21, 2020

Yep, exactly. I have a branch in progress doing that that I'm hoping to upload today.

jsha added a commit to jsha/ureq that referenced this issue Jun 22, 2020
@jsha
Only retry idempotent requests.
1f36375 
This also reverts a change to send_body that was originally added to
return the number of bytes written. It's no longer needed now that we
check the size of the reader in advance.

Fixes algesten#76.
@jsha jsha mentioned this issue Jun 22, 2020
Merged
@jsha jsha closed this as completed in #80 Jun 22, 2020
jsha added a commit that referenced this issue Jun 22, 2020
@jsha
Only retry idempotent requests. (#80)
00461fb 
This also reverts a change to send_body that was originally added to
return the number of bytes written. It's no longer needed now that we
check the size of the reader in advance.

Fixes #76.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Only retry idempotent requests. jsha/ureq
2 participants
@jsha @algesten