elastic / go-elasticsearch

The official Go client for Elasticsearch
https://github.com/elastic/go-elasticsearch#go-elasticsearch
Apache License 2.0
5.54k stars 609 forks source link

support for chainable net.Error checks #821

Open czy0538 opened 4 months ago

Anaethelion commented 4 months ago

Hi @czy0538

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

Thank you!

czy0538 commented 4 months ago

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.