go-elasticsearch icon indicating copy to clipboard operation
go-elasticsearch copied to clipboard

support for chainable net.Error checks

Open czy0538 opened this issue 1 year ago • 2 comments

czy0538 avatar Feb 26 '24 08:02 czy0538

Hi @czy0538

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

Thank you!

Anaethelion avatar Mar 05 '24 10:03 Anaethelion

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.

czy0538 avatar Mar 05 '24 12:03 czy0538