Skip to content

Conversation

@brettsam
Copy link
Member

No description provided.

@brettsam brettsam requested review from ankitkumarr and fabiocav May 26, 2021 13:57
@brettsam brettsam changed the title updating all packages to preview4 [pack] updating all packages to preview4 May 26, 2021
@brettsam
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

public class AllowSynchronousIOMiddlewareTests
{
[Fact]
[Fact(Skip = "Seems very difficult to trigger this failure now.")]
Copy link
Member Author

Choose a reason for hiding this comment

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

We have a test that does a synchronous read of the request and we expect it to fail (just to confirm that our following tests are actually doing what they're supposed to). This stopped working when I moved to .NET 6.0.

It looks like a fix in the TestServer now writes the Content-Length automatically (since 5.0 rc1)

Because of this, the HttpTrigger now sees this as readable and calls EnableBuffering(), which swaps out the Request body stream for one that does not throw an exception with sync reads. This is the behavior that happens in prod -- but now we're seeing it here b/c the TestServer was fixed.

I tried a bunch of other stuff and was unable to throw this exception from within a function. Even this didn't work in the TestServer: #7064

I'm skipping this now -- I'm not sure how valuable this is going forward but we can revisit.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering if we need a follow-up issue to either remove this test or fix / modify this test.

@brettsam brettsam merged commit cca296f into v4.x Jun 23, 2021
@brettsam brettsam deleted the brettsam/preview4 branch June 23, 2021 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants