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

NullRef in GetFallbackWorkerConfig #9859

Open
mathewc opened this issue Feb 2, 2024 · 3 comments · May be fixed by #10550
Open

NullRef in GetFallbackWorkerConfig #9859

mathewc opened this issue Feb 2, 2024 · 3 comments · May be fixed by #10550
Assignees
Labels

Comments

@mathewc
Copy link
Member

mathewc commented Feb 2, 2024

I was just investigating a CRI and I saw the following null ref in the logs. Looking more widely I see it's happening across many apps and versions (v4) on different codepaths. Here's the stacktrace:

System.NullReferenceException : Object reference not set to an instance of an object.
   at Microsoft.Azure.WebJobs.Script.FunctionMetadataManager.GetFallbackWorkerConfig() at /_/src/WebJobs.Script/Host/FunctionMetadataManager.cs : 129
   at Microsoft.Azure.WebJobs.Script.FunctionMetadataManager.LoadFunctionMetadata(Boolean forceRefresh,Boolean includeCustomProviders,IFunctionInvocationDispatcher dispatcher,IList`1 workerConfigs) at /_/src/WebJobs.Script/Host/FunctionMetadataManager.cs : 137
   at Microsoft.Azure.WebJobs.Script.FunctionMetadataManager.GetFunctionMetadata(Boolean forceRefresh,Boolean applyAllowList,Boolean includeCustomProviders,IList`1 workerConfigs) at /_/src/WebJobs.Script/Host/FunctionMetadataManager.cs : 90
   at Microsoft.Azure.WebJobs.Script.WebHost.Management.WebFunctionsManager.GetFunctionsMetadata(Boolean includeProxies,Boolean forceRefresh) at /_/src/WebJobs.Script.WebHost/Management/WebFunctionsManager.cs : 74
   at async Microsoft.Azure.WebJobs.Script.WebHost.Management.WebFunctionsManager.GetFunctionsMetadata(??)
@kshyju
Copy link
Member

kshyju commented Feb 7, 2024

@liliankasem recently fixed #9484 Which has same stack trace. We need to check logs to confirm customer is using a host version with this fix.

@mathewc
Copy link
Member Author

mathewc commented Feb 7, 2024

@liliankasem recently fixed #9484 Which has same stack trace. We need to check logs to confirm customer is using a host version with this fix.

Running the below query in production shows the issue is happening across versions, including the latest host version. The PR you reference seems to make changes way upstream. I think we need to fix this actual method to prevent errors in any callstacks. E.g. it should do proper null checks and return an empty collection if needed.

All("FunctionsLogs")
| where PreciseTimeStamp > ago(1d)
| where Level == 2
| where Details startswith "System.NullReferenceException : Object reference not set to an instance of an object."
| where Details contains "GetFallbackWorkerConfig()"
| summarize count() by HostVersion
| order by count_ desc
| take 10

@soninaren
Copy link
Member

The issue primarily stems from the change made in this PR. #9264. The goal of the change in #9264 was to load the latest worker.config when LoadFunctionMetadata() in FunctionMetadataManager. This would allow the customers to use language worker arguments. Prior to this change customers were not able to pass language worker arguments in consumption. The worker config was statically loaded in placeholder and was carried through to the function apps.

Unfortunately, as a result when the LoadFunctionMetadata() is called when the Script host is not available, we run into null ref exception. To resolve this, we would have to examine each scenario where LoadFunctionMetadata() and identify a proper resolution to each one of them.

@fabiocav fabiocav assigned jviau and unassigned soninaren Oct 17, 2024
@jviau jviau linked a pull request Oct 21, 2024 that will close this issue
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants