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

upstream connect error or disconnect/reset before headers. reset reason: protocol error #43874

Open
1 task done
olizarevichroman opened this issue Sep 9, 2022 · 11 comments
Open
1 task done
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update.
Milestone

Comments

@olizarevichroman
Copy link

olizarevichroman commented Sep 9, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

We've launched ASP NET Core 7 Preview app in a Kubernetes cluster with Envoy as a proxy.

When we send PUT requests and a response contains a body - we have 502 upstream connect error or disconnect/reset before headers. reset reason: protocol error for PUT. For GET it works, for PUT without a response body it also works.

After downgrading to NET 6 it works

Expected Behavior

Result status code should be 2xx

Steps To Reproduce

Send a PUT HTTP request to an endpoint that returns a result with a body.

Exceptions (if any)

No response

.NET Version

7.0.0-preview.7.22375.6

Anything else?

ASP NET Core runtime version: 7.0.0-preview.7.22376.6
NET Core runtime version: 7.0.0-preview.7.22375.6

@Tratcher
Copy link
Member

Tratcher commented Sep 9, 2022

Please share the server logs for this request.

https://docs.microsoft.com/en-us/aspnet/core/fundamentals/logging/?view=aspnetcore-6.0#configure-logging

What was the response status code for 6.0? A 204? 204's aren't allowed to have a response body, and 7.0 enforces that.

@Tratcher Tratcher added the Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. label Sep 9, 2022
@ghost
Copy link

ghost commented Sep 9, 2022

Hi @olizarevichroman. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

@olizarevichroman
Copy link
Author

Hi @Tratcher
logs.txt

@ghost ghost added Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. and removed Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels Sep 12, 2022
@olizarevichroman
Copy link
Author

olizarevichroman commented Sep 12, 2022

@Tratcher
I've found that we tried to return ObjectResult passing an empty object as a parameter with 204 status code.
So, after changing it with NoContentResult it works fine.

Thanks!

@olizarevichroman
Copy link
Author

@Tratcher Also, what did you mean by 204's aren't allowed to have a response body, and 7.0 enforces that.?

Do you know how does 7.0 enforce that?

@Tratcher
Copy link
Member

Was that log from 6 or 7? I see a 204 with a Content-Length of 2. In 7.0 Kestrel will throw an exception if the status code is 204 and the app writes to the body. I don't see an exception in the logs, but I also don't see the 2 byte body being written.

@olizarevichroman
Copy link
Author

olizarevichroman commented Sep 12, 2022

@Tratcher
yup, i have an exception in logs like here

But response still goes back without error) Tried it via Postman and this action, getting back 204. Is it expected?

[HttpPut("update")]
public IActionResult Update() => new ObjectResult(new {})
{
    StatusCode = StatusCodes.Status204NoContent,
};

@Tratcher
Copy link
Member

Tratcher commented Sep 12, 2022

That's not ideal. The exception should make it change to a 500 at least. I think @adityamandaleeka was still cleaning that up.

@olizarevichroman
Copy link
Author

olizarevichroman commented Sep 12, 2022

Sounds good
@Tratcher, thanks a lot)

@Tratcher
Copy link
Member

This looks related to #43316 (comment).

@Tratcher Tratcher added this to the .NET 8 Planning milestone Sep 12, 2022
@ghost
Copy link

ghost commented Sep 12, 2022

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 25, 2023
@amcasey amcasey modified the milestones: .NET 8 Planning, Backlog Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update.
Projects
None yet
Development

No branches or pull requests

6 participants
@Tratcher @javiercn @amcasey @olizarevichroman and others