Uh oh!
There was an error while loading. Please reload this page.
Ban post-delivery retries for mid-body fetch failures - #294
Conversation
sroussey
commented
Aug 15, 2026
1. |
sroussey
commented
Aug 15, 2026
CI on The The This needs a new libs release containing #790, not a version bump to an existing one. Nothing in sec can close it: the override is correct, and writing it any other way would leave the fetch pointed at the unresolved domain input, since Two things to expect on the release that unblocks this:
I'll pick this back up when a libs release lands. Generated by Claude Code |
A sec fetch task's input is a domain key — a CIK, a date, an accession — so the request was built by overriding execute(). FetchUrlTask no longer calls execute() on a streamable dispatch, so that override is skipped and the fetch goes out with whatever unresolved input it was handed; the base class now refuses the override outright rather than fetching the wrong URL quietly. resolveFetchInput is the seam it provides for exactly this, and it runs first in executeStream, so both dispatch paths build the same request. Header merging is unchanged. Claude-Session: https://claude.ai/code/session_01RFf49R1YHc5s8JAAEhRHyj
workglow refuses to retry a fetch whose body failed after bytes reached a stream receiver: the consumer's subscription outlives the attempt, so a re-issue from byte 0 concatenates onto the partial body and the run reports success over the join. SecFetchJob runs its OWN retry loop inside execute(), which never sees the queue's policy — the ban held here only incidentally, through isRetriableError's `retryable === false` short-circuit, with no test on either side of the boundary. Measured with the ban removed upstream: 4 attempts instead of 1. The complement is pinned too. With no emitStreamEvent nothing was delivered anywhere, so the failure stays retryable and this loop absorbs it — which is what a large EDGAR download depends on. Both use a registered safeFetch rather than Bun.serve, so they run under vitest on node instead of joining the skipped block. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RFf49R1YHc5s8JAAEhRHyj
workglow refuses to retry a fetch whose body failed after bytes reached a stream receiver: the subscription outlives the attempt, so a re-issue from byte 0 concatenates a second body onto the first and the job then reports success over the join. This loop honored that only through isRetriableError, and the timeout branch deliberately bypasses isRetriableError — a per-attempt timeout aborts mid-body and surfaces as AbortSignalJobError, a shape the body classifier passes through untouched, so it never becomes the terminal BODY_TRUNCATED that stops every other post-delivery retry. Measured before the fix, on a 200 whose body trickles slower than the timeout with a receiver attached: 4 attempts, four bodies concatenated onto one subscription. That is sec's main large-download path — SecFetchTask always routes through the queue, JobQueueWorker always supplies emitStreamEvent, and since execute() drains executeStream() even a non-streamable fetch subscribes. So the loop now watches its own delivery: emitStreamEvent is wrapped for the whole job and latches on the first delta, before that emit resolves, and a latched job never retries. The complement is pinned too — a timeout that fires before the body loop reads anything delivers nothing, so it still retries with the receiver attached. What lifts the ban is that no byte reached it, not the absence of a subscription. The knobs are read at module load, so the two tests live in their own file with short values; the rest of the suite keeps the production defaults. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RFf49R1YHc5s8JAAEhRHyj
…3.44 The sectionRunner narrowing error is pre-existing on main and blocks the build independently of this branch. The workglow pin moves to the current published release; the resolveFetchInput seam this branch overrides still needs a libs release past 0.3.44.
1fe65c0 to
ba545b1Compare…SecFetchAccessionDocTask Refactored the `guessResponseType` function to simplify response type determination based on file extensions, improving readability and maintainability. Introduced a new utility function to check for materializing response types. Additionally, added unit tests for `SecFetchAccessionDocTask` to verify URL resolution and ensure correct handling of response types, particularly for stream requests. This enhances the robustness of the fetch task implementation.
…ror handling Refactored the `runCliProcess` function to utilize async/await for better readability and error handling. The function now uses the `once` method from the `events` module to handle process closure, ensuring a cleaner separation of stdout and stderr data collection. This change enhances the overall maintainability of the CLI process execution logic.
…nd add tests Introduced the `ipoProceeds` function to calculate IPO proceeds based on various inputs, including gross proceeds, price per unit, and units offered. Updated the `processForm424` function to utilize this new calculation method, ensuring accurate handling of cases where unit terms may be missing. Added comprehensive tests for `ipoProceeds` and integrated a new test for `processForm424` to validate the correct extraction of IPO proceeds from the prospectus cover. This enhancement improves the robustness of the Form 424 processing logic.
Uh oh!
There was an error while loading. Please reload this page.
Summary
Prevents
SecFetchJobfrom retrying fetch attempts after bytes have already been delivered to a stream receiver. Once a consumer's subscription receives body chunks, re-issuing the request from byte 0 would concatenate a second body onto the first, corrupting the stream. This fix applies to both mid-body socket errors and per-attempt timeouts.Changes
SecFetchJob.ts: Added delivery tracking via
deliveredToReceiverlatch that fires when any delta event (not just binary ones) is emitted to a stream receiver. The retry loop now checks this flag before retrying, preventing concatenation of partial bodies. Includes detailed comments explaining why timeout-driven aborts need special handling since they bypass the normalBODY_TRUNCATEDclassification.SecFetchJob.test.ts:
bodyFailsMidStream()andwholeBody()for testing mid-body failuresdescribe("post-delivery retry ban")test suite with two complementary tests:SecFetchJobTimeout.test.ts (new file): Isolated test suite for per-attempt timeout behavior, using environment variables to set aggressive timeouts (
80ms,1msbackoff). Tests that:SecCachedFetchTask.ts: Refactored
execute()to overrideresolveFetchInput()instead, moving URL resolution into the proper seam that all dispatch paths (includingexecuteStream()) run through. This ensures the fetch input is correctly resolved regardless of whether the task is streamed or not.Implementation Details
The delivery check is placed before other retry conditions because timeout-driven aborts have a different error shape than mid-body socket errors. Socket errors are already classified as terminal
BODY_TRUNCATEDby workglow's body classifier once bytes have gone out, but timeout aborts keep their bareError/AbortErrorshape and would otherwise pass through theisRetriableError()check. The delivery latch provides a unified ban for both failure modes.The
deliveredToReceiverflag is latched the moment any delta event is emitted (not just binary deltas), since what makes bytes unrepeatable is that something received them, not the specific event type.https://claude.ai/code/session_01RFf49R1YHc5s8JAAEhRHyj