Repository navigation
fix: remove channel filter in e-rcv - #911
Conversation
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline finding, I also checked whether dropping the Canary-only filter (while keeping num=1000) could cause silent truncation now that all channels are merged into the response — a single Chromium milestone has nowhere near 1000 release entries across Stable/Beta/Dev/Extended/Canary combined, so the existing cap isn't a practical risk here.
Extended reasoning...
Single-line change in src/e-rcv.ts removing the channel=Canary query parameter from a chromiumdash.appspot.com fetch_releases call, so the request now returns releases across all channels for the milestone instead of just Canary; no auth/crypto/permissions surface is touched. A CONFIRMED pre-existing finding (missing resp.ok check before resp.json(), unlike the sibling fetchChromiumDashCommit function) is already posted inline, which is reason enough to want human eyes rather than auto-approve, and I additionally checked the num=1000 cap for truncation risk and ruled it out given realistic per-milestone release counts.
| const releases = (await fetch( | ||
| `https://chromiumdash.appspot.com/fetch_releases?channel=Canary&platform=Linux,Mac,Win32,Windows&milestone=${milestone}&num=1000`, | ||
| `https://chromiumdash.appspot.com/fetch_releases?platform=Linux,Mac,Win32,Windows&milestone=${milestone}&num=1000`, | ||
| ).then((resp) => resp.json())) as Array<{ version: string }>; |
There was a problem hiding this comment.
🟣 pre-existing, not blocking: Pre-existing, but now exercised with a much bigger, less-targeted query: the fetch_releases call at src/e-rcv.ts:172-174 never checks resp.ok before resp.json(), unlike fetchChromiumDashCommit at line 110 which does and calls fatal() on failure. Removing channel=Canary makes this call pull every channel for the milestone instead of one, increasing payload size/latency against the same unguarded call. If chromiumdash returns a non-200 or non-JSON body, resp.json() throws and the CLI crashes with an unhandled rejection instead of the clean fatal() message every other chromiumdash call produces. Fix: check resp.ok (and handle JSON parse failure) before resp.json() here, consistent with fetchChromiumDashCommit.
A small fix can ride a push you are already making; otherwise a short reply is enough.
Why this was flagged
Trigger: fetch_releases at src/e-rcv.ts:172-174 is called once per milestone in e rcv; after this diff it omits channel=Canary, so it now requests Linux/Mac/Win32/Windows releases across all channels (Stable/Beta/Dev/Extended/Canary) instead of just Canary, enlarging the response per call. The code does .then((resp) => resp.json()) with no resp.ok check, unlike fetchChromiumDashCommit (line 110) which checks resp.ok and calls fatal(). If chromiumdash errors (5xx, rate-limit, timeout) on this heavier query, resp.json() throws on a non-JSON body, producing an unhandled promise rejection that crashes the process instead of the fatal() UX used elsewhere in the same file for the same endpoint family. Nothing in the diff adds this check even though it touches exactly this call.
Verification: pre-existing. The fetch at src/e-rcv.ts:172-174 does .then((resp) => resp.json()) with no resp.ok check, unlike fetchChromiumDashCommit which guards at lines 110-112. On a chromiumdash error, resp.json() rejects on non-JSON, or returns a non-array so releases.map at line 175 throws. The missing-guard route is byte-identical on base and HEAD; removing channel=Canary only widens a response still capped by num=1000, so merging makes nothing worse here.
Bump the pinned @electron/build-tools SHA from 32e2253f to 3bf7c9f5 to pick up electron/build-tools#912, which raises siso's remote timeout for Windows clang-cl compiles to 10m (matching macOS) so heavy compiles finish remotely and get cached instead of falling back to local on every warm build. Also picks up electron/build-tools#911 (e-rcv only). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GkpQ9nmAGxxVpwT8wPzfbC
…4958) Bump the pinned @electron/build-tools SHA from 32e2253f to 3bf7c9f5 to pick up electron/build-tools#912, which raises siso's remote timeout for Windows clang-cl compiles to 10m (matching macOS) so heavy compiles finish remotely and get cached instead of falling back to local on every warm build. Also picks up electron/build-tools#911 (e-rcv only). Claude-Session: https://claude.ai/code/session_01GkpQ9nmAGxxVpwT8wPzfbC Co-authored-by: Claude <noreply@anthropic.com>
…5026) Bump the pinned @electron/build-tools SHA from 5a3a3bf9 to 3bf7c9f5 to pick up electron/build-tools#912, which raises siso's remote timeout for Windows clang-cl compiles to 10m (matching macOS) so heavy compiles finish remotely and get cached instead of falling back to local on every warm build. Also picks up electron/build-tools#911 (e-rcv only). Manual backport of #54958. Claude-Session: https://claude.ai/code/session_01GkpQ9nmAGxxVpwT8wPzfbC Co-authored-by: Claude <noreply@anthropic.com>
…5025) Bump the pinned @electron/build-tools SHA from 5a3a3bf9 to 3bf7c9f5 to pick up electron/build-tools#912, which raises siso's remote timeout for Windows clang-cl compiles to 10m (matching macOS) so heavy compiles finish remotely and get cached instead of falling back to local on every warm build. Also picks up electron/build-tools#911 (e-rcv only). Manual backport of #54958. Claude-Session: https://claude.ai/code/session_01GkpQ9nmAGxxVpwT8wPzfbC Co-authored-by: Claude <noreply@anthropic.com>
…5027) Bump the pinned @electron/build-tools SHA from 5a3a3bf9 to 3bf7c9f5 to pick up electron/build-tools#912, which raises siso's remote timeout for Windows clang-cl compiles to 10m (matching macOS) so heavy compiles finish remotely and get cached instead of falling back to local on every warm build. Also picks up electron/build-tools#911 (e-rcv only). Manual backport of #54958. Claude-Session: https://claude.ai/code/session_01GkpQ9nmAGxxVpwT8wPzfbC Co-authored-by: Claude <noreply@anthropic.com>
Doesn't add value (there's always way under 1,000 release versions returned). and can prevent using
e rcvwith certain Chromium values