Conversation
|
Someone is attempting to deploy a commit to the canghe's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Follow-up self-review found two defects in the original commit of this PR. Both are fixed in 144be2e. 1. The session was created with 2. Cache was left enabled
3.
Session construction is extracted into an exported Root cause confirmation for issue #73 Worth recording why the dedicated session is needed at all rather than just relying on Electron's default proxy resolution. One consequence to be aware of when reviewing: the download path now always uses system-proxy resolution, so it intentionally does not follow the "use system proxy" toggle. For this issue that is the desired behavior, but it does mean this one path is no longer governed by that preference. Verification (run locally on macOS arm64)
Note the update check path ( |
|
Pushed What was missing: the system-proxy session fixes routing. It does not fix the reported symptom ("现在卡下载"). Fix: a Reverse verification (the part I care about): restoring the pre-fix file makes 2 of the 5 new cases fail, and they fail by hanging to vitest's 5s timeout rather than by tripping an assertion — the bug reproducing literally. Two of the other cases are anti-regression: a streaming download pushed past T+120s still resolves, and Harness pinned to the versions this repo declares (vitest 4.1.0 / typescript 5.7.3 / eslint 9.39.4 / @typescript-eslint 8.57.1 / electron 40.2.1) using the repo's own One judgement call for you: both phases use 60s as two separate constants. Merging them into one would be less code but couples two independent budgets. I kept them separate; easy to change if you disagree. |
Status update (2026-09-20): rebased onto the new
|
Synced with
|
Problem
Update downloads were stalling for users who route traffic through a local proxy (e.g. Clash Verge on macOS). The download function always used
session.defaultSession.fetch(), which respects whatever proxy mode has been set on the default session. When the WeSight "use system proxy" preference is off (the default), the session is indirectmode — so users behind a proxy got stuck at the download step.Root Cause
downloadUpdateinappUpdateInstaller.tscalledsession.defaultSession.fetch(url, ...). If the app-level proxy preference was not explicitly turned on, the default session ran indirectmode and the download bypassed any system proxy.Fix
Create a dedicated partition session (
persist:wesight-update-download) and set its proxy tosystemmode before each download. This:sessionis already imported in the file.Files Changed
src/main/libs/appUpdateInstaller.tsSelf-review
downloadUpdateis touched.session.fromPartitionreturns the same session object on repeated calls with the same name — safe to call before each download.setProxy({ mode: 'system' })on the partition session does not affectsession.defaultSession.npx tsc --noEmitexits 0).session.defaultSessionfor other network calls.Closes #73
Part 2 — the connect phase was still unguarded (2026-09-19)
The proxy session above fixes the routing half of #73. It does not fix the reported symptom. The issue title is "现在卡下载" — stuck downloading — and that state could still last forever.
Root cause
DOWNLOAD_INACTIVITY_TIMEOUT_MS(60s) is only armed afterresponse.bodyexists:A proxy that completes the TCP handshake and then black-holes the request never produces a response.
session.fetch()stays pending, no timer is running, and the only escape is the user clicking cancel. That is exactly "卡下载" — and it is the failure mode a misconfigured local proxy produces most often, which makes it the likeliest thing the reporter actually hit.Fix
DOWNLOAD_RESPONSE_TIMEOUT_MS(60s), armed immediately beforefetchand cleared in afinallyaround it, reusing the existingAbortControllerso the existing abort-reason handling maps it to the same timeout error message.finally(not after) is what keeps a streaming download from being killed at T+60s — once a response arrives the timer is gone and the inactivity timer takes over.finally, so no download path can leak a pending timer.Verification
Harness built with the versions this repo pins (vitest 4.1.0 / typescript 5.7.3 / eslint 9.39.4 / @typescript-eslint 8.57.1 / electron 40.2.1), using the repo's own
vitest.config.tsandeslint.config.mjs.appUpdateResponseTimeout.test.ts: 5 cases, all passing. Combined with the existing session test: 12 passed.appUpdateInstaller.tsmakes 2 of the 5 fail — and they fail by hanging to the 5s vitest timeout, which is the bug reproducing literally rather than an assertion tripping.vi.getTimerCount()must be 0 after success.eslinton both changed files: 0 problems (includessimple-import-sort).tsc --strict:appUpdateInstaller.tsclean. The test file reports 2TS2305errors for theelectronmock's test-only exports — pre-existing convention in this repo, not introduced here: this PR's ownappUpdateDownloadSession.test.tsreports 5 errors of the same kind under the identical check.Self-review
finallyimmediately afterfetchsettles, before any body reading.Sessiontype already imported by Part 1.One thing worth a maintainer's eye
Both phases now use 60s, declared as two separate constants. Collapsing them into one constant would be less code but would couple two independent budgets; I kept them separate deliberately. Say the word if you'd prefer one.