Azure / azure-functions-host

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

ping response protocol #3178

Open jocawtho opened 6 years ago

jocawtho commented 6 years ago

Investigative information

Please provide the following:

Repro steps

Provide the steps required to reproduce the problem:

Issue started with 6/19 release Not sure how to repro, it might just be on unhealthy apps. I found a symptomatic app and rdp'd to the worker to investigate. Using netmon, i found that pings to the root of the site were returning "Host is not running" or some such message. This causes a protocol exception with the 204 http status code in the scale controller. from NetMon: "HTTPPayloadLine: Function host is not running."

Expected behavior

Provide a description of the expected behavior.

There should be no http response with a 204 status code.

Actual behavior

Provide a description of the actual behavior observed.

from NetMon: "HTTPPayloadLine: Function host is not running."

Known workarounds

Provide a description of any known workarounds.

This isnt "broken" the ping still gets through. It just causes thousands and thousands of errors in the scale controller

paulbatum commented 6 years ago

So I'm looking at this:

https://github.com/Azure/azure-functions-host/blob/1b7ded23fd9213b2fe605da05ed7f4d94578ada2/src/WebJobs.Script.WebHost/WebJobsApplicationBuilderExtension.cs#L28-L33

and this:

https://github.com/Azure/azure-functions-host/blob/1b7ded23fd9213b2fe605da05ed7f4d94578ada2/src/WebJobs.Script.WebHost/Middleware/HomepageMiddleware.cs#L32-L46

and this:

https://github.com/Azure/azure-functions-host/blob/1b7ded23fd9213b2fe605da05ed7f4d94578ada2/src/WebJobs.Script.WebHost/Middleware/HttpExceptionMiddleware.cs#L19-L31

And i'm wondering, is it possible that the HttpExceptionMiddleware is catching an exception after the 204 was set, and then its setting the reason phrase and thats the issue?

I'm wondering why are we even setting this reason phrase? Apparently its not supported in HTTP/2: https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.http.features.ihttpresponsefeature.reasonphrase?view=aspnetcore-2.1

According to RFC 7230, we are misusing reason phrase:

The reason-phrase element exists for the sole purpose of providing a textual description associated with the numeric status code, mostly out of deference to earlier Internet application protocols that were more frequently used with interactive text clients. A client SHOULD ignore the reason-phrase content.

https://tools.ietf.org/html/rfc7230#section-3.1.2

I suggest we remove the behavior that is setting the reason phrase, and see if that resolves this issue.

jocawtho commented 5 years ago

Hey Paul, pinging this again. Did we have a reason for this yet? Thanks :)

fabiocav commented 5 years ago

@jocawtho are you still seeing this? This issue got lost in the triage process.

jocawtho commented 5 years ago

Yes, This is still the most noisy error we see in the scale controller