Skip to content

fix(notification): detach delete-after-processing from request cancellation - #99

Open
grgao wants to merge 1 commit into
mainfrom
fix/delete-after-processing-detach-cancel
Open

fix(notification): detach delete-after-processing from request cancellation#99
grgao wants to merge 1 commit into
mainfrom
fix/delete-after-processing-detach-cancel

Conversation

@grgao

@grgao grgao commented Jul 21, 2026

Copy link
Copy Markdown

Problem

The Cloudflare Logpush S3 consumer (NewObjectConsumer) fails to delete objects after processing, with every object logging DeleteObject: context canceled (FT-979, ~560/hr, 4.1× baseline). Undeleted objects are re-delivered/re-scanned, causing duplicate ingestion, extra S3 GETs, and downstream 429s.

Root cause

objectHandler.handleCreate runs ServeHTTP (fetch → process → commit) and then bucket.DeleteObject(ctx, key) on the same request context. A handler can legitimately succeed after that request context is already canceled: firetiger's ingest commit coalescer commits under context.WithoutCancel and returns success even once the upstream notification client (EventBridge/SQS) has timed out and dropped the connection, canceling the request context. The already-succeeded work then hits DeleteObject on a dead context, so the cleanup delete fails and the object survives.

Fix

Run the post-processing delete on a context detached from request cancellation (context.WithoutCancel), bounded by its own timeout, mirroring how the object was already fully processed. Cleanup no longer depends on the request context still being live.

Regression test TestObjectHandlerDeleteAfterProcessingSurvivesRequestCancellation reproduces the exact context canceled signature (fails without the fix, passes with it).

@grgao grgao self-assigned this Jul 21, 2026
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.

1 participant