.Net: System.InvalidOperationException: Collection was modified; enumeration operation may not execute from Microsoft.SemanticKernel.Connectors.OpenAI

[gleborgne]

Describe the bug
I try upgrading from 1.7.1 but any version above that I tried are throwing exceptions when I am using GetChatMessageContentsAsync. If using GetStreamingChatMessageContentsAsync with the exact same parameters everything is working fine.

I am using Azure Open AI with a few kernel plugins and ToolCallBehavior = ToolCallBehavior.AutoInvokeKernelFunctions,.

System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
at System.Collections.Generic.Dictionary`2.Enumerator.MoveNext()
at Microsoft.SemanticKernel.Connectors.OpenAI.ClientCore.GetChatMessage(ChatChoice chatChoice, ChatCompletions responseData)
at Microsoft.SemanticKernel.Connectors.OpenAI.ClientCore.GetChatMessageContentsAsync(ChatHistory chat, PromptExecutionSettings executionSettings, Kernel kernel, CancellationToken cancellationToken)

From what I can see from the logs, the call to Azure Open AI was done and the problem is when linking to the kernel functions.

To Reproduce
Steps to reproduce the behavior:

1. Go to '...'
2. Click on '....'
3. Scroll down to '....'
4. See error

Expected behavior
no error ?

Platform

• OS: Windows
• IDE: Visual Studio
• Language: C#
• Source: NuGet SemanticKernel 1.8.0 and up

[stephentoub]

Can you share a simple repro?

[gleborgne]

TryReproSK.zip

I have the bug with this simple project

[stephentoub]

Thanks!

@SergeyMenshykh, you added this code:
https://github.com/microsoft/semantic-kernel/blame/f66a30bbd9bee1715062b07cfad537fc10f54aee/dotnet/src/Connectors/Connectors.OpenAI/AzureSdk/ClientCore.cs#L1224-L1227

                        foreach (var argument in arguments)
                        {
                            arguments[argument.Key] = argument.Value?.ToString();
                        }

That's generally invalid: you can't change the dictionary while you're enumerating it.

[stephentoub]

That said, @gleborgne, I notice you're running on .NET Core 3.1. That is out of support. I also expect if you update to .NET 6, .NET 7, or .NET 8 this issue will go away for you, which is also likely why testing didn't flag it. It's ok on newer versions of .NET to mutate the dictionary in this way as long as it doesn't increase its size, which it shouldn't in the cited case because the key should always be there. (But, @SergeyMenshykh, as long as we need to support .NET Framework, this is still an issue there.)

[gleborgne]

Well, I will be glad upgrading but on a legacy project with a few hundred thousands lines of code it's not that simple

[stephentoub]

Well, I will be glad upgrading but on a legacy project with a few hundred thousands lines of code it's not that simple

Unrelated to the point of this issue, but I'm curious, what issues are you hitting when you upgrade from .NET Core 3.1 to .NET 6 that make it not simple?

[gleborgne]

We have custom code on top of entity framework for handling dynamic queries on json data in Azure SQL. And upgrading to .net 8 means rewriting a lot of that code because EF has a lot of breaking changes.

[roji]

@gleborgne Shay here from the EF team - am interested in hearing about the breaking changes which you refer to above, could you provide a bit more detail on your experience?

[gleborgne]

We are using a lot of low level APIs and a few hacks to inject what we need in the generated SQL queries. Those low level APIs have changed since EF Core 2.x. As far as I remember, those parts of EF where re-written by the EF Team for handling CosmosDb, and handling json natively in EF Core.
By the way, we have a branch with an ongoing migration to .Net8 and latest EF version and the migration of EF is almost done. We have a few other code related to third parties and a lot of QA before releasing this version.

[roji]

Just to be sure, are you saying you used APIs flagged as "internal"? If not, can you give an example of API(s) you were using which broke?

[MetaITDev]

Hi!
Do you have any estimate when this fix will be published?
I really excited of SemanticKernel library. I created a prototype how to integrate it to our application using .NET 8.
Everything worked fine! But our application still use .NET 4.7.2 and when I copied my logic to test real integration, nothing works because of this issue.

[AndersMad]

Reading above - I need to voice that I do hope too you keep supporting older .NET versions as some of us are stuck there "forever" due to WebForms (EF6 to Core seems manageable but scary).

I just converted all API to this - but found this issue too late. All else is working fine (Dalle-3 and function calls via streaming chat, whisper, embedding, llama3) spite the state / all the pragma warning disable needed. Sometimes it was a bit of a headache with e.g. some ctors being internal.

NOTE: You can use the streaming option as a work around until this is fixed.

[stephentoub]

Do you have any estimate when this fix will be published?

@markwallace-microsoft, can you comment?

I do hope too you keep supporting older .NET versions

Yes. That hasn't changed. This just slipped through testing, as CI currently only runs tests on .NET Core. #6063 tracks adding a .NET Framework leg to CI.