search

hashicorp / go-retryablehttp

Retryable HTTP client in Go
Mozilla Public License 2.0
1.99k stars 251 forks source link

Retry response code 307 - temporary redirect #123

Closed andrejvanderzee closed 3 years ago

andrejvanderzee commented 3 years ago

Response code 307 should also be retried as it requires another round-trip to the server.

https://github.com/hashicorp/vault/pull/10898

andrejvanderzee commented 3 years ago

@jefferai Any chance to get this approved? 307 should be "retried" on the URL in the Location header returned by the server, instead of abandoned. We are experiencing severe production problems with the vault agent side container in Kubernetes because of this, and probably others too :-(

jefferai commented 3 years ago

I don't think we want to go to a retry policy here. 307 isn't an error, it's a redirect. Putting it in the retry policy makes it subject to backoff, which we definitely don't want.

Fundamentally a 307 isn't a retry. I think we should mirror the official Go http lib here. IIRC they automatically follow a 307 unless a redirect policy is given.

In Vault's case it actually already specifies a redirect policy. I think likely what should happen is that on a 307 -- or potentially any 3XX -- this lib simply returns just as it would with a 2XX and lets any specified redirect policy take over.

I don't want to go into many Vault specifics here -- we can take it to the other ticket -- but generally seeing 307s at all is actually evidence of a misconfiguration. Vault standby nodes transparently forward requests to the active node over the cluster connection. The only reason for them to 307 is if that cluster connection is not able to be established or maintained. Usually this means the cluster port is not accessible, or the configuration file has some incorrect values.

andrejvanderzee commented 3 years ago

Yeah I agree its not a general HTTP retry issue, and we are having a different issue with vault agent.

Closing this issue.