fix(proxy): surface chosen ProxyURL on Request and Response

[Shinku-Chen]

Summary

• r.Request.ProxyURL was always empty after Visit() through any ProxyFunc, including the built-in RoundRobinProxySwitcher.
• Adds Response.ProxyURL mirrored from Request.ProxyURL so OnResponse / OnError can both see the proxy that was tried.
• No migration required for custom ProxyFunc implementations.

Root cause

httpBackend.Init sets http.Client.Timeout, which makes net/http.send() call forkReq() and shallow-copy the request before Transport.RoundTrip. Writing to the fork's ctx via *pr = *pr.WithContext(...) was lost when the fork was discarded — colly read ProxyURLKey off the original request and got nothing.

Approach

Use a *string holder placed in the request context — pointer writes survive both forkReq and the error path (where no *Response flows back).

• SetProxyFunc wraps the user's ProxyFunc: captures origCtx first, then writes the returned *url.URL through the holder. Capturing origCtx keeps the holder reachable even if a legacy f shadows ProxyURLKey on pr.
• syncProxyURL reads the holder twice: inside checkResponseHeadersFunc (so OnResponseHeaders sees it) and after Cache returns (error-path fallback). Response.ProxyURL is mirrored at the three Response construction sites.
• proxy/proxy.go: roundRobinSwitcher.GetProxy no longer touches the context — wrapper handles the write.

Test plan

• TestRoundRobinProxySwitcher_PropagatesProxyURL / …_ProxyURLOnError — built-in switcher, success + dial-refused.
• TestSetProxyFunc_LegacyContextStringPropagates / …OnError — custom legacy-shape ProxyFunc using WithContext+string; asserts the returned *url.URL (not the user's discarded ctx write) is what surfaces.

go test ./... -count=1

All green locally and on hosted CI (Go 1.21–1.24, Codecov, License Compliance) at 1295e39.

[StantonMatt]

I checked this locally.

Validation passed:

• go test ./proxy -run 'TestRoundRobinProxySwitcher_PropagatesProxyURL|TestRoundRobinProxySwitcher_ProxyURLOnError' -count=1
• go test ./... -count=1
• git diff --check upstream/master...HEAD

I also rechecked the visible GitHub checks: Build/Test for Go 1.21-1.24, Codecov, and License Compliance are all green.

The default RoundRobinProxySwitcher path looks good to me, including the error path before response headers.

One compatibility point I would tighten before merge: the PR body says custom ProxyFunc implementations need no migration, but the implementation now only reads ProxyURLKey as *string. A custom proxy function following the previous exported-key pattern still returns the proxy URL correctly, but no longer populates Request.ProxyURL / Response.ProxyURL:

c.SetProxyFunc(func(pr *http.Request) (*url.URL, error) {
    ctx := context.WithValue(pr.Context(), colly.ProxyURLKey, proxyURL.String())
    *pr = *pr.WithContext(ctx)
    return proxyURL, nil
})

I verified that shape with a local-only scratch test on this branch; Visit() succeeded, but Request.ProxyURL was empty in OnResponse.

So I think either syncProxyURL should accept both *string and legacy string values from ProxyURLKey, or the PR should explicitly document that custom proxy functions that set ProxyURLKey need to switch to the new pointer-holder pattern. A small test for the custom ProxyFunc case would make that expectation clear.

[StantonMatt]

Rechecked current head 1295e39.

The new SetProxyFunc wrapper plus the added custom ProxyFunc tests address the compatibility concern I raised earlier: legacy WithContext(..., ProxyURLKey, string) mutations no longer need to be read back directly, because the wrapper records the returned *url.URL through the shared holder and the tests cover both success and error paths.

Local checks:

go test ./proxy -run 'TestRoundRobinProxySwitcher_PropagatesProxyURL|TestRoundRobinProxySwitcher_ProxyURLOnError|TestSetProxyFunc_LegacyContextStringPropagates|TestSetProxyFunc_LegacyContextStringOnError' -count=1 -v
go test ./... -count=1
git diff --check origin/master...HEAD

Those passed. The visible hosted checks are also green now: Build/Test for Go 1.21-1.24, Codecov, and License Compliance.

[Shinku-Chen]

Thanks for taking the time to re-verify on 1295e39, @StantonMatt — really appreciate the careful follow-up. Glad the wrapper + new tests address the concern. Now just waiting on a maintainer pass for merge.