Skip to content

Fix: Download with progress callback fails with 'Operation was cancel…#693

Merged
apsonawane merged 1 commit into
mainfrom
asonawane/download
May 4, 2026
Merged

Fix: Download with progress callback fails with 'Operation was cancel…#693
apsonawane merged 1 commit into
mainfrom
asonawane/download

Conversation

@apsonawane
Copy link
Copy Markdown
Contributor

Fix: Download with progress callback fails with "Operation was cancelled by user"

Problem

Calling model->Download() with a progress callback causes the native core to abort with:

[Foundry][Error] Error downloading model [qwen3.5-2b-generic-gpu]:
Operation was cancelled by user

The same call without a callback works correctly.

Root Cause

The C++ SDK declared NativeCallbackFn as returning void, while the native core DLL (C#) expects the callback to return int (0 = continue, 1 = cancel). Since the void callback never sets the return register, the C# marshalling reads an indeterminate value (garbage), interprets it as non-zero (cancel), and aborts the download.

All other SDKs correctly define the callback as returning int:

SDK Callback return type Correct?
C++ (before fix) void No
C# int Yes
JavaScript int32_t Yes
Python c_int Yes
Rust i32 Yes

Fix

Changed NativeCallbackFn and UserCallbackFn from void(*)() to int(*)() and updated all callback lambdas to return 0 (continue).

Files Changed

  • sdk/cpp/src/foundry_local_internal_core.hNativeCallbackFn return type voidint
  • sdk/cpp/src/flcore_native.hUserCallbackFn return type voidint
  • sdk/cpp/src/model.cpp — download progress callback lambda returns 0
  • sdk/cpp/src/core_helpers.h — streaming callback lambda returns 0
  • sdk/cpp/test/model_variant_test.cpp — added regression test Download_WithCallback_ReturnsZeroToContinue

Testing

All 163 unit tests pass, including a new regression test that invokes the callback and asserts the return value is 0.

Copilot AI review requested due to automatic review settings May 4, 2026 17:55
@vercel
Copy link
Copy Markdown

vercel Bot commented May 4, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
foundry-local Ready Ready Preview, Comment May 4, 2026 5:55pm

Request Review

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR fixes the C++ SDK/native-core callback ABI mismatch that caused downloads with a progress callback to be interpreted as cancelled.

Changes:

  • Updated native callback typedefs from void to int to match the core DLL contract.
  • Adjusted download and streaming callback lambdas to return 0 so the core continues instead of cancelling.
  • Added a regression test for the download callback path.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
sdk/cpp/test/model_variant_test.cpp Adds a regression test for the download callback return value.
sdk/cpp/src/model.cpp Updates the download progress callback lambda to return int.
sdk/cpp/src/foundry_local_internal_core.h Changes the internal native callback typedef to int.
sdk/cpp/src/flcore_native.h Changes the exported callback typedef/comment to the int contract.
sdk/cpp/src/core_helpers.h Updates the streaming callback lambda to return int.
Comments suppressed due to low confidence (1)

sdk/cpp/src/core_helpers.h:1

  • Once onChunk throws, this callback stores the exception but still returns 0 (continue) both immediately after the failure and on every subsequent invocation. With the new callback contract, that means the native core will keep streaming data even though the caller has already failed, wasting work and potentially delaying error propagation until the full stream finishes. Return the cancel value after capturing an exception, and when st->exception is already set.
// Copyright (c) Microsoft Corporation. All rights reserved.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread sdk/cpp/test/model_variant_test.cpp
@apsonawane apsonawane enabled auto-merge (squash) May 4, 2026 18:47
@apsonawane apsonawane merged commit 598fc19 into main May 4, 2026
52 checks passed
@apsonawane apsonawane deleted the asonawane/download branch May 4, 2026 19:30
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.

3 participants