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

Only hedge idempotent requests #61

Open
MichaHoffmann opened this issue Aug 28, 2024 · 2 comments
Open

Only hedge idempotent requests #61

MichaHoffmann opened this issue Aug 28, 2024 · 2 comments
Labels
question Further information is requested

Comments

@MichaHoffmann
Copy link

I might be wrong, but I think right now we hedge all requests, regardless if idempotent or not which might result in a bug. Shouldnt we check that we only hedge idempotent requests to be safe?

@GiedriusS
Copy link

Maybe https://github.com/cristalhq/hedgedhttp/blob/main/hedged.go#L40-L41 *http.Request could be exposed to NextFn and then Next could return 0 when req.Method == "POST" (etc.)?

@cristaloleg cristaloleg added the question Further information is requested label Aug 29, 2024
@cristaloleg
Copy link
Member

Hey @MichaHoffmann you are right, cristalhq/hedgedhttp doesn't separate idempotent and non-idempotent requests. And will not 'cause idempotency can be achieved in a different way (header, cookie, url param, etc). Better to leave this responsibility to the user.

Regarding how this can be achived, @GiedriusS is absolutely correct. NextFn 1st return param says how many hedged request should be done, zero is exactly what is should be used for non-idempotent requests.

I will add GUIDE.md file and will document that (I'm shocked that there is no GUIDE.md, almost all other cristalhq libs have it).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants