More robust status command #38
Conversation
| @@ -27,7 +28,7 @@ type request struct { | |||
|
|
|||
| // MakeRequest attempts to send an API query to the Wiretap server. | |||
| func makeRequest(req request) ([]byte, error) { | |||
| client := &http.Client{} | |||
| client := &http.Client{Timeout: 3 * time.Second} | |||
There was a problem hiding this comment.
Yup, should've had this from the beginning. Thanks.
| if err != nil { | ||
| message := "failed to fetch node's configuration as peer" | ||
| log.Printf("%s: %v", message, err) | ||
| nodes["TIMEOUT"] = Node{ |
There was a problem hiding this comment.
This will be overridden by other failed nodes (and isn't necessarily caused by a timeout).
Just an idea, but what do you think about adding a new field in the Node struct so the errors can be saved along the corresponding nodes? This way we don't lose info and can potentially display it in the visualization down the road.
There was a problem hiding this comment.
Yeah this was just my quickest solution to get the rest of the connected network to display. It would definitely be preferable to actually save failure info specific to the node and display it nicely.
There was a problem hiding this comment.
Gotcha. Does this line need to exist at all then?
nodes["TIMEOUT"] = ...There was a problem hiding this comment.
Annoyingly, yes, Can't remember exactly where, but without adding some kind of placeholder entry here it was causing another crash somewhere else.
There was a problem hiding this comment.
I commented it out and it seems to work just fine.
The only place nodes is used is in the findChildren function and is always indexed by public key so it should have no effect.
src/cmd/status.go
Outdated
| @@ -111,8 +121,8 @@ func (c statusCmdConfig) Run() { | |||
| // Use node tree to build diagram tree. | |||
| t.AddChild(tree.NodeString(fmt.Sprintf(`client | |||
| relay: %v... | |||
| e2ee: %v... | |||
| relay: %v... | |||
There was a problem hiding this comment.
I believe the trailing whitespace here was intentional to give the boxes some padding. Did you mean to remove the spaces or is it something like a side effect of editor/linter?
There was a problem hiding this comment.
Not intentional, editor did it automatically.
There was a problem hiding this comment.
This is odd, file permissions on the changed files went from 644 → 755. From some quick searching this may be a side effect of using WSL if you didn't explicitly change them
There was a problem hiding this comment.
Yup, probably WSL being weird. I didn't mean to change that.
Before (one server setup but not connected): After: |
|
Added a second commit to undo the whitespace and file permission changes. |
|
I don't have write access to this repository anymore, but I approve! |
Fixes #37
Adds a 3s timeout when trying to get info from a server via the
statuscommand, and still shows data about the rest of the working network instead of immediately exiting.