Skip to content

libstore/filetransfer: Fix double callback on enqueueFileTransfer that is shutting down#363

Merged
cole-h merged 3 commits intomainfrom
double-callback-filetransfer
Feb 27, 2026
Merged

libstore/filetransfer: Fix double callback on enqueueFileTransfer that is shutting down#363
cole-h merged 3 commits intomainfrom
double-callback-filetransfer

Conversation

@cole-h
Copy link
Member

@cole-h cole-h commented Feb 23, 2026 •
edited by coderabbitai bot
Loading

Motivation

Context

Summary by CodeRabbit

  • Bug Fixes
    • Prevents spurious "interrupted" errors for transfers that were never queued.
    • Ensures transfer enqueue state is correctly set across all download paths to avoid false failure reports.
    • Improves internal handling so transfer completion and error reporting are more reliable.

@cole-h cole-h force-pushed the double-callback-filetransfer branch from 85f09b4 to 51f0d13 Compare February 23, 2026 15:53
@coderabbitai
Copy link

coderabbitai bot commented Feb 23, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 51f0d13 and d40dfb4.

📒 Files selected for processing (1)
  • src/libstore/filetransfer.cc
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/libstore/filetransfer.cc
📝 Walkthrough

Walkthrough

Added a public boolean field enqueued to curlFileTransfer::TransferItem, set when items are enqueued. The destructor now only raises an Interrupted error if a transfer is incomplete and enqueued is true. Removed an unused using directive for std::string_literals.

Changes

Cohort / File(s) Summary
Transfer Item State Tracking
src/libstore/filetransfer.cc		 Added bool enqueued to TransferItem; set item->enqueued = true at enqueue points; destructor now checks !done && enqueued before raising Interrupted; removed using std::string_literals directive.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A tiny flag named enqueued hops in with cheer,
Marking each transfer that’s waiting here,
The destructor listens only when it’s true,
No needless alarms — just tidy and true,
Hooray for small fixes, a calm queue debut! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the bug being fixed: a double callback issue in the filetransfer module during shutdown, which aligns with the changes that guard the Interrupted error in the destructor with the new enqueued flag.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch double-callback-filetransfer

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

github-actions bot commented Feb 23, 2026 •
edited
Loading

🚀 Deployed on https://69a1e6238f71a335e2cbf3ec--determinate-nix.netlify.app

@github-actions github-actions bot temporarily deployed to pull request February 23, 2026 16:01 Inactive
proski
proski reviewed Feb 23, 2026
View reviewed changes
src/libstore/filetransfer.cc Outdated
bool paused = false; // whether the request has been paused previously
bool active = false; // whether the handle has been added to the multi object
bool paused = false; // whether the request has been paused previously
bool enqueued = false; // whether the request has been added the incoming queue
Copy link

@proski proski Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the incoming queue -> to the incoming queue

@cole-h cole-h marked this pull request as ready for review February 27, 2026 18:34
@cole-h cole-h enabled auto-merge February 27, 2026 18:37
@github-actions github-actions bot temporarily deployed to pull request February 27, 2026 18:44 Inactive
@cole-h cole-h added this pull request to the merge queue Feb 27, 2026
Merged via the queue into main with commit fe3515c Feb 27, 2026
28 checks passed
@cole-h cole-h deleted the double-callback-filetransfer branch February 27, 2026 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@edolstra edolstra edolstra approved these changes

+1 more reviewer

@proski proski proski left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cole-h @edolstra @proski @xokdvium