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

Add retrying capability #85

Closed
gavv opened this issue Dec 21, 2020 · 4 comments
Closed

Add retrying capability #85

gavv opened this issue Dec 21, 2020 · 4 comments
Labels
feature New feature or request help wanted Contributions are welcome
Milestone

Comments

@gavv
Copy link
Owner

gavv commented Dec 21, 2020

Add three new Request methods:

  • WithRetry(maxRetry int)
  • WithRetryDelay(minDelay, maxDelay time.Duration)
  • WithRetryPolicy(RetryNetworkErrors|RetryNetworkAndServerErrors|RetryAnyErrors)

WithRetry enabled retrying. If it's not called, test fails after first round trip failure. If it's called, test will try up to maxRetry times and only then fail.

WithRetryDelay controls pauses between retries. If it's not called, default pause is 1 second. If it's called, pause starts from minDelay and grows exponentially until it reaches maxDelay or we're out of retries.

WithRetryPolicy controls what to retry. If it's not called, default policy is RetryNetworkAndServerErrors, i.e. retry temporary network errors and temporary server errors (5xx). RetryNetworkErrors means retry only temporary network errors. RetryAnyErrors means retry absolutely all errors.

Previous attempt was here: #73. In particular, see #73 (comment)

We should also add documentation comments for new methods, cover them with tests (ideally both unit and e2e, but at least e2e), and add a new section to quick start in README.

@gavv gavv added feature New feature or request help wanted Contributions are welcome labels Dec 21, 2020
@ShreyanGoswami
Copy link

Hello, can I give it a try?

@bmihaylov
Copy link

Hey,
taking a look at your review of the previous attempt I saw the reference to the retryablehttp lib.
I poked around and it turns out they introduced the ability to get their client wrapped as a standard http.Client.
Combining that with httpexpect ability to set client on a per Request or Expect instance basis there is no need to change httpexpect .
Here is a minimal example which would suffice for a lot of cases in my opinion, given that their defaults seem sensible:

package main

import (
	"net/http"
	"net/http/httptest"
	"testing"

	"github.com/gavv/httpexpect/v2"
	rhttp "github.com/hashicorp/go-retryablehttp"
)

func TestRetry(t *testing.T) {
	fail := true
	handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		if fail {
			fail = false
			http.Error(w, "Try again!!", 500)
		}
	})
	server := httptest.NewServer(handler)
	defer server.Close()

	e := httpexpect.New(t, server.URL)
	e.GET("/").WithClient(rhttp.NewClient().StandardClient()).
		Expect().
		Status(http.StatusOK)
}

It's your call obviously, but talking as a user I see no reason to add code to support this "natively".
It would either add a nontrivial implementation or a dependency, each with it's own problems, plus even if retryablehttp
is used we'd either limit configuration to provide a simple api, or just wrap theirs with the same complexity without actually making it easier for an httpexpect user to configure a non-default retry policy.
What do you think?
Either way I'd be happy to contribute.

@gavv
Copy link
Owner Author

gavv commented Feb 15, 2021

@bmihaylov thanks, good insight!

Since httpexpect already hides HTTP client from you by default, and since this library is mostly a convenient wrapper (on top of net.http and a few other libs), I think it may make sense to provide out of the box retry support with a few simple retry policies. If you're happy with them, you can use it; in a more complicated use case, you can inject custom http client.

@gavv
Copy link
Owner Author

gavv commented Feb 20, 2021

Done.

Commit: d0e64a7

Example: https://github.com/gavv/httpexpect#retry-support

@gavv gavv closed this as completed Feb 20, 2021
@gavv gavv added this to the v2 milestone Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request help wanted Contributions are welcome
Projects
None yet
Development

No branches or pull requests

3 participants