[release/7.0-rc1] Remove assert from Http3Connection.SendAsync

[rzikm]

Manual backport of #74348 to release/7.0-rc1

Customer Impact

None, this change affects only tests (Asserts don't run in Release configuration). We have seen an increased number of crashes on Debug runs on Alpine Linux where we recently enabled HTTP/3 test coverage. This PR serves to mainly de-noise CI reports.

Testing

Functional tests pass on CI, and the affected test was run many times in a tight loop on the affected configuration.

Risk

Low (or None even). The change does not affect the behavior of shipped product.

[ghost]

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

Issue Details

---

Manual backport of https://github.com/dotnet/runtime/pull/74348to release/7.0-rc1

/cc @wfurt

Customer Impact

None, this change affects only tests (Asserts don't run in Release configuration). We have seen an increased number of crashes on Debug runs on Alpine Linux where we recently enabled HTTP/3 test coverage. This PR serves to mainly de-noise CI reports.

Testing
Functional tests pass on CI, and the affected test was run many times in a tight loop on the affected configuration.

Risk
Low (or None even). The change does not affect the behavior of production builds.

Author:	rzikm
Assignees:	-
Labels:	area-System.Net.Http
Milestone:	-

[danmoseley]

None, this change affects only tests

It's a new catch block in release bits, right?

[danmoseley]

I'm fine with removing the assert, since it's debug only and helps tests. Do we have a good enough reason for changing the release codepath though?

[carlossanlop]

@danmoseley does this one make the cut for release/7.0-rc1 or should it retarget release/7.0?

await danmoseley.ApprovalAsync();

[rzikm]

I'm fine with removing the assert, since it's debug only and helps tests. Do we have a good enough reason for changing the release codepath though?

The added catch block in product code does not have any user-visible runtime effects AFAICT. I added it to avoid calling Abort() and storing the QuicException as Http3Connection._abortException since it would be misleading during dump investigation (by the time the exception is thrown, the connection was already closed before, so the call does really do anything).

I can split the change and port only the assert removal if this causes any concerns.

[wfurt]

I would expect it mostly impacts our tests. Since we are abandoning 7.0-rc1 soon I don't think it matters.

[danmoseley]

If it will help stabilize tests, yes please a PR with just the assert removed against release/7.0 would be fine. any test only changes don't need approval. In general we should port any test stabilization/improvements.

I'll close this