Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Reset peers.json if the content is not loadable #405

Merged
merged 3 commits into from
Jul 27, 2018
Merged

Reset peers.json if the content is not loadable #405

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a805bcc
Reset peers.json if the content is not loadable
chevdor Jul 23, 2018
2080885
Merge branch 'master' into will-fix-404
gavofyork Jul 24, 2018
3dc3838
Whitespace
gavofyork Jul 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 27 additions & 8 deletions substrate/network-libp2p/src/network_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use std::fs;
use std::io::{Error as IoError, ErrorKind as IoErrorKind, Read, Write};
use std::path::Path;
use std::sync::atomic;
use std::{thread, time};
use std::time::{Duration, Instant};

// File where the peers are stored.
Expand Down Expand Up @@ -175,17 +176,35 @@ impl NetworkState {
let node_store = if let Some(ref path) = config.net_config_path {
let path = Path::new(path).join(NODES_FILE);
if let Ok(node_store) = JsonPeerstore::new(path.clone()) {
debug!(target: "sub-libp2p", "Initialized peer store for JSON \
file {:?}", path);
debug!(target: "sub-libp2p", "Initialized peer store for JSON file {:?}", path);
NodeStore::Json(node_store)
} else {
warn!(target: "sub-libp2p", "Failed to open peer storage {:?} \
; peers won't be saved", path);
NodeStore::Memory(MemoryPeerstore::empty())
warn!(target: "sub-libp2p", "Failed to open peer storage {:?}; peers file will be reset", path);
fs::remove_file(&path).expect("Failed deleting peers.json");

// we check for about 1s if the file was really deleted and move on
for _x in 0..200 {
if !Path::new(&path).exists() {
break;
} else {
debug!("Waiting for effective deletion of invalid/outdate peers.json");
thread::sleep(time::Duration::from_millis(5));
}
}

if let Ok(peerstore) = JsonPeerstore::new(path.clone()) {
debug!("peers.json reset");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this also happens if the file doesn't exist in the first place, ie. if the user starts polkadot for the first time, so the message is not totally correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the file does not exist in the first place, the first test L179 will kick in and the message you mention L196 will never show up.

NodeStore::Json(peerstore)
} else {
warn!(target: "sub-libp2p",
"Failed to reset peer storage {:?}; peers change will not be saved",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can also happen if the user doesn't have the permission to open the file, so the message should just be Failed to open peer storage IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would not the message L182 already say that?
I can change the message, no problem. We would then have 2 identical messages. Would you prefer that?

If the file cannot be open because of permissions, this should be caught by the first tests. The message Failed to reset peer storage really comes second, only after we tried deleting and recreating the file.

path
);
NodeStore::Memory(MemoryPeerstore::empty())
}
}
} else {
debug!(target: "sub-libp2p", "No peers file configured ; peers \
won't be saved");
debug!(target: "sub-libp2p", "No peers file configured ; peers won't be saved");
NodeStore::Memory(MemoryPeerstore::empty())
};

Expand Down Expand Up @@ -554,7 +573,7 @@ impl NetworkState {
/// You must pass an `UnboundedSender` which will be used by the `send`
/// method. Actually sending the data is not covered by this code.
///
/// The various methods of the `NetworkState` that close a connection do
/// The various methods of the `NetworkState` that close a connection do
/// so by dropping this sender.
pub fn custom_proto(
&self,
Expand Down