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

Switch from net.Error.Temporary to net.Error.Timeout #270

Closed
gavv opened this issue Feb 2, 2023 · 2 comments · Fixed by #278
Closed

Switch from net.Error.Temporary to net.Error.Timeout #270

gavv opened this issue Feb 2, 2023 · 2 comments · Fixed by #278
Assignees
Labels
feature New feature or request help wanted Contributions are welcome important Important task
Milestone

Comments

@gavv
Copy link
Owner

gavv commented Feb 2, 2023

Currently httpexpect.Request has 4 retry policies:

  • DontRetry
  • RetryTemporaryNetworkErrors
  • RetryTemporaryNetworkAndServerErrors
  • RetryAllErrors

RetryTemporaryNetworkErrors and RetryTemporaryNetworkAndServerErrors use net.Error.Temporary to identify temporary network errors (see Request.shouldRetry). This method were deprecated in recent go versions:

// An Error represents a network error.
type Error interface {
	error
	Timeout() bool // Is the error a timeout?

	// Deprecated: Temporary errors are not well-defined.
	// Most "temporary" errors are timeouts, and the few exceptions are surprising.
	// Do not use this method.
	Temporary() bool
}

Hence, we should migrate from Temporary to Timeout, but without breaking compatibility.

Steps:

  • introduce two new retry policies: RetryTimeoutErrors, RetryTimeoutAndServerErrors
  • RetryTimeoutErrors should work like RetryTemporaryNetworkErrors, but it should use net.Error.Timeout
  • RetryTimeoutAndServerErrors should work like RetryTemporaryNetworkAndServerErrors, but it should use net.Error.Timeout
  • mark RetryTemporaryNetworkErrors and RetryTemporaryNetworkAndServerErrors as deprecated and suggest to use corresponding new policies instead
  • update tests to work with new policies; exclude deprecated policies from tests because these tests are huge already

We will remove deprecated policies in v3.

@gavv gavv added feature New feature or request help wanted Contributions are welcome important Important task labels Feb 2, 2023
@gavv gavv pinned this issue Feb 3, 2023
@kcaashish
Copy link
Contributor

Hello @gavv, I would like to work on this issue. I understand the steps you have mentioned, any other pointers / suggestions for me?

@gavv
Copy link
Owner Author

gavv commented Feb 4, 2023

Hi, you're welcome!

You may find some general hints in HACKING.md and ask questions in chat https://discord.gg/5SCPCuCWA9

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 important Important task
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants