From 2c866277a293f778f54290d7ddc246203f8eef54 Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Thu, 16 May 2019 23:11:46 +0200 Subject: [PATCH] lib/api, lib/connections, gui: Show connection error for disconnected devices (fixes #3345) (#5727) * lib/api, lib/connections, gui: Show connection error for disconnected devices (fixes #3345) This adds functionality in the connetions service to track the last error per address. That is in turn exposed in the /rest/system/status API method, as that is also where we already show the listener status from the connection service. The GUI uses this info where it lists addresses, showing errors (if any) in red underneath each address. I also slightly refactored the existing status method on the connection service to have a better name and return typed information. * ok * review * formatting * review --- gui/default/index.html | 10 ++- .../syncthing/core/syncthingController.js | 10 +++ lib/api/api.go | 3 +- lib/api/mocked_connections_test.go | 10 ++- lib/connections/service.go | 69 ++++++++++++++++--- lib/connections/structs.go | 1 + 6 files changed, 88 insertions(+), 15 deletions(-) diff --git a/gui/default/index.html b/gui/default/index.html index 1868052cc..bca5b8c7d 100644 --- a/gui/default/index.html +++ b/gui/default/index.html @@ -715,8 +715,14 @@ - {{addr}}
 - {{addr}}
 + + {{addr}}
+ {{abbreviatedError(addr)}}
 + + + {{addr}}
+ {{abbreviatedError(addr)}}
 + diff --git a/gui/default/syncthing/core/syncthingController.js b/gui/default/syncthing/core/syncthingController.js index b47f17772..44ea33a7a 100755 --- a/gui/default/syncthing/core/syncthingController.js +++ b/gui/default/syncthing/core/syncthingController.js @@ -2406,4 +2406,14 @@ angular.module('syncthing.core') $scope.saveConfig(); } }; + + $scope.abbreviatedError = function (addr) { + var status = $scope.system.lastDialStatus[addr]; + if (!status || !status.error) { + return null; + } + var time = $filter('date')(status.when, "HH:mm:ss") + var err = status.error.replace(/.+: /, ''); + return err + " (" + time + ")"; + } }); diff --git a/lib/api/api.go b/lib/api/api.go index 5146f09a6..eb4c415e2 100644 --- a/lib/api/api.go +++ b/lib/api/api.go @@ -898,7 +898,8 @@ func (s *service) getSystemStatus(w http.ResponseWriter, r *http.Request) { res["discoveryErrors"] = discoErrors } - res["connectionServiceStatus"] = s.connectionsService.Status() + res["connectionServiceStatus"] = s.connectionsService.ListenerStatus() + res["lastDialStatus"] = s.connectionsService.ConnectionStatus() // cpuUsage.Rate() is in milliseconds per second, so dividing by ten // gives us percent res["cpuPercent"] = s.cpu.Rate() / 10 / float64(runtime.NumCPU()) diff --git a/lib/api/mocked_connections_test.go b/lib/api/mocked_connections_test.go index 54aee73f8..608a12640 100644 --- a/lib/api/mocked_connections_test.go +++ b/lib/api/mocked_connections_test.go @@ -6,9 +6,17 @@ package api +import ( + "github.com/syncthing/syncthing/lib/connections" +) + type mockedConnections struct{} -func (m *mockedConnections) Status() map[string]interface{} { +func (m *mockedConnections) ListenerStatus() map[string]connections.ListenerStatusEntry { + return nil +} + +func (m *mockedConnections) ConnectionStatus() map[string]connections.ConnectionStatusEntry { return nil } diff --git a/lib/connections/service.go b/lib/connections/service.go index f0a49c226..95547f2e2 100644 --- a/lib/connections/service.go +++ b/lib/connections/service.go @@ -90,10 +90,22 @@ var tlsVersionNames = map[uint16]string{ // dialers. Successful connections are handed to the model. type Service interface { suture.Service - Status() map[string]interface{} + ListenerStatus() map[string]ListenerStatusEntry + ConnectionStatus() map[string]ConnectionStatusEntry NATType() string } +type ListenerStatusEntry struct { + Error *string `json:"error"` + LANAddresses []string `json:"lanAddresses"` + WANAddresses []string `json:"wanAddresses"` +} + +type ConnectionStatusEntry struct { + When time.Time `json:"when"` + Error *string `json:"error"` +} + type service struct { *suture.Supervisor cfg config.Wrapper @@ -112,6 +124,9 @@ type service struct { listeners map[string]genericListener listenerTokens map[string]suture.ServiceToken listenerSupervisor *suture.Supervisor + + connectionStatusMut sync.RWMutex + connectionStatus map[string]ConnectionStatusEntry // address -> latest error/status } func NewService(cfg config.Wrapper, myID protocol.DeviceID, mdl Model, tlsCfg *tls.Config, discoverer discover.Finder, @@ -151,6 +166,9 @@ func NewService(cfg config.Wrapper, myID protocol.DeviceID, mdl Model, tlsCfg *t FailureBackoff: 600 * time.Second, PassThroughPanics: true, }), + + connectionStatusMut: sync.NewRWMutex(), + connectionStatus: make(map[string]ConnectionStatusEntry), } cfg.Subscribe(service) @@ -378,18 +396,23 @@ func (s *service) connect() { uri, err := url.Parse(addr) if err != nil { + s.setConnectionStatus(addr, err) l.Infof("Parsing dialer address %s: %v", addr, err) continue } if len(deviceCfg.AllowedNetworks) > 0 { if !IsAllowedNetwork(uri.Host, deviceCfg.AllowedNetworks) { + s.setConnectionStatus(addr, errors.New("network disallowed")) l.Debugln("Network for", uri, "is disallowed") continue } } dialerFactory, err := getDialerFactory(cfg, uri) + if err != nil { + s.setConnectionStatus(addr, err) + } switch err { case nil: // all good @@ -424,6 +447,7 @@ func (s *service) connect() { } dialTargets = append(dialTargets, dialTarget{ + addr: addr, dialer: dialer, priority: priority, deviceID: deviceID, @@ -431,7 +455,7 @@ func (s *service) connect() { }) } - conn, ok := dialParallel(deviceCfg.DeviceID, dialTargets) + conn, ok := s.dialParallel(deviceCfg.DeviceID, dialTargets) if ok { s.conns <- conn } @@ -633,19 +657,19 @@ func (s *service) ExternalAddresses() []string { return util.UniqueStrings(addrs) } -func (s *service) Status() map[string]interface{} { +func (s *service) ListenerStatus() map[string]ListenerStatusEntry { + result := make(map[string]ListenerStatusEntry) s.listenersMut.RLock() - result := make(map[string]interface{}) for addr, listener := range s.listeners { - status := make(map[string]interface{}) + var status ListenerStatusEntry - err := listener.Error() - if err != nil { - status["error"] = err.Error() + if err := listener.Error(); err != nil { + errStr := err.Error() + status.Error = &errStr } - status["lanAddresses"] = urlsToStrings(listener.LANAddresses()) - status["wanAddresses"] = urlsToStrings(listener.WANAddresses()) + status.LANAddresses = urlsToStrings(listener.LANAddresses()) + status.WANAddresses = urlsToStrings(listener.WANAddresses()) result[addr] = status } @@ -653,6 +677,28 @@ func (s *service) Status() map[string]interface{} { return result } +func (s *service) ConnectionStatus() map[string]ConnectionStatusEntry { + result := make(map[string]ConnectionStatusEntry) + s.connectionStatusMut.RLock() + for k, v := range s.connectionStatus { + result[k] = v + } + s.connectionStatusMut.RUnlock() + return result +} + +func (s *service) setConnectionStatus(address string, err error) { + status := ConnectionStatusEntry{When: time.Now().UTC().Truncate(time.Second)} + if err != nil { + errStr := err.Error() + status.Error = &errStr + } + + s.connectionStatusMut.Lock() + s.connectionStatus[address] = status + s.connectionStatusMut.Unlock() +} + func (s *service) NATType() string { s.listenersMut.RLock() defer s.listenersMut.RUnlock() @@ -769,7 +815,7 @@ func IsAllowedNetwork(host string, allowed []string) bool { return false } -func dialParallel(deviceID protocol.DeviceID, dialTargets []dialTarget) (internalConn, bool) { +func (s *service) dialParallel(deviceID protocol.DeviceID, dialTargets []dialTarget) (internalConn, bool) { // Group targets into buckets by priority dialTargetBuckets := make(map[int][]dialTarget, len(dialTargets)) for _, tgt := range dialTargets { @@ -793,6 +839,7 @@ func dialParallel(deviceID protocol.DeviceID, dialTargets []dialTarget) (interna wg.Add(1) go func(tgt dialTarget) { conn, err := tgt.Dial() + s.setConnectionStatus(tgt.addr, err) if err == nil { res <- conn } diff --git a/lib/connections/structs.go b/lib/connections/structs.go index dcd3002e6..cdb14c08c 100644 --- a/lib/connections/structs.go +++ b/lib/connections/structs.go @@ -195,6 +195,7 @@ func (o *onAddressesChangedNotifier) notifyAddressesChanged(l genericListener) { } type dialTarget struct { + addr string dialer genericDialer priority int uri *url.URL