feat(client): wrap err with upstream information

[mdenushev]

Currently, there is no way to get upstream information in RetryIfErr function, but upstream information is very important for logging purposes.

[Copilot]

Pull request overview

This PR adds upstream server information to errors returned by the HTTP client, making it easier to identify which upstream server caused an error during logging and debugging. The implementation wraps errors in a new ErrWithUpstream struct that includes the upstream address.

Key Changes:

• Introduced ErrWithUpstream struct to wrap errors with upstream information
• Updated error wrapping in DoDeadline and Do methods to include upstream address
• Modified tests to use errors.Is() for error checking instead of direct equality comparison

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
client.go	Added ErrWithUpstream type and wrapErrWithUpstream helper function; wrapped timeout and retry errors with upstream information
client_test.go	Updated error assertions from direct equality checks (err != ErrTimeout) to errors.Is() checks to support wrapped errors

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[erikdubbelboer]

The first argument to the RetryIfErr function is the request, isn't the information already in there?

[mdenushev]

The first argument to the RetryIfErr function is the request, isn't the information already in there?

No, only Response provides RemoteAddr func

[erikdubbelboer]

I'm not sure what to do with this. As you can see from the tests this is quite a backwards incompatible change, we usually don't do those. Can you think of another way to do this that is backwards compatible?

[mdenushev]

I see 3 possible options here:

1. Duplicate upstream information to Request, but it is already on Response and it would be more confusing outside of RetryIfErr.
2. Create yet another one RetryIfErrUpstream, but is also would make it more complicated (but a little less, than the first option)
3. Leave it as it is, since error checking errors.Is is there since Go 1.13...

[erikdubbelboer]

I agree it's probably best to make this minor backwards incompatible change.

[erikdubbelboer]

I'm curious why you only wrap the error here? When someone uses this method they can just read c.Addr themselves? I thought this was about Client.RetryIfErr where you can't know the addr?

[mdenushev]

@erikdubbelboer Hi, sorry for silence, I have been thinking about implementing more backward compatible solution and I think the most backward compatible solution is to introduce RetryIfErrWithUpstream field in client. What do you think about it?

[erikdubbelboer]

Yes I'm ok with RetryIfErrWithUpstream.

[mdenushev]

Created new PR #2176, so closing this one