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

Return wrapped network.ErrReset error type with more info #3156

Open
MarcoPolo opened this issue Jan 24, 2025 · 4 comments
Open

Return wrapped network.ErrReset error type with more info #3156

MarcoPolo opened this issue Jan 24, 2025 · 4 comments
Labels
exp/beginner Can be confidently tackled by newcomers good first issue Good issue for new contributors help wanted Seeking public contribution on this issue P2 Medium: Good to have, but can wait until someone steps up

Comments

@MarcoPolo
Copy link
Collaborator

MarcoPolo commented Jan 24, 2025

Right now we replace specific stream errors with network.ErrReset (example). We should instead return a new error type that unwraps to a network.ErrReset with a .Unwrap() []error method and includes addition error information gleaned from the transport. This is at least a problem in the WebTransport and QUIC transports. Note that we probably want the multi-error return version and return both a network.ErrReset and the underlying stream error.

@MarcoPolo MarcoPolo added exp/beginner Can be confidently tackled by newcomers good first issue Good issue for new contributors help wanted Seeking public contribution on this issue P2 Medium: Good to have, but can wait until someone steps up labels Jan 24, 2025
@Khwahish29
Copy link

I'd like to take this up!
Please assign this to me

@marten-seemann
Copy link
Contributor

with a .Is method

The better way to do this is using Unwrap. Took me a while to figure this out. Have a look at the quic-go errors, especially the TransportError.

@MarcoPolo
Copy link
Collaborator Author

@MarcoPolo
Copy link
Collaborator Author

@Khwahish29 . Sounds good. Could you please either wait until #2927 is merged or base your changes on that PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exp/beginner Can be confidently tackled by newcomers good first issue Good issue for new contributors help wanted Seeking public contribution on this issue P2 Medium: Good to have, but can wait until someone steps up
Projects
None yet
Development

No branches or pull requests

3 participants