Discoveries are now closed and unregistered after failure

Author: silvanocerza

arduino/discovery/discovery.go

@@ -374,21 +374,12 @@ func (disc *PluggableDiscovery) Stop() error {
 }
 
 // Quit terminates the discovery. No more commands can be accepted by the discovery.
-func (disc *PluggableDiscovery) Quit() error {
-	if err := disc.sendCommand("QUIT\n"); err != nil {
-		return err
-	}
-	if msg, err := disc.waitMessage(time.Second * 10); err != nil {
-		return fmt.Errorf(tr("calling %[1]s: %[2]w"), "QUIT", err)
-	} else if msg.EventType != "quit" {
-		return errors.Errorf(tr("communication out of sync, expected '%[1]s', received '%[2]s'"), "quit", msg.EventType)
-	} else if msg.Error {
-		return errors.Errorf(tr("command failed: %s"), msg.Message)
-	} else if strings.ToUpper(msg.Message) != "OK" {
-		return errors.Errorf(tr("communication out of sync, expected '%[1]s', received '%[2]s'"), "OK", msg.Message)
+func (disc *PluggableDiscovery) Quit() {
+	_ = disc.sendCommand("QUIT\n")
+	if _, err := disc.waitMessage(time.Second * 5); err != nil {
+		logrus.Errorf("Quitting discovery %s: %s", disc.id, err)
 	}
 	disc.killProcess()
-	return nil
 }
 
 // List executes an enumeration of the ports and returns a list of the available

arduino/discovery/discovery_client/main.go

@@ -135,9 +135,7 @@ out:
 	}
 
 	for _, disc := range discoveries {
-		if err := disc.Quit(); err != nil {
-			log.Fatal("Error stopping discovery:", err)
-		}
+		disc.Quit()
 		fmt.Println("Discovery QUITed")
 		for disc.State() == discovery.Alive {
 			time.Sleep(time.Millisecond)

arduino/discovery/discoverymanager/discoverymanager.go

@@ -22,6 +22,7 @@ import (
 	"github.com/arduino/arduino-cli/arduino/discovery"
 	"github.com/arduino/arduino-cli/i18n"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
 // DiscoveryManager is required to handle multiple pluggable-discovery that
@@ -64,6 +65,14 @@ func (dm *DiscoveryManager) Add(disc *discovery.PluggableDiscovery) error {
 	return nil
 }
 
+// remove quits and deletes the discovery with specified id
+// from the discoveries managed by this DiscoveryManager
+func (dm *DiscoveryManager) remove(id string) {
+	dm.discoveries[id].Quit()
+	delete(dm.discoveries, id)
+	logrus.Infof("Closed and removed discovery %s", id)
+}
+
 // parallelize runs function f concurrently for each discovery.
 // Returns a list of errors returned by each call of f.
 func (dm *DiscoveryManager) parallelize(f func(d *discovery.PluggableDiscovery) error) []error {
@@ -103,6 +112,7 @@ func (dm *DiscoveryManager) RunAll() []error {
 		}
 
 		if err := d.Run(); err != nil {
+			dm.remove(d.GetID())
 			return fmt.Errorf(tr("discovery %[1]s process not started: %[2]w"), d.GetID(), err)
 		}
 		return nil
@@ -119,6 +129,7 @@ func (dm *DiscoveryManager) StartAll() []error {
 			return nil
 		}
 		if err := d.Start(); err != nil {
+			dm.remove(d.GetID())
 			return fmt.Errorf(tr("starting discovery %[1]s: %[2]w"), d.GetID(), err)
 		}
 		return nil
@@ -139,6 +150,7 @@ func (dm *DiscoveryManager) StartSyncAll() (<-chan *discovery.Event, []error) {
 
 		eventCh, err := d.StartSync(5)
 		if err != nil {
+			dm.remove(d.GetID())
 			return fmt.Errorf(tr("start syncing discovery %[1]s: %[2]w"), d.GetID(), err)
 		}
 
@@ -170,6 +182,7 @@ func (dm *DiscoveryManager) StopAll() []error {
 		}
 
 		if err := d.Stop(); err != nil {
+			dm.remove(d.GetID())
 			return fmt.Errorf(tr("stopping discovery %[1]s: %[2]w"), d.GetID(), err)
 		}
 		return nil
@@ -185,9 +198,7 @@ func (dm *DiscoveryManager) QuitAll() []error {
 			return nil
 		}
 
-		if err := d.Quit(); err != nil {
-			return fmt.Errorf(tr("quitting discovery %[1]s: %[2]w"), d.GetID(), err)
-		}
+		d.Quit()
 		return nil
 	})
 	return errs

commands/board/list.go

@@ -296,15 +296,14 @@ func Watch(instanceID int32, interrupt <-chan bool) (<-chan *rpc.BoardListWatchR
 					Error:     boardsError,
 				}
 			case <-interrupt:
-				errs := dm.StopAll()
-				if len(errs) > 0 {
+				for _, err := range dm.StopAll() {
+					// Discoveries that return errors have their process
+					// closed and are removed from the list of discoveries
+					// in the manager
 					outChan <- &rpc.BoardListWatchResponse{
 						EventType: "error",
-						Error:     tr("stopping discoveries: %s", errs),
+						Error:     tr("stopping discoveries: %s", err),
 					}
-					// Don't close the channel if quitting all discoveries
-					// failed, otherwise some processes might be left running.
-					continue
 				}
 				return
 			}