Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.Net: Bug: 1.23.0 throws an exception when streaming a chat completion with auto-invoked tool call #9313

Closed
jphorv-bdo opened this issue Oct 17, 2024 · 12 comments · Fixed by #9454
Assignees
Labels
bug Something isn't working .NET Issue or Pull requests regarding .NET code

Comments

@jphorv-bdo
Copy link

jphorv-bdo commented Oct 17, 2024

Describe the bug
An exception is thrown when streaming a response with toolcall auto invocation, though only when the prompt causes the function to be invoked.

Possibly related to PR 9273?

Stack trace:

System.ArgumentNullException: Value cannot be null. (Parameter 'bytes')
         at System.ArgumentNullException.Throw(String paramName)
         at System.Text.Encoding.GetString(Byte* bytes, Int32 byteCount)
         at System.BinaryData.ToString()
         at Microsoft.SemanticKernel.Connectors.OpenAI.ClientCore.GetStreamingChatMessageContentsAsync(String targetModel, ChatHistory chatHistory, PromptExecutionSettings executionSettings, Kernel kernel, CancellationToken cancellationToken)+MoveNext()
         at Microsoft.SemanticKernel.Connectors.OpenAI.ClientCore.GetStreamingChatMessageContentsAsync(String targetModel, ChatHistory chatHistory, PromptExecutionSettings executionSettings, Kernel kernel, CancellationToken cancellationToken)+MoveNext()
         at Microsoft.SemanticKernel.Connectors.OpenAI.ClientCore.GetStreamingChatMessageContentsAsync(String targetModel, ChatHistory chatHistory, PromptExecutionSettings executionSettings, Kernel kernel, CancellationToken cancellationToken)+System.Threading.Tasks.Sources.IValueTaskSource<System.Boolean>.GetResult()

To Reproduce
Steps to reproduce the behavior:

  1. use SK 1.23.0
  2. Configure a kernel with a plugin/function.
  3. Request a chat completion via Microsoft.SemanticKernel.Connectors.OpenAI.ClientCore.GetStreamingChatMessageContentsAsync(), with OpenAIPromptExecutionSettings having ToolCallBehavior set to ToolCallBehavior.AutoInvokeKernelFunctions, with a prompt that will trigger the function invocation.
  4. See exception thrown.
    Expected behavior
    I expect the chat completion completes successfully.

Platform

  • OS: Windows
  • IDE: Visual Studio 2022
  • Language: C#
  • Source: Microsoft.SemanticKernel 1.23.0

Additional context
Using Azure OpenAI services, model gpt-4o.

UPDATE

After some testing I found the issue exists with SK 1.22.0 also, and my conclusion on the specific cause:

Using Semantic Kernel to do streaming chat completion, with auto tool invocation, when a function is called, results in an uncaught exception thrown by a specific version of a transitive dependency, System.Memory.Data 6.0.0.

This issue is realized now because the latest versions of other common Microsoft dependencies (Azure.Identity 1.13.0 and Azure.Storage.Blobs 12.22.2) have raised one of their minimum dependency versions such that System.Memory.Data is transitively updated from 1.0.2 to 6.0.0.

Workaround: in my project add a direct reference to System.Memory.Data >= 7.00 (I'm going with 8.0.1).

@jphorv-bdo jphorv-bdo added the bug Something isn't working label Oct 17, 2024
@markwallace-microsoft markwallace-microsoft added .NET Issue or Pull requests regarding .NET code triage labels Oct 17, 2024
@github-actions github-actions bot changed the title Bug: 1.23.0 throws an exception when streaming a chat completion with auto-invoked tool call .Net: Bug: 1.23.0 throws an exception when streaming a chat completion with auto-invoked tool call Oct 17, 2024
@jphorv-bdo
Copy link
Author

More info, this has something to do with System.Memory.Data which provides the System.BinaryData type.
I updated some other packages at the same time I updated from SK 1.22.0 to 1.23.0:

  • Azure.Identity from 1.12.1 to 1.13.0
  • Azure.Storage.Blobs from 12.22.1 to 12.22.2

Both of those depend on Azure.Core, which in turn depends on System.Memory.Data. And recently, Azure.Core changed its dependency requirements:

  • Azure.Core 1.43.0 required System.Memory.Data >= 1.0.2
  • Azure.Core 1.44.0 (and up) require System.Memory.Data >= 6.0.0

So updating those packages simultaneously resulted in a much newer version of System.Memory.Data being brought into my project, and with this newer version of System.Memory.Data, the error occurs.

If I downgrade those 2 packages (Azure.Identity and Azure.Storage.Blobs to 1.21.1 and 12.22.1 respectively) and keep SK version 1.23.0, the exception does not occur.

@jphorv-bdo
Copy link
Author

I've confirmed that the exception is thrown in SK 1.22.0 also when my project updates to Azure.Identity 1.21.1 and Azure.Storage.Blobs 12.22.1.

@jphorv-bdo
Copy link
Author

Further investigation: if I don't upgrade any packages then I've got a working codebase using SK 1.22.0. It has no direct dependency on System.Memory.Data and so through transitive dependencies it installs System.Memory.Data 1.0.2. Again, this works.

If I take a direct dependency on System.Memory.Data:

  • v6.0.0: the exception occurs as described above
  • v7.0.0: no exception, SK completion works fine
  • v8.0.0: no exception, SK completion works fine
  • v8.0.1: no exception, SK completion works fine

Next I'll try updating the Azure packages and SK to 1.23, and with a direct dependency on System.Memory.Data 8.0.1 I expect things will work.

@jphorv-bdo
Copy link
Author

That workaround works as expected. So restating the bug report (I will add this to the body as well).

Using Semantic Kernel to do streaming chat completion, with auto tool invocation, when a function is called, results in an uncaught exception thrown by a specific version of transitive dependency, System.Memory.Data 6.0.0.

This issue is realized now because the latest versions of other common Microsoft dependencies (Azure.Identity 1.13.0 and Azure.Storage.Blobs 12.22.2) have raised one of their minimum dependency versions such that System.Memory.Data is transitively updated from 1.0.2 to 6.0.0.

Workaround: in my project add a direct reference to System.Memory.Data >= 7.00 (I'm going with 8.0.1).

@RogerBarreto
Copy link
Member

@jphorv-bdo Thank you very much for the valuable context and investigation, in our repository our dependencies with System.Memory.Data targets 8.0.0.

<PackageVersion Include="System.Memory.Data" Version="8.0.0" />

Will try your approach with the 6.0.0 and see if we can catch / avoid triggering this exception for our code-base.

@RogerBarreto RogerBarreto self-assigned this Oct 18, 2024
@ystvan
Copy link

ystvan commented Oct 18, 2024

This dependency combination reproduces the bug (with auto-invoked tool call)

<PackageReference Include="Azure.AI.OpenAI" Version="2.1.0-beta.1" />
<PackageReference Include="Azure.Identity" Version="1.13.0" />
<PackageReference Include="Azure.Storage.Blobs" Version="12.22.2" />
<PackageReference Include="Microsoft.SemanticKernel" Version="1.23.0" />
<PackageReference Include="Microsoft.SemanticKernel.Connectors.AzureAISearch" Version="1.23.0-preview" />

I have added

<PackageReference Include="System.Memory.Data" Version="8.0.1" />

and the exception is gone.

What a Friday! This thread saved my weekend and deployment next Monday ❤️

@jphorv-bdo
Copy link
Author

What change was made to SK to fix this? I couldn't find any commits that appear related.

@jphorv-bdo
Copy link
Author

@RogerBarreto you mentioned attempting to catch/avoid triggering the exception that occurs with System.Memory.Data v6.0.0, but @evchaki closed this and I don't see any relevant commits.

It seems like something should be done, shouldn't it?

@RogerBarreto
Copy link
Member

@jphorv-bdo Im happy the problem can be avoided with your resolution

I will reopen the issue and investigate a bug catch for this misusage, but keep in mind that this is not the related to SK when using a non-supported 6.0 underlying version of System.Memory.

@jphorv-bdo
Copy link
Author

I will reopen the issue and investigate a bug catch for this misusage, but keep in mind that this is not the related to SK when using a non-supported 6.0 underlying version of System.Memory.

@RogerBarreto I see, I didn't know it was unsupported. Never even thought about it until this bug appeared. But if it isn't supported then shouldn't one of the SK NuGet packages have a version requirement of System.Memory.Data >= 7.0 to prevent this from happening?

@RogerBarreto
Copy link
Member

Actually our requirement for Azure.Identity is >= 1.12, as the problem appears in 1.13 we can't control this, I will investigate upgrading the Azure.Identity to 1.13 and see what happens to ensure compatibility.

@RogerBarreto RogerBarreto reopened this Oct 25, 2024
github-merge-queue bot pushed a commit that referenced this issue Oct 29, 2024
…es (#9454)

### Motivation and Context

This add a small fix when using SK with `System.Memory.Data 6.0.0`.

This bug happens when updating the `Azure.Core` to
[1.44.0](https://www.nuget.org/packages/Azure.Core/1.44.0) which
requires `System.Memory.Data` of 6 or above that introduce an error when
attempting to `.ToString()` on an empty `BinaryData` object.

- Fix #9313
@github-project-automation github-project-automation bot moved this from Sprint: In Review to Sprint: Done in Semantic Kernel Oct 29, 2024
@jphorv-bdo
Copy link
Author

Thanks for the fix @RogerBarreto !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working .NET Issue or Pull requests regarding .NET code
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants