Improve network error handling #148
Conversation
diegomrsantos
changed the title
diegomrsantos
force-pushed
the
improve-network-error-handling
branch
from
b41027f to
3aa2b49
Compare
diegomrsantos
force-pushed
the
improve-network-error-handling
branch
from
3aa2b49 to
e33f28b
Compare
anchor/network/src/discovery.rs
Outdated
| pub enum DiscoveryError { | ||
| #[error("Failed to parse keypair into an ENR key: {0}")] | ||
| EnrKey(String), | ||
|
|
||
| #[error("Failed to build ENR: {0}")] | ||
| EnrBuild(String), | ||
|
|
||
| #[error("Discv5 initialization error: {0}")] | ||
| Discv5Init(String), | ||
|
|
||
| #[error("Discv5 start error: {0}")] | ||
| Discv5Start(String), | ||
| } |
There was a problem hiding this comment.
Do these all need to be String? Actual error types should be a bit clearer and should not affect the displayed message as the format string uses the Display impl of the inner type, which is the same thing as the to_string that you manually call below.
There was a problem hiding this comment.
Do you mean the original error? If so, that's the type of the original errors, like in https://github.com/sigp/lighthouse/blob/stable/beacon_node/lighthouse_network/src/discovery/enr_ext.rs#L281
There was a problem hiding this comment.
Good catch, thanks. I changed it for EnrBuild and I'll double-check the others.
There was a problem hiding this comment.
Hopefully I covered everything now
| bitfield.set(9, true).unwrap(); | ||
|
|
||
| builder.add_value::<Bytes>("subnets", &bitfield.as_ssz_bytes().into()); | ||
| builder.add_value::<Bytes>("subnets", &BitVector::<U128>::new().as_ssz_bytes().into()); |
There was a problem hiding this comment.
@dknopik This is about what we talked yesterday. Should we create an empty BitVector at this point?
There was a problem hiding this comment.
Yes, the subnets will be updates as the subnet tracker messages come in: 9e9e485
There was a problem hiding this comment.
as always, open for other suggestions though
There was a problem hiding this comment.
From what I understand, it makes sense.
diegomrsantos
force-pushed
the
improve-network-error-handling
branch
from
423fcbf to
5b3a009
Compare
diegomrsantos
force-pushed
the
improve-network-error-handling
branch
from
0d97c23 to
07455cc
Compare
Issue Addressed
closes #75
Proposed Changes
This PR aims to improve network error handling. The key technical changes proposed include:
thiserror.