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

Errors returned by Host methods should be more descriptive #2334

Closed
Wondertan opened this issue Jun 5, 2023 · 2 comments · Fixed by #2331
Closed

Errors returned by Host methods should be more descriptive #2334

Wondertan opened this issue Jun 5, 2023 · 2 comments · Fixed by #2331

Comments

@Wondertan
Copy link
Contributor

Wondertan commented Jun 5, 2023

The Host provides two main user-facing methods: Connect and NewStream. Usually, they work great, however, when something goes the wrong way, the returned errors are not helpful to debug what actually went wrong.

A simple example is a deadlined context on NewStream. When this happens, a developer will only see context deadline exceeded, which gives zero information, while it could happen during reconnection, protocol negotiation, awaiting identify, or something else(?). This information is essential to be able to understand what could go wrong in NewStream quickly.

Additionally, the errors would contain additional information about particular stream open attempts, like which peer and protocols. This is especially useful for projects that have multiple sub-protocols running in parallel and talking to the same or different set of peers.

Ideally, we should decide on the API pattern we want to follow for this in the whole go-libp2p ecosystem, as the Host is only the tip of the iceberg, and there are subcomponents and various must-have protocols which would benefit from it.

My proposal is to define a new type in core for Host that contains all the information and wraps errors that happened down the stack. On top of that, the Host implementations would add more context about the failed stage of a failed method. The proposal is implemented in #2331

P.S. Some form of tracing infrastructure in go-libp2p would also solve this problem. It may directly use OpenTelemetry tracing, which has a stable API, or may define a little interface shim for this.

@marten-seemann
Copy link
Contributor

This needs to fit into the larger story of how ewe report errors, see libp2p/specs#479.

@Wondertan
Copy link
Contributor Author

The proposed error codes there are excellent, but they are too low-level. The proposed error type is complementary to these error codes and not exclusive. The error codes won't fully cover there problem described here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🎉 Done
Development

Successfully merging a pull request may close this issue.

2 participants