Skip to content

HTTP/3 GET Aborted stress scenario fix - #55582

Merged
CarnaViire merged 1 commit into
dotnet:mainfrom
CarnaViire:stress-get-abort-check
Jul 14, 2021
Merged

HTTP/3 GET Aborted stress scenario fix#55582
CarnaViire merged 1 commit into
dotnet:mainfrom
CarnaViire:stress-get-abort-check

Conversation

@CarnaViire

@CarnaViireCarnaViire commented Jul 13, 2021

Copy link
Copy Markdown
Member

This adds checks for HTTP/3 specific exceptions to GET Aborted scenario

@ghost

ghost commented Jul 13, 2021

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

This adds checks for HTTP/3 specific exceptions to GET Aborted scenario

Author:CarnaViire
Assignees:-
Labels:

area-System.Net.Quic

Milestone:-

@CarnaViire

CarnaViire commented Jul 13, 2021

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-libraries stress-http

@azure-pipelines

azure-pipelinesBot commented Jul 13, 2021

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

wfurt
wfurt approved these changes Jul 13, 2021
View reviewed changes

@wfurtwfurt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

ManickaP
ManickaP approved these changes Jul 13, 2021
View reviewed changes

@ManickaPManickaP left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks.

Comment threadsrc/libraries/System.Net.Http/tests/StressTests/HttpStress/ClientOperations.cs
switch (name)
{
case "QuicStreamAbortedException":
if (e.InnerException?.Message?.Equals("Stream aborted by peer (258).") ?? false) // 258 = H3_INTERNAL_ERROR (0x102)

@ManickaPManickaPJul 13, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Once we finally clean up the error reporting, we should just check for status within the message as we do with H/2 since this is fragile. But it's good enough for now!

@CarnaViireCarnaViireJul 13, 2021

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I totally agree 😊

@CarnaViire

CarnaViire commented Jul 13, 2021

Copy link
Copy Markdown
MemberAuthor

GET Aborted looks good https://dev.azure.com/dnceng/public/_build/results?buildId=1235274&view=logs&j=0da5d1d9-276d-5173-c4c4-9d4d4ed14fdb&t=1451f5f3-0108-5a08-5b92-e984b2a85bbd

client_1 | [07/13/2021 18:21:45] Total: 8,041 Runtime: 00:02:00
client_1 | 0: GET Success: 533	Canceled: 38	Fail: 0
client_1 | 1: GET Partial Success: 535	Canceled: 37	Fail: 0
client_1 | 2: GET Headers Success: 533	Canceled: 39	Fail: 0
client_1 | 3: GET Parameters Success: 536	Canceled: 37	Fail: 0
client_1 | 4: GET Aborted Success: 537	Canceled: 36	Fail: 0
client_1 | 5: POST Success: 539	Canceled: 34	Fail: 0
client_1 | 6: POST Multipart Data Success: 528	Canceled: 45	Fail: 0
client_1 | 7: POST Duplex Success: 538	Canceled: 35	Fail: 0
client_1 | 8: POST Duplex Slow Success: 528	Canceled: 45	Fail: 0
client_1 | 9: POST Duplex Dispose Success: 536	Canceled: 37	Fail: 0
client_1 | 10: POST ExpectContinue Success: 0	Canceled: 30	Fail: 0
client_1 | 11: HEAD Success: 534	Canceled: 38	Fail: 0
client_1 | 12: PUT Success: 548	Canceled: 24	Fail: 0
client_1 | 13: PUT Slow Success: 539	Canceled: 32	Fail: 0
client_1 | 14: GET Slow Success: 528	Canceled: 42	Fail: 0
client_1 | TOTAL Success: 7,492	Canceled: 549	Fail: 0

@geoffkizer

geoffkizer commented Jul 13, 2021

Copy link
Copy Markdown
Contributor

What's up with POST ExpectContinue?

@CarnaViire

CarnaViire commented Jul 13, 2021

Copy link
Copy Markdown
MemberAuthor

What's up with POST ExpectContinue?

What we believe might be a Kestrel bug, as it happens with HTTP/2 too #55261

@CarnaViire
CarnaViire merged commit 8bf8030 into dotnet:mainJul 14, 2021
@CarnaViire
CarnaViire deleted the stress-get-abort-check branch July 14, 2021 18:11
@karelzkarelz added this to the 6.0.0 milestone Jul 15, 2021
@ManickaPManickaP mentioned this pull request Jul 20, 2021
Merged
@MihaZupanMihaZupan mentioned this pull request Aug 5, 2021
Closed
@ghostghost locked as resolved and limited conversation to collaborators Aug 14, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@ManickaPManickaPManickaP approved these changes
@wfurtwfurtwfurt approved these changes
@geoffkizergeoffkizerAwaiting requested review from geoffkizer

Assignees

@CarnaViireCarnaViire

Labels

area-System.Net.Quic

Projects

None yet

Milestone

6.0.0

Development

Successfully merging this pull request may close these issues.

5 participants

@CarnaViire@geoffkizer@ManickaP@wfurt@karelz