Fix connect timeout not accounting for DNS resolution time by Copilot · Pull Request #810 · ithewei/libhv

Copilot · 2026-03-10T13:49:48Z

Fix ConnectTimeout in base/hsocket.c to account for DNS resolution time in the connect timeout
Fix http_client_connect in http/client/HttpClient.cpp to track elapsed time for SSL handshake timeout
Fix http_client_exec in http/client/HttpClient.cpp to add timeout check after connect call
Update build files to link htime.c with all targets using hsocket.c
Build and verify all tests pass
Address code review feedback: use CHECK_TIMEOUT macro instead of inline duplicate
Address code review feedback: add hsocket.c and htime.c to nslookup CMake target
Address reviewer feedback: explicit closesocket(connfd) before returning -ETIMEDOUT in ConnectTimeout

Original prompt

This section details on the original issue you should resolve

<issue_title>安卓使用http请求</issue_title>
<issue_description>现在发现一个问题使用http的 int http_client_send(HttpRequest* req, HttpResponse* resp); 接口时并且在完全无网络的情况下connect_timeout 设置3秒 timeout 设置 5s 但是超时返回差不多需要40秒我在安卓系统上做的测试</issue_description>

<agent_instructions>Please analyze and respond to this issue.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

Fixes 安卓使用http请求 #787

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

The ConnectTimeout function previously only applied the timeout to the TCP connect phase (poll/select), but DNS resolution via getaddrinfo() had no timeout applied. On platforms like Android with no network, getaddrinfo() can block for 30-40 seconds, making user-set connect_timeout and timeout values ineffective. Changes: - ConnectTimeout now tracks elapsed time for DNS resolution and subtracts it from the TCP connect timeout. If DNS takes longer than the timeout, it returns ETIMEDOUT immediately. - http_client_connect adjusts SSL handshake timeout by subtracting time already spent on DNS+TCP connect. - http_client_exec checks overall request timeout after connect, handling cases where connect took longer than expected. - Updated build files (CMakeLists.txt, Makefile) to link htime.c with all targets that use hsocket.c. Co-authored-by: ithewei <26049660+ithewei@users.noreply.github.com>

…ly on SSL timeout Co-authored-by: ithewei <26049660+ithewei@users.noreply.github.com>

Copilot

Pull request overview

This PR tightens timeout accounting for TCP connect + TLS handshake in the synchronous HTTP client and updates unit-test build rules to link the time utilities now required by hsocket.c.

Changes:

Track elapsed time across ConnectTimeout() and apply the remaining budget to the TLS handshake socket receive timeout.
Update ConnectTimeout() to subtract time spent in initial connect setup from the poll/select wait budget.
Fix unit-test build targets (CMake + Makefile) to link base/htime.c where base/hsocket.c is compiled directly.

Reviewed changes

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

File	Description
`unittest/CMakeLists.txt`	Adds `../base/htime.c` to several unittest executables that compile `hsocket.c` directly.
`Makefile`	Adds `base/htime.c` to additional unittest build commands that include `base/hsocket.c`.
`http/client/HttpClient.cpp`	Accounts for connect time when budgeting TLS handshake receive timeout; adds an extra overall-timeout check after connect.
`base/hsocket.c`	Makes `ConnectTimeout()` time-aware by subtracting setup time before waiting for connect completion.

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

Copilot · 2026-03-10T14:04:56Z

http/client/HttpClient.cpp

+        if (timeout_ms > 0) {
+            cur_time = gettick_ms();
+            if (cur_time - start_time >= timeout_ms) {
+                cli->Close();
+                return ERR_TASK_TIMEOUT;


The post-connect timeout check duplicates the existing CHECK_TIMEOUT logic and introduces a second timeout-handling codepath (direct return vs goto timeout). Consider reusing CHECK_TIMEOUT here to keep timeout behavior consistent and avoid future divergence (e.g., if cleanup logic changes).

Copilot · 2026-03-10T14:04:57Z

unittest/CMakeLists.txt

+add_executable(ftp ftp_test.c ../protocol/ftp.c ../base/hsocket.c ../base/htime.c)
 target_include_directories(ftp PRIVATE .. ../base ../protocol)

-add_executable(sendmail sendmail_test.c ../protocol/smtp.c ../base/hsocket.c ../util/base64.c)
+add_executable(sendmail sendmail_test.c ../protocol/smtp.c ../base/hsocket.c ../base/htime.c ../util/base64.c)
 target_include_directories(sendmail PRIVATE .. ../base ../protocol ../util)


nslookup links only nslookup_test.c and dns.c, but dns.c calls WSAInit() under OS_WIN, which is implemented in base/hsocket.c. This target will fail to link on Windows. Consider adding ../base/hsocket.c (and ../base/htime.c, since hsocket.c now depends on gettick_ms) to the nslookup executable sources to keep CMake parity with the Makefile unittest build.

ithewei · 2026-03-11T03:27:56Z

@copilot apply changes based on the comments in this thread

… target - Replace inline timeout check with CHECK_TIMEOUT macro by moving variable declarations before the connect: label to avoid crossing initializations with goto. - Add hsocket.c and htime.c to nslookup CMake target for Windows link compatibility (dns.c calls WSAInit from hsocket.c). Co-authored-by: ithewei <26049660+ithewei@users.noreply.github.com>

Copilot · 2026-03-11T03:30:33Z

@copilot apply changes based on the comments in this thread

Applied both review changes in 899eb0d:

Replaced inline timeout check after connect with CHECK_TIMEOUT macro — moved variable declarations (recvbuf, data, len, etc.) before the connect: label to avoid crossing initializations with goto timeout.
Added ../base/hsocket.c and ../base/htime.c to the nslookup CMake target for Windows link compatibility.

ithewei · 2026-03-11T03:44:48Z

base/hsocket.c

+    unsigned int elapsed = gettick_ms() - start_time;
+    int remaining = ms - (int)elapsed;
+    if (remaining <= 0) {
+        errno = ETIMEDOUT;


@copilot 这里需要closesocket(connfd)吧？

是的，已修复。改为显式调用 closesocket(connfd) 并直接返回 -ETIMEDOUT，不再依赖 socket_errno_negative()（在 Windows 上它读取 WSAGetLastError() 而非 errno，可能返回错误的错误码）。见 d19257f。

…ETIMEDOUT When DNS resolution exhausts the timeout budget, explicitly close the socket with closesocket() and return -ETIMEDOUT directly, instead of relying on socket_errno_negative() which reads errno indirectly and could return the wrong error code on Windows. Co-authored-by: ithewei <26049660+ithewei@users.noreply.github.com>

Initial plan

e3a20d1

Copilot AI assigned Copilot and ithewei Mar 10, 2026

Copilot started work on behalf of ithewei March 10, 2026 13:49 View session

Copilot AI and others added 2 commits March 10, 2026 13:58

Address code review: use consistent error handling and fail immediate…

1ac34d6

…ly on SSL timeout Co-authored-by: ithewei <26049660+ithewei@users.noreply.github.com>

ithewei marked this pull request as ready for review March 10, 2026 14:00

Copilot AI review requested due to automatic review settings March 10, 2026 14:00

Copilot started reviewing on behalf of ithewei March 10, 2026 14:00 View session

Copilot AI changed the title ~~[WIP] Fix HTTP timeout issue on Android~~ Fix connect timeout not accounting for DNS resolution time Mar 10, 2026

Copilot finished work on behalf of ithewei March 10, 2026 14:04

Copilot AI reviewed Mar 10, 2026

View reviewed changes

Copilot started work on behalf of ithewei March 11, 2026 03:28 View session

Copilot finished work on behalf of ithewei March 11, 2026 03:30

ithewei reviewed Mar 11, 2026

View reviewed changes

Copilot started work on behalf of ithewei March 11, 2026 03:44 View session

Copilot finished work on behalf of ithewei March 11, 2026 03:47

ithewei merged commit 85d1265 into master Mar 11, 2026
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix connect timeout not accounting for DNS resolution time#810

Fix connect timeout not accounting for DNS resolution time#810
ithewei merged 5 commits intomasterfrom
copilot/fix-http-timeout-issue

Copilot AI commented Mar 10, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

ithewei commented Mar 11, 2026

Uh oh!

Copilot AI commented Mar 11, 2026

Uh oh!

ithewei Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Copilot AI commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comments on the Issue (you are @copilot in this section)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

ithewei commented Mar 11, 2026

Uh oh!

Copilot AI commented Mar 11, 2026

Uh oh!

ithewei Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Mar 10, 2026 •

edited

Loading