Azure / azure-functions-host

The host/runtime that powers Azure Functions
https://functions.azure.com
MIT License
1.93k stars 441 forks source link

Using comments in host.json fails silently for isolated worker using the ASP.NET integration #9583

Open klemmchr opened 1 year ago

klemmchr commented 1 year ago

When using comments in host.json the file is not loaded and no error is reported. All settings set in host.json are ignored and the default settings are applied. There is no log output related to this issue.

Repro steps

  1. Add some easy to test setting in host.json (e.g. extensions.http.routePrefix
  2. Add a comment somewhere in host.json
  3. Start the function and observe that the host.json was not loaded. No error is generated.

Expected behavior

The application fails to start and reports an error.

Actual behavior

The application starts and no error is shown. The settings are not loaded.

bhagyshricompany commented 1 year ago

The behavior you are describing is expected in Azure Functions. The host.json file in Azure Functions is used to configure various settings for the host environment and your functions, such as function timeout, CORS settings, and extensions configuration. However, comments are not supported in the host.json file, and they do not cause the application to fail or produce errors.

Here's why comments in host.json behave this way:

  1. JSON Specification: JSON (JavaScript Object Notation) is a data interchange format, and according to the JSON specification, it does not support comments. Comments are not part of the JSON standard, so JSON parsers, including the one used by Azure Functions, will ignore comments.

  2. Azure Functions Host: The Azure Functions runtime is designed to be robust and fault-tolerant. It is configured to ignore unsupported constructs in configuration files like host.json rather than failing the application startup.

To address this issue, consider using other means to document your configuration settings, such as creating a separate document or readme file that describes the settings and their purposes. You can also use descriptive key names to make the purpose of each configuration setting clear within host.json.

Here's an example of how you might structure host.json with descriptive key names:

{
  "extensions": {
    "http": {
      "routePrefix": "api"
    },
    "otherExtension": {
      "setting1": "value1",
      "setting2": "value2"
    }
  }
}

In this example, the key names (routePrefix, setting1, setting2) are chosen to be descriptive, making it easier for someone reading the configuration to understand the purpose of each setting without the need for comments.

klemmchr commented 1 year ago

However, comments are not supported in the host.json file, and they do not cause the application to fail or produce errors.

This is not true. They prevent the application from loading the host.json file which can be mandatory depending on the use case. If for example the routePrefix is not read all HTTP endpoints are configured with the wrong route.

Comments are not part of the JSON standard, so JSON parsers, including the one used by Azure Functions, will ignore comments.

Which is not the case either since using JSON comments in host.json causes the application to not properly load the configuration. This is what this issue is all about. I've spend six hours debugging this issue just to find out that some commented out setting causes the whole application to not even start properly.

The Azure Functions runtime is designed to be robust and fault-tolerant. It is configured to ignore unsupported constructs in configuration files like host.json rather than failing the application startup.

This is not true either. If the host.json includes some invalid characters rendering the JSON format invalid the function host won't start.

image

To address this issue, consider using other means to document your configuration settings, such as creating a separate document or readme file that describes the settings and their purposes. You can also use descriptive key names to make the purpose of each configuration setting clear within host.json.

Excuse me, please what?! This is a bug report and not some SO question asking for advice on how to document my settings. My issue is that commenting out certain keys in the host.json creates an unexpected result and provides no warning or notice whatsoever. This cannot be intended if a structurally invalid host.json is preventing the host from starting.

I would like to request a member or contributor of this repository to address this issue rather than some random person telling me how to document my stuff when I'm reporting a bug.

bhagyshricompany commented 1 year ago

Sorry for inconvenience.Thanks for providing screen shot to get idea.it seems it coming will discuss and update for this.

bhagyshricompany commented 11 months ago

@fabiocav pls comment validate.

sebastianburckhardt commented 11 months ago

IMO the most robust solution (i.e. least likely to unnecessarily break the application) is to ignore comments in host.json during parsing. From what I can tell this is also the original behavior of the functions host - prior to moving to the isolated worker. So, disallowing comments is a breaking change.

FYI, ignoring comments seems to be a simple matter of configuring System.Text.Json: https://learn.microsoft.com/en-us/dotnet/api/system.text.json.jsoncommenthandling?view=netcore-3.1

ishepherd commented 9 months ago

Edited Using the ASP.NET Core integration makes incoming HTTP requests fail, if there are comments in host.json.

The failures start if I use ConfigureFunctionsWebApplication.

With ConfigureFunctionsWorkerDefaults then there are no failures (but possibly the host.json is being silently ignored, I haven't verified that)

MS, before fixing this, please investigate how this failure is seen by the user. The exception is logged, but no response comes back. It just sits there until eventual App Service timeout (<html><head><title>500 - The request timed out....)

From what I can tell this is also the original behavior of the functions host - prior to moving to the isolated worker. So, disallowing comments is a breaking change.

Correct. Comments were always previously allowed in host.json. I suppose originally a side effect of Newtonsoft.Json, but Microsoft official samples relied (rely?) upon the behaviour.


System.Text.Json.JsonException: '/' is invalid after a value. Expected either ',', '}', or ']'. Path: $.logging | LineNumber: 10 | BytePositionInLine: 6.
   at System.Text.Json.ThrowHelper.ReThrowWithPath(ReadStack& state, JsonReaderException ex)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan`1 utf8Json, JsonTypeInfo`1 jsonTypeInfo, Nullable`1 actualByteCount)
   at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan`1 json, JsonTypeInfo`1 jsonTypeInfo)
   at System.Text.Json.JsonSerializer.Deserialize[TValue](String json, JsonSerializerOptions options)
   at Microsoft.Azure.Functions.Worker.Extensions.Http.AspNetCore.AspNetMiddleware.FunctionsEndpointDataSource.GetRoutePrefix(String hostJsonString) in D:\a\_work\1\s\extensions\Worker.Extensions.Http.AspNetCore\src\AspNetMiddleware\FunctionsEndpointDataSource.cs:line 133
   at Microsoft.Azure.Functions.Worker.Extensions.Http.AspNetCore.AspNetMiddleware.FunctionsEndpointDataSource.GetRoutePrefixFromHostJson(String scriptRoot) in D:\a\_work\1\s\extensions\Worker.Extensions.Http.AspNetCore\src\AspNetMiddleware\FunctionsEndpointDataSource.cs:line 127
   at Microsoft.Azure.Functions.Worker.Extensions.Http.AspNetCore.AspNetMiddleware.FunctionsEndpointDataSource.BuildEndpoints() in D:\a\_work\1\s\extensions\Worker.Extensions.Http.AspNetCore\src\AspNetMiddleware\FunctionsEndpointDataSource.cs:line 61
   at Microsoft.Azure.Functions.Worker.Extensions.Http.AspNetCore.AspNetMiddleware.FunctionsEndpointDataSource.get_Endpoints() in D:\a\_work\1\s\extensions\Worker.Extensions.Http.AspNetCore\src\AspNetMiddleware\FunctionsEndpointDataSource.cs:line 44
   at Microsoft.AspNetCore.Routing.CompositeEndpointDataSource.CreateEndpointsUnsynchronized()
   at Microsoft.AspNetCore.Routing.CompositeEndpointDataSource.EnsureEndpointsInitialized()
   at Microsoft.AspNetCore.Routing.CompositeEndpointDataSource.get_Endpoints()
   at Microsoft.AspNetCore.Routing.DataSourceDependentCache`1.Initialize()
   at System.Threading.LazyInitializer.EnsureInitializedCore[T](T& target, Boolean& initialized, Object& syncLock, Func`1 valueFactory)
   at System.Threading.LazyInitializer.EnsureInitialized[T](T& target, Boolean& initialized, Object& syncLock, Func`1 valueFactory)
   at Microsoft.AspNetCore.Routing.Matching.DataSourceDependentMatcher..ctor(EndpointDataSource dataSource, Lifetime lifetime, Func`1 matcherBuilderFactory)
   at Microsoft.AspNetCore.Routing.Matching.DfaMatcherFactory.CreateMatcher(EndpointDataSource dataSource)
   at Microsoft.AspNetCore.Routing.EndpointRoutingMiddleware.InitializeCoreAsync()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Routing.EndpointRoutingMiddleware.<Invoke>g__AwaitMatcher|10_0(EndpointRoutingMiddleware middleware, HttpContext httpContext, Task`1 matcherTask)
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application)
ericsson007 commented 8 months ago

I'm disappointed that there is no concrete progress on this issue. I experienced this when upgraded a Functions project from .net6 to .net8 and the VS 2022 Upgrade Assistant replaced "ConfigureFunctionsWorkerDefaults" with "ConfigureFunctionsWebApplication". There are no errors when starting the app but it doesn't respond to http requests. The line " Executing 'Functions.Common_Info' (Reason='This function was programmatically called via the host APIs.', Id=ea32ee64-a5bd-4bf5-9303-8e7634166a0d)" shows, but nothings happens, see image.

image

The correct behavior in my opinion is to parse the json even when there are comments, that will make it backward compatible. If that is not feasible an exception should be raised. Please fix this so the transition from .net6 to .net8 is smooth and doesn't waste hours for others that will run into this.

rahulsingii commented 6 months ago

Agree with @ericsson007, after upgrading to .net 8 and isolated, I was not sure why suddenly I am not able to debug locally. I added additional project to test the same but in the end found that I was not able to debug because there was a comment in my host.json file. This should be fixed!

BrettBailey commented 4 months ago

Having also fallen fowl of this I want to voice my agreement with @klemmchr. The JSON parser should be able tolerate comments properly, without silently failing.

Whilst comments may not be part of the JSON specification they are commonly used elsewhere in the stack, such as local.settings.json. It should be consistent. It needs to be fixed.

Coruscate5 commented 3 months ago

Echo everything here, seems like a pretty obvious regression.

The only reason the comments were present was due to VS 2022 warnings that were spurious and inconsistent with the schema validation, the "offensive" line was this:

//The property name warning is due to poor MSFT schema validation, and cannot be undone in JSON

mattfrear commented 2 months ago

I just wasted half a day on this too. Echoing what @BrettBailey says - comments are allowed in local.settings.json so they should be allowed in host.json too.

My observations are that my ServiceBusTrigger functions still worked but my HttpTrigger functions would timeout.

hubaksis commented 2 months ago

Hi everyone,

I've just spent 4 hours trying to figure out why a newly added function with HttpTrigger was timing out. No matter what I tried, it just kept timing out or throws an exception if requests stops. Out of sheer frustration, I created a new project and moved the code over line by line. To my surprise, the new project worked perfectly, while the old one continued to fail.

After painstakingly comparing every file in all folders, I discovered the culprit: a single commented line in hosts.json.

I cannot express how incredibly frustrating it is to lose hours of productivity over such a minor issue. It’s infuriating that this problem still exists.

klemmchr commented 2 months ago

@paulbatum can we get any attention on this (rather simple) issue? There are dozens of people affected by this.

daletho commented 1 month ago

What a waste of time debugging this. At no point in the isolated model upgrade documentation does it mention comments are no longer supported in the host.json file.

JimmyJonesJr commented 1 month ago

I also lost a good 5+ hours to this. You would think at the very least the official upgrade tooling would check for this sort of thing. I've been very disappointed with the upgrade process given how it's been touted as very low effort officially.

ifan-t commented 1 month ago

Echoing the above. Thankfully didn't waste quite as much time as some.

NattyMojo commented 3 weeks ago

Checking in here, has anyone experienced this in local.settings.json as well? I haven't been having an issue with the host.json to my knowledge yet, but have had to remove all the comments from settings during the upgrade.

Edit: judging from above most people are able to keep comments in local settings, just not host :(

drdamour commented 2 weeks ago

@NattyMojo local.settings.json looks to have been https://github.com/Azure/azure-functions-core-tools/issues/3856