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

support for chainable net.Error checks #821

Open
wants to merge 1 commit into
base: 7.17
Choose a base branch
from

Conversation

czy0538
Copy link

@czy0538 czy0538 commented Feb 26, 2024

No description provided.

@Anaethelion
Copy link
Contributor

Hi @czy0538

Can you add a test that would highlight the change and what it allows ?

Thank you!

@czy0538
Copy link
Author

czy0538 commented Mar 5, 2024

Hi @czy0538

Can you add a test that would highlight the change and what it allows ?

Thank you!

@Anaethelion
I would be happy to provide an example that explains why the modification is necessary. Since I discovered this issue within our company's internal codebase, I am unable to provide the original code. However, I will create an example of the problem during my free time.

Here is an explanation of the issue: In our project, we pass our encapsulated RoundTripper, which handles errors using error chain(https://go.dev/blog/go1.13-errors). This has been widely used since go1.13. However, type assertions cannot accurately detect the type of error chain. Therefore, it need to be replaced with the errors.As method to avoid missing some warppednet.Errors.

The As function tests whether an error is a specific type.

// Similar to:
// if e, ok := err.(*QueryError); ok { … }
var e *QueryError
// Note: *QueryError is the type of the error.
if errors.As(err, &e) {
// err is a *QueryError, and e is set to the error's value
}
In the simplest case, the errors.Is function behaves like a comparison to a sentinel error, and the errors.As function behaves like a type assertion. When operating on wrapped errors, however, these functions consider all the errors in a chain.

In other words, all projects that pass a RoundTripper and utilize error chains face the risk of some timeout errors not being correctly retried. This issue is often challenging to detect, as our code has been running for over a year.

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

Successfully merging this pull request may close these issues.

None yet

2 participants