error: Refactor the overarching litep2p::Error #204
Labels
enhancement
New feature or request
Comments
lexnv
added a commit
that referenced
this issue
Aug 21, 2024
… error reporting (#206) The purpose of this PR is to pave the way for making the Identify protocol more robust, which is currently linked with the low number of peers and connective issues over a long period of time - paritytech/polkadot-sdk#4925 This PR adds a coherent `DialError` that exposes the minimal information users need to know about dial failures. - paritytech/polkadot-sdk#5239 A new litep2p event is added for reporting multiple dial errors that occur on different protocols back to the user: ```rust /// A list of multiple dial failures. ListDialFailures { /// List of errors. /// /// Depending on the transport, the address might be different for each error. errors: Vec<(Multiaddr, DialError)>, }, ``` This event eases the debugging of substrate connectivity issues. At the same time, it can be used in a future PR to inform back to the Identify protocol which self-reported addresses of some peers are unreachable: - #203 ### Next Steps - Add more tests - Warp sync + sync full nodes since this is touching individual transports ### Future Work - The overarching `litep2p::Error` needs a closer look and a refactoring: - #204 - #128 - ConnectionError event for individual transports can be simplified: - #205 - I've observed some inconsistencies in handling TCP vs WebSocket connection timeouts. I believe that we can have another pass and share even more code between them: - #70 --------- Signed-off-by: Alexandru Vasile <[email protected]> Co-authored-by: Dmitry Markin <[email protected]>
lexnv
added a commit
that referenced
this issue
Sep 5, 2024
… reasons (#227) This PR makes several changes to the errors in general, with the main goal of extracting the reject-reason from the request response protocols: - an `ImmediateDial` error is introduced for failing to dial peers due to internal errors (no addresses available, tried to dial self, already connected etc) to distinguish between network dial failures - opening substreams now return a `SubstreamError` instead of using the overarching litep2p::Error - substreams are now implementing `Stream<Error = SubstreamError>` for consistency with opening substreams - Reject reasons include immediate dialing errors. Ideally, we could expose the same level of information that is exposed via `ListDialErrors` (provide a tuple of addresses with individual dial errors), however that would require a bigger refactor to the code. For now this information is enough for Substrate metrics to provide more information and align with litep2p metrics. This PR is part of a bigger effort to simply the overarching error enum: - #204 Closes: #188 --------- Signed-off-by: Alexandru Vasile <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The litep2p::Error contains multiple variants that grow over time.
Clean up the error into something more coherent and group errors by individual components / use-case, similar to the
DialError.This will help us provide more descriptive error messages and identify easily the place of occurrence:
litep2p/src/error.rs
Line 43 in c9e7aff
The text was updated successfully, but these errors were encountered: