Skip to content

fix(update): increase npm registry fetch timeout from 5s to 15s - #1724

Merged
liangshuo-1 merged 1 commit into
larksuite:mainfrom
Lekkoo:fix/update-check-timeout
Jul 2, 2026
Merged

fix(update): increase npm registry fetch timeout from 5s to 15s#1724
liangshuo-1 merged 1 commit into
larksuite:mainfrom
Lekkoo:fix/update-check-timeout

Conversation

@Lekkoo

@Lekkoo Lekkoo commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Problem

lark-cli update --check fails with context deadline exceeded when the network path to registry.npmjs.org has high TLS handshake latency. This is common behind TUN-mode proxies (e.g. Clash Verge TUN), VPNs, or transcontinental routes.

Root Cause

internal/update/update.go uses a hardcoded 5-second HTTP client timeout for the npm registry fetch:

fetchTimeout = 5 * time.Second

This timeout covers the entire request lifecycle (DNS + TCP + TLS + headers + body). Under TUN-mode proxies, the TLS handshake alone takes 4-6 seconds because traffic is intercepted at the virtual interface and re-routed through a proxy tunnel:

# Measured behind Clash TUN (US node, from China)
curl timing:
  DNS resolve:   ~0ms   (fake-ip 198.18.0.x)
  TCP connect:   ~0ms   (local TUN interface)
  TLS handshake: 4.3-5.9s  ← bottleneck
  Total:         4.7-6.3s

curl succeeds because it has no default connect timeout. Go's http.Client.Timeout = 5s is too tight — the TLS handshake hasn't finished before the deadline fires.

The 5s timeout also makes the failure non-deterministic: sometimes the TLS handshake completes in 4.3s and the check succeeds, sometimes it takes 5.9s and fails. This makes the bug hard to reproduce and diagnose.

Fix

Increase fetchTimeout from 5s to 15s:

fetchTimeout = 15 * time.Second

15s provides comfortable headroom for high-latency network paths while still failing fast on genuinely unreachable networks. The npm registry endpoint returns a tiny JSON payload (<1KB), so the extra timeout does not delay real failures meaningfully.

Verification

  • go build ./internal/update/ — passes
  • go test ./internal/update/ -count=1 — passes
  • Manual test with the Go test program confirms: timeout=5s fails intermittently, timeout=10s/15s succeeds consistently

Notes

Switching Clash Verge between TUN / System Proxy / rule modes does not help — TUN is always intercepting traffic when enabled, and the TLS latency is inherent to the proxy round-trip. Setting HTTP_PROXY/HTTPS_PROXY env vars also does not help because the Go binary already uses http.DefaultTransport which honors proxy env vars; the issue is the timeout, not proxy detection.

Summary by CodeRabbit

  • Bug Fixes
    • Increased the timeout for registry fetches, helping reduce failed or interrupted npm update requests on slower connections.

The update checker fetches https://registry.npmjs.org/@larksuite/cli/latest
with a 5-second HTTP client timeout. Under high-latency network conditions
(TUN-mode proxies, VPNs, transcontinental routes), TLS handshake alone can
take 4-6 seconds, causing the check to fail with:

  context deadline exceeded (Client.Timeout exceeded while awaiting headers)

Measured example behind a Clash TUN proxy (US node from China):
  DNS resolve:    ~0ms (fake-ip)
  TCP connect:    ~0ms (local TUN)
  TLS handshake:  4.3-5.9s  <-- bottleneck
  Total:          4.7-6.3s

curl succeeds because it has no default connect timeout, but the Go HTTP
client with Timeout=5s is too tight. The registry endpoint returns a tiny
JSON payload (<1KB), so 15s is more than enough headroom while still
failing fast on genuinely unreachable networks.
@Lekkoo
Lekkoo requested a review from liangshuo-1 as a code owner July 2, 2026 13:23
@github-actions github-actions Bot added the size/M Single-domain feat or fix with limited business impact label Jul 2, 2026
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 138c42e5-6873-4985-863d-8b5377909916

📥 Commits

Reviewing files that changed from the base of the PR and between 9f15067 and 85cc555.

📒 Files selected for processing (1)
  • internal/update/update.go

📝 Walkthrough

Walkthrough

The fetchTimeout constant in internal/update/update.go was increased from 5 seconds to 15 seconds, affecting the HTTP client timeout used for npm registry requests.

Changes

Timeout Configuration Update

Layer / File(s) Summary
Increase fetch timeout
internal/update/update.go
The fetchTimeout constant value is changed from 5 to 15 seconds, increasing the timeout applied to HTTP client requests.

Estimated code review effort: 1 (Trivial) | ~2 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the PR's main change: increasing the npm registry fetch timeout from 5s to 15s.
Description check ✅ Passed The PR covers summary, motivation, fix, and verification, but it doesn't use the exact Changes/Test Plan/Related Issues template sections.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@85cc5557a93e584ec9a1652112e81d019ae5645f

🧩 Skill update

npx skills add Lekkoo/cli#fix/update-check-timeout -y -g

@liangshuo-1
liangshuo-1 merged commit efa3439 into larksuite:main Jul 2, 2026
31 checks passed
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


王伟达 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants