onBeforeRequest 报错时支持走重试逻辑

[bao-io]

背景

当前 OnBeforeRequest 中间件一旦返回 error，Request.do() 会直接 return，不会走 RetryCondition / RetryHook。重试逻辑只在请求发出之后才会触发。
但有些逻辑需要在请求发出前执行（例如获取 token、预检查），失败时同样希望能按 RetryCondition 判断是否重试。

改动

• 抽出 shouldRetry、prepareRetry、tryRetry，复用请求后的重试逻辑
• udBeforeRequest 报错时调用 tryRetry，满足条件则 continue retry
• 在调用 RetryCondition 前先构造 resp（含 Request 和 Err），保证 hook 里能拿到 resp.Request，即使请求还没发到网络

行为说明

• 仅覆盖用户中间件 udBeforeRequest，内置 beforeRequest（如 parseRequestURL）报错仍直接返回
• RetryCondition 返回 false、MaxRetries 耗尽、context.Canceled 等行为与原来一致

测试

• TestRetryOnBeforeRequestError
• TestRetryConditionHasRequestOnBeforeRequestError
• TestNoRetryOnBeforeRequestErrorWhenConditionFalse

[imroc]

Reviewed as part of PR Review Loop (2026-07-03).

Thanks for the contribution! The refactoring is clean — extracting shouldRetry, prepareRetry, and tryRetry makes the retry logic nicely reusable. Tests are well-structured. A few observations and questions:

Observations

1. prepareRetry nil-safety on resp

In prepareRetry, there's a if resp != nil guard before clearing resp.body, resp.result, resp.error. This is correct since tryRetry ensures *resp is non-nil before calling prepareRetry, but the guard adds defense-in-depth. Good.

However, note that in the before-request error path, resp was just constructed as &Response{Request: r} in tryRetry — it has no body/result/error to clear. The nil-guard handles this correctly, so no issue, just calling it out.

2. Scope: only udBeforeRequest is covered, not beforeRequest

The PR description mentions this, but it's worth highlighting: built-in middleware (parseRequestHeader, parseRequestCookie, parseRequestURL, parseRequestBody) errors still bypass retry. This is a reasonable design choice (those errors are typically non-retryable, e.g. malformed URL or body), but it means the behavior is asymmetric. If a user's udBeforeRequest runs after a built-in middleware has partially modified request state, retry should still work since built-in middleware re-runs on each iteration.

3. Retry interval on before-request errors

prepareRetry calls r.retryOption.GetRetryInterval(resp, r.RetryAttempt). Since resp has no status code or body in the before-request error case, any retry interval logic that inspects the response (e.g. reading Retry-After from a response header) won't apply. This is expected behavior but worth documenting.

4. Question: contextCanceled check

In the original code, contextCanceled was checked before retry. In the refactored shouldRetry, errors.Is(err, context.Canceled) is checked at the top. However, in the before-request error path, the contextCanceled variable hasn't been set yet (it's set after roundTrip). This is fine because shouldRetry checks err directly via errors.Is, but the two code paths now check context cancellation differently. Not a bug, just a consistency note.

5. Minor: retry: label

Using a labeled continue retry is fine, but an alternative would be to let the before-request error fall through to the existing retry logic at the bottom of the loop. The current approach is more explicit though.

Test coverage

The three test cases cover the key scenarios well:

• Retry succeeds after transient failures ✅
• RetryCondition has access to resp.Request ✅
• No retry when condition returns false ✅

Consider adding a test for context.Canceled propagation when a before-request hook returns a canceled error — to confirm it doesn't retry.

Overall, this is a solid improvement. No blockers, just the minor suggestions above.