Skip to content

fix(api): retry expired cleanup and enforce URL timeout during DNS - #32

Merged
AndreyBegma merged 3 commits into
AndreyBegma:developfrom
DiadkoShmek:codex/filesdrop-cleanup-dns-retries
Oct 21, 2026
Merged

AndreyBegma merged 3 commits into
AndreyBegma:developfrom
DiadkoShmek:codex/filesdrop-cleanup-dns-retries

Conversation

@DiadkoShmek

Copy link
Copy Markdown
Contributor

Summary

Two API reliability fixes discussed with Andriy on Discord, based on develop
494ba189629cde828f3b3d5dab278589f3f39246.

Keep failed expiration cleanup retryable

Previously, cleanup deleted database rows before unlinking files. A transient
unlink failure therefore left bytes on disk without an expired row for the
next cron pass to retry.

Wait for all unlink outcomes, delete rows only for successful removals, and
retain failed removals for a later pass. Partial success is committed before
the failure is propagated: a single persistent failure must not retain the
entire oldest 500-row batch.

Regression coverage uses the real temporary-file storage with the in-memory
repository: transient unlink failure, database deletion failure after unlink,
and 501 expired files with one persistent unlink failure. The latter confirms
that a subsequent pass reaches the next batch while retaining the failed row.

Apply the URL-upload deadline while DNS is pending

UrlFetcher.open() previously awaited DNS without observing the caller's
AbortSignal, so the existing 30-second timer could fire while the request
continued waiting for DNS.

Pass the signal through resolution and stop waiting when it aborts. Preserve
the abort error for the existing upload handler's 408 mapping. Address checks,
connection pinning and redirect validation remain in place. Both late DNS
fulfilment and rejection have handlers; an already-aborted operation starts
neither DNS nor HTTP.

Local verification

  • Node 24.14.0, project-pinned Bun 1.4.0 and frozen lockfile; lifecycle
    scripts disabled during the scoped dependency installation.
  • Shared package compiled and Prisma client generated from the existing schema
    using a local placeholder URL; no database connection was needed.
  • Native API Jest suite: 12 suites, 236 tests passed (--runInBand).
  • API TypeScript check: tsc -p apps/api/tsconfig.json --noEmit passed.
  • Biome check on the four changed files and git diff --check passed.
  • Six new regression cases failed against the unchanged develop source with
    the same dependencies. A further 501-file counterexample caught batch
    starvation in the initial candidate and passes with this revision.

Scope and limits

  • No live service or production data was tested. DNS and outbound HTTP in the
    new tests are injected doubles; cleanup uses disposable local files.
  • DNS cancellation bounds the waiting request; it does not cancel the native
    OS DNS lookup itself.
  • A batch in which every unlink fails still terminates with an error; this PR
    does not add pagination around entirely failing batches.
  • Full monorepo web/bot tests, full build and remote CI have not been run
    locally. The existing ts-jest warning about shared compiled JS is unchanged.

Implementation and regression tests were prepared with Codex assistance.

DiadkoShmek and others added 3 commits October 20, 2026 16:36
One persistently failing unlink no longer stops the pass: the failure is
logged, the row stays for the next pass, later batches still advance, and
the removed count reflects what was actually deleted. The loop stops once
a batch consists only of failures.
@AndreyBegma
AndreyBegma merged commit 10c96ac into AndreyBegma:develop Oct 21, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants