Open Camios opened 6 months ago
I tried generating a new solution for minimal reproduction, but I don't get the same problem. I copied the classes out of the problematic solution into the new solution and it doesn't have the same problem.
Just to clarify, a new solution with this function is working just fine as expected?
It seems like the migration tool struggled here, so maybe there are improvements to make to that. However without a repro, it will be difficult to investigate this. It could be an issue with the project file, and/or nuget references - are you on all the latest packages for the worker, worker sdk, and cosmos extension?
Re: samples - we have more scenarios documented here: https://github.com/Azure/azure-functions-dotnet-worker/blob/main/samples/Extensions/CosmosDB/CosmosInputBindingFunctions.cs
Hi @liliankasem
Just to clarify, a new solution with this function is working just fine as expected?
Yes
I found the upgrade assistant failed in the many ways but it was its oversight of not adding one or both of these packages to the project which is causing the runtime exception about missing file Microsoft.Bcl.AsyncInterfaces
:
Microsoft.ApplicationInsights.WorkerService
Microsoft.Azure.Functions.Worker.ApplicationInsights
Here's a longer list of things the upgrade assistant didn't do to my existing Azure Function App project (mix of problems it left behind and things it doesn't do compared to new project):
Microsoft.Azure.Functions.Worker.Extensions.CosmosDB
Microsoft.ApplicationInsights.WorkerService
Microsoft.Azure.Functions.Worker.ApplicationInsights
CosmosDB
parameter-based attribute output bindings (IAsyncCollector
) to method-based CosmosDBOutput
attributes and doesn't convert the parameter to a return value. collectionName
and ConnectionStringSetting
.System.Web.Http
usage:
QueryCollection
should have been converted to QueryString.CreateHttpRequest
no longer has a ReadAsStringAsync
method, needs to be converted to create StreamReader
from HttpRequest.Body
<Something>MessageResult
and they needed to be converted to BadRequestObjectResult
, StatusCodeResult
, ObjectResults
, etcstartup.cs
with [assembly: FunctionsStartup(typeof(Startup))]
but I think it should have converted to HostBuilder.ConfigureServices
in the newly added prrogram.cs
and removed startup.cs
?<ImplicitUsings>enabled</ImplicitUsings>
, but pretty sure "enabled"
should be "enable"
(or "true"
). Newtonsoft.Json
serializers to System.Text.Json
but didn't convert any of the POCO models that were using Newtonsoft-specific attributes. This annoyingly causes runtime exceptions. And there's no way to swap back to Newtonsoft and System.Text.Json
has functional differences which might be breaking (like a lack of property-level JsonProperty
DefaultValue
handling)launchSettings.json
serviceDependencies.json
serviceDependencies.local.json
serviceDependencies.local.json.user
<FrameworkReference Include="Microsoft.AspNetCore.App" />
which wasn't in the existing project wasn't added to the upgraded project. Should the upgrade assistant be adding AspNetCore framework reference to upgraded Function Apps too?The upgrade assistant should be doing "best effort" to convert and showing a warning if there's a problem.
Thank you for the detailed write up! I'm going to consider the original issue resolved and repurpose this issue to track migration assistant enhancements - sound fair?
From my perspective:
Re: samples - we have more scenarios documented here: https://github.com/Azure/azure-functions-dotnet-worker/blob/main/samples/Extensions/CosmosDB/CosmosInputBindingFunctions.cs
Thanks for the link, I just found the Examples on learn.microsoft.com were insufficient and ideally expanded. And if it doesn't exist then linking to the github samples from learn.microsoft.com page would be helpful too.
Had Dependency injection in startup.cs with [assembly: FunctionsStartup(typeof(Startup))] but I think it should have converted to HostBuilder.ConfigureServices in the newly added prrogram.cs and removed startup.cs?
Correct, DI should be moved to Program.cs and Startup.cs should be removed.
It went from using Newtonsoft.Json serializers to System.Text.Json but didn't convert any of the POCO models that were using Newtonsoft-specific attributes. This annoyingly causes runtime exceptions. And there's https://github.com/Azure/azure-functions-dotnet-worker/issues/2131 and System.Text.Json has functional differences which might be breaking (like a lack of property-level JsonProperty DefaultValue handling)
You should be able to configure a Newtonsoft serializer (example). There is unfortunately a bug with this when using AspNetCore .ConfigureFunctionsWebApplication
. But you should still be able to do this using .ConfigureFunctionsWorkerDefaults
if you don't need AspNetCore. The Cosmos extension uses the worker config serializer (as shown in the example linked) as the serializer for the CosmosClientOptions.
New project created a mix of files in the Properties folder, but they weren't added during the upgrade to the existing project. Does it need them?
I think you only need launchSettings.json
for VS, the rest can probably be removed.
The new project also added
which wasn't in the existing project wasn't added to the upgraded project. Should the upgrade assistant be adding AspNetCore framework reference to upgraded Function Apps too?
<FrameworkReference Include="Microsoft.AspNetCore.App" />
is only required if you want to use the new HTTP extension Worker.Extensions.Http.AspNetCore
(same for ConfigureFunctionsWebApplication
), otherwise you don't need it.
It went from using Newtonsoft.Json serializers to System.Text.Json but didn't convert any of the POCO models that were using Newtonsoft-specific attributes. This annoyingly causes runtime exceptions. And there's https://github.com/Azure/azure-functions-dotnet-worker/issues/2131 and System.Text.Json has functional differences which might be breaking (like a lack of property-level JsonProperty DefaultValue handling)
You should be able to configure a Newtonsoft serializer (example). There is unfortunately a bug with this when using AspNetCore
.ConfigureFunctionsWebApplication
. But you should still be able to do this using.ConfigureFunctionsWorkerDefaults
if you don't need AspNetCore. The Cosmos extension uses the worker config serializer (as shown in the example linked) as the serializer for the CosmosClientOptions.
When debugging the Cosmos input binding's JSON deserializing, I saw Microsoft.Azure.Functions.Worker.CosmosDBConverter
has a hard-coded JsonSerializerOptions
when deserializing which would prevent any settings override in Program.cs.
Is that CosmosDBConverter
only used by Function Apps with the ASP.NET Core integrations (is that the bug you refer to)?
Is a different DB converter used when the Function app doesn't use ASP.NET Core integration?
CosmosDBConverter has a hard-coded JsonSerializerOptions when deserializing which would prevent any settings override in Program.cs.
Yes it does have a hardcoded JsonSerializerOptions
; what I was trying to convey is that there is a difference between that serializer and the cosmos client serializer that can be configured. The one that is hardcoded in the Converter is used when binding to a POCO. And the other is used by the CosmoClient, this one can be configured via workerOptions.Serializer.
Wanting the latter one to be configurable is perfectly valid feedback, and likely was an oversight. Please open a new issue to help us triage and address that.
Is that CosmosDBConverter only used by Function Apps with the ASP.NET Core integrations (is that the bug you refer to)? Is a different DB converter used when the Function app doesn't use ASP.NET Core integration?
No, this converter is used for all Cosmos input bindings.
Description
I had a .net 6 in-process model function app which has a HTTP Trigger and a CosmosDBInput binding. I ran the .net upgrade assistant to convert to a .net 8 isolated worker model. It claimed success but left me with a bunch of Cosmos binding related errors. I tried fixing them manually and got it running, but now I'm getting a really strange runtime exception "FunctionInputConverterException".
It seems to wrap the real failure to find assembly Microsoft.Bcl.AsyncInterfaces 5.0.0.0
Related complaint: The examples section for isolated model is very sparse and deserves to expanded to cover more examples.
As far as I can tell from the Usage section, it should support IEnumerable and if I don't specify Id or SqlQuery, then it will return all documents.
The CosmosDBInput binding is just supposed to return all items in the database's container but fails with below exception. I'm not using any variables from the route (which is a known bug)
Steps to reproduce