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

Revert "Revert "libp2p: add QUIC and WebTransport to default listen addresses"" #10854

Merged
merged 5 commits into from
May 30, 2023

Conversation

magik6k
Copy link
Contributor

@magik6k magik6k commented May 10, 2023

Reverts #10852, fixes tests

@magik6k magik6k requested a review from a team as a code owner May 10, 2023 20:35
@arajasek
Copy link
Contributor

arajasek commented May 10, 2023

@magik6k Thanks for deflaking this test. I may be imagining it, but I'm pretty sure these deal errored error attempting to close stream: close called for canceled stream 13 failures also only happen with the change being un-reverted here. Can you please take a look?

@marten-seemann
Copy link
Contributor

The error comes from quic-go: https://github.com/quic-go/quic-go/blob/v0.34.0/send_stream.go#L404-L407
It happens when you first reset the stream, then call close. In most circumstances, it can be ignored. Is there any reason you're surfacing this kind of error?

@arajasek
Copy link
Contributor

The error comes from quic-go: https://github.com/quic-go/quic-go/blob/v0.34.0/send_stream.go#L404-L407 It happens when you first reset the stream, then call close. In most circumstances, it can be ignored. Is there any reason you're surfacing this kind of error?

It looks this error makes the deal pipeline think the deal has failed. I'd have to dig to see where that happens (and how we could ignore it, assuming we should be doing so).

@magik6k magik6k merged commit b997f4a into master May 30, 2023
@magik6k magik6k deleted the revert-10852-sbansal/revert-10848 branch May 30, 2023 17:34
@marten-seemann
Copy link
Contributor

The error comes from quic-go: https://github.com/quic-go/quic-go/blob/v0.34.0/send_stream.go#L404-L407 It happens when you first reset the stream, then call close. In most circumstances, it can be ignored. Is there any reason you're surfacing this kind of error?

It looks this error makes the deal pipeline think the deal has failed. I'd have to dig to see where that happens (and how we could ignore it, assuming we should be doing so).

@arajasek What did you digging reveal?

Why was this change silently reverted?

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