Fix intermittent connection resets on scan comparison by polling the diff-scans endpoints - #283
Closed
lelia wants to merge 1 commit into
Closed
Fix intermittent connection resets on scan comparison by polling the diff-scans endpoints#283lelia wants to merge 1 commit into
lelia wants to merge 1 commit into
Conversation
The scan comparison (fullscans.stream_diff) held a single HTTP connection open, fully idle, while the API computed the diff. Network middleboxes with TCP idle timeouts - notably Azure NAT gateways, which default to 4 minutes - kill that connection with a RST, surfacing as intermittent "Connection reset by peer" / blank "API Error:" failures on the final comparison step of long scans (CE-354). The comparison now creates a diff-scan resource (POST /orgs/{org}/diff-scans/from-ids) and polls GET /orgs/{org}/diff-scans/{id}?cached=true with short bounded requests: 202 while the diff is computing, 200 with the result once ready. No request is ever idle long enough to be reaped, and the poll interval backs off 5s -> 30s to stay quota-friendly (each poll costs 1 quota unit). Transient poll failures retry; a 30-minute backstop guards against a diff scan that never completes. Any failure of the new flow (e.g. org tokens missing the diff-scans:create / diff-scans:list / full-scans:list scopes) logs a warning and falls back to the legacy streaming comparison, so the change is transparent to existing users. Requires socketdev>=3.4.0 for diffscans.get query-param/202 support. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lelia
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Diff-mode scans on self-hosted CI runners intermittently fail on the final comparison step with
Connection error after 261.68 seconds: ('Connection aborted.', ConnectionResetError(104, 'Connection reset by peer'))followed by a blankAPI Error:, even though the full scan itself succeeded and the results are on the dashboard.Root Cause
The scan comparison used
fullscans.stream_diff, which sends one request and then holds the connection open — completely idle — while the API computes the diff. When the comparison takes longer than a network middlebox's TCP idle timeout (Azure NAT gateways default to 4 minutes; the reported resets fired at ~262s on runners egressing through Azure), the middlebox reaps the "zombie" connection and sends a RST. Our API logs showrequest aborted(client-side termination) with no response ever written. Intermittency tracks diff computation time: fast diffs finish before the idle timer, slow ones (no baseline / large dependency trees) don't.Fix
Per dougbot's suggestion, the comparison now uses the diff-scans endpoints with polling instead of a held-open connection:
POST /orgs/{org}/diff-scans/from-idscreates a diff-scan resource for the two full scans (returns metadata immediately;on_duplicate=redirectmakes reruns of the same pair benign).GET /orgs/{org}/diff-scans/{id}?cached=true— the API answers 202 while the diff is computing and 200 with the artifacts once ready. Every request is short and bounded, so nothing is ever idle long enough to be reaped.Details:
APIFailure.is_transient_error()) retry within the loop — the diff keeps computing server-side regardless.stream_diffpath, so tokens missing the newly-required scopes (diff-scans:create,diff-scans:list,full-scans:list) keep working exactly as today. No flags, no behavior change otherwise — full-scan creation, head-scan management, and reachability finalize are untouched.omit_license_detailsmirrors the existing lean-diff behavior from CE-224.Dependencies / rollout:
socketdev>=3.4.0— companion SDK PR: Add cached diff-scan polling support to DiffScans.get socket-sdk-python#97 (adds query-param + 202 support todiffscans.get). The SDK must be released to PyPI first;uv.lockneeds regenerating after that (lockfile intentionally not updated here since 3.4.0 isn't published yet).Testing: new
tests/core/test_diff_scan_polling.pycovers poll-until-ready, backoff schedule, transient-error retry, non-transient propagation, timeout backstop, duplicate-redirect shortcut, and the streaming fallback. Full suite: 413 passed, 2 skipped (run against the local SDK build).Public Changelog
Diff-mode scan comparison no longer holds an idle HTTP connection open while the API computes the diff. The CLI now polls the diff-scans endpoints with short bounded requests, fixing intermittent "Connection reset by peer" failures on the final comparison step behind network gear with TCP idle timeouts (e.g. Azure NAT gateways). The change is transparent; if the API token lacks the diff-scans scopes the CLI falls back to the previous behavior.