Skip to content

Conversation

@pakrym
Copy link
Contributor

@pakrym pakrym commented Jan 8, 2019

Fixes #6415

Description

When using in-proc mode with ANCM, POST requests can randomly result in contex.Request.Body.ReadAsync throwing because successful error code is not handled correctly in all places.

Regression?

Regression from out-of-process hosting model.

Risk

Very Low. Handling same error code we did before but in more centralized fashion.

@pakrym pakrym added the Servicing-consider Shiproom approval is required for the issue label Jan 8, 2019
@pakrym pakrym requested a review from jkotalik January 8, 2019 16:28
@pakrym
Copy link
Contributor Author

pakrym commented Jan 8, 2019

cc @muratg

@muratg muratg added this to the 2.2.x milestone Jan 8, 2019
@jamshedd jamshedd added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Jan 8, 2019
@natemcmaster
Copy link
Contributor

Please wait until https://github.com/aspnet/AspNetCore-Internal/issues/1610 is resolved before merging.

@natemcmaster natemcmaster modified the milestones: 2.2.x, 2.2.2 Jan 8, 2019
@jamshedd
Copy link
Member

jamshedd commented Jan 8, 2019

Approved for 2.2.2.

@pakrym pakrym force-pushed the pakrym/eof-hr-2-2 branch from fb2c641 to fbe72d1 Compare January 14, 2019 22:48
@pakrym pakrym merged commit 437baf6 into release/2.2 Jan 15, 2019
</PropertyGroup>
<PropertyGroup Condition=" '$(VersionPrefix)' == '2.2.2' ">
<PackagesInPatch>
@aspnet/signalr;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI @BrennanConroy

Intentional removal? We need this if we want to patch NPM packages.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😢

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bad merge.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pakrym You're fixing it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

@natemcmaster natemcmaster deleted the pakrym/eof-hr-2-2 branch January 18, 2019 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Servicing-approved Shiproom has approved the issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants