settings: add network.connection_timeout and network.cloud_api.skip_board_detection_calls (#2770)

* settings: add `connection_timeout`

* add test

* add docs

* Added gRPC flag to ignore Cloud API detection in BoardList* operations

* Set default duration to 60 seconds

* Updated JSON schema for configuration

* Added option to disable call to Arduino Cloud API for board detection

* Adjusted unit-tests

---------

Co-authored-by: Cristian Maglie <[email protected]>

Author: alessio-perugini

Diff for: commands/service_board_list.go

@@ -139,7 +139,7 @@ func identifyViaCloudAPI(props *properties.Map, settings *configuration.Settings
 }
 
 // identify returns a list of boards checking first the installed platforms or the Cloud API
-func identify(pme *packagemanager.Explorer, port *discovery.Port, settings *configuration.Settings) ([]*rpc.BoardListItem, error) {
+func identify(pme *packagemanager.Explorer, port *discovery.Port, settings *configuration.Settings, skipCloudAPI bool) ([]*rpc.BoardListItem, error) {
 	boards := []*rpc.BoardListItem{}
 	if port.Properties == nil {
 		return boards, nil
@@ -170,7 +170,7 @@ func identify(pme *packagemanager.Explorer, port *discovery.Port, settings *conf
 
 	// if installed cores didn't recognize the board, try querying
 	// the builder API if the board is a USB device port
-	if len(boards) == 0 {
+	if len(boards) == 0 && !skipCloudAPI && !settings.SkipCloudApiForBoardDetection() {
 		items, err := identifyViaCloudAPI(port.Properties, settings)
 		if err != nil {
 			// this is bad, but keep going
@@ -225,7 +225,7 @@ func (s *arduinoCoreServerImpl) BoardList(ctx context.Context, req *rpc.BoardLis
 
 	ports := []*rpc.DetectedPort{}
 	for _, port := range dm.List() {
-		boards, err := identify(pme, port, s.settings)
+		boards, err := identify(pme, port, s.settings, req.GetSkipCloudApiForBoardDetection())
 		if err != nil {
 			warnings = append(warnings, err.Error())
 		}
@@ -298,7 +298,7 @@ func (s *arduinoCoreServerImpl) BoardListWatch(req *rpc.BoardListWatchRequest, s
 
 			boardsError := ""
 			if event.Type == "add" {
-				boards, err := identify(pme, event.Port, s.settings)
+				boards, err := identify(pme, event.Port, s.settings, req.GetSkipCloudApiForBoardDetection())
 				if err != nil {
 					boardsError = err.Error()
 				}

Diff for: commands/service_board_list_test.go

@@ -157,7 +157,7 @@ func TestBoardIdentifySorting(t *testing.T) {
 	defer release()
 
 	settings := configuration.NewSettings()
-	res, err := identify(pme, &discovery.Port{Properties: idPrefs}, settings)
+	res, err := identify(pme, &discovery.Port{Properties: idPrefs}, settings, true)
 	require.NoError(t, err)
 	require.NotNil(t, res)
 	require.Len(t, res, 4)

Diff for: commands/service_settings_test.go

@@ -224,12 +224,73 @@ func TestDelete(t *testing.T) {
 	srv := NewArduinoCoreServer()
 	loadConfig(t, srv, paths.New("testdata", "arduino-cli.yml"))
 
-	_, err := srv.SettingsGetValue(context.Background(), &rpc.SettingsGetValueRequest{Key: "network"})
+	// Check loaded config
+	resp, err := srv.ConfigurationSave(context.Background(), &rpc.ConfigurationSaveRequest{
+		SettingsFormat: "yaml",
+	})
+	require.NoError(t, err)
+	require.YAMLEq(t, `
+board_manager:
+  additional_urls:
+    - http://foobar.com
+    - http://example.com
+
+daemon:
+  port: "50051"
+
+directories:
+  data: /home/massi/.arduino15
+  downloads: /home/massi/.arduino15/staging
+
+logging:
+  file: ""
+  format: text
+  level: info
+
+network:
+  proxy: "123"
+`, resp.GetEncodedSettings())
+
+	// Check default and setted values
+	res, err := srv.SettingsGetValue(context.Background(), &rpc.SettingsGetValueRequest{Key: "network"})
+	require.NoError(t, err)
+	require.Equal(t, `{"proxy":"123"}`, res.GetEncodedValue())
+	// Maybe should be like this?
+	// require.Equal(t, `{"proxy":"123","connection_timeout":"1m0s"}`, res.GetEncodedValue())
+	res, err = srv.SettingsGetValue(context.Background(), &rpc.SettingsGetValueRequest{Key: "network.connection_timeout"})
+	require.Equal(t, `"1m0s"`, res.GetEncodedValue()) // default value
 	require.NoError(t, err)
 
+	// Run deletion
 	_, err = srv.SettingsSetValue(context.Background(), &rpc.SettingsSetValueRequest{Key: "network", EncodedValue: ""})
 	require.NoError(t, err)
+	resp, err = srv.ConfigurationSave(context.Background(), &rpc.ConfigurationSaveRequest{
+		SettingsFormat: "yaml",
+	})
+	require.NoError(t, err)
+	require.YAMLEq(t, `
+board_manager:
+  additional_urls:
+    - http://foobar.com
+    - http://example.com
 
-	_, err = srv.SettingsGetValue(context.Background(), &rpc.SettingsGetValueRequest{Key: "network"})
-	require.Error(t, err)
+daemon:
+  port: "50051"
+
+directories:
+  data: /home/massi/.arduino15
+  downloads: /home/massi/.arduino15/staging
+
+logging:
+  file: ""
+  format: text
+  level: info
+`, resp.GetEncodedSettings())
+	// Check default and setted values
+	res, err = srv.SettingsGetValue(context.Background(), &rpc.SettingsGetValueRequest{Key: "network"})
+	require.NoError(t, err)
+	require.Equal(t, `{"connection_timeout":"1m0s"}`, res.GetEncodedValue())
+	res, err = srv.SettingsGetValue(context.Background(), &rpc.SettingsGetValueRequest{Key: "network.connection_timeout"})
+	require.Equal(t, `"1m0s"`, res.GetEncodedValue()) // default value
+	require.NoError(t, err)
 }

Diff for: docs/configuration.md

@@ -45,6 +45,11 @@
     [time.ParseDuration()](https://pkg.go.dev/time#ParseDuration), defaults to `720h` (30 days).
 - `network` - configuration options related to the network connection.
   - `proxy` - URL of the proxy server.
+  - `connection_timeout` - network connection timeout, the value format must be a valid input for
+    [time.ParseDuration()](https://pkg.go.dev/time#ParseDuration), defaults to `60s` (60 seconds). `0` means it will
+    wait indefinitely.
+  - `cloud_api.skip_board_detection_calls` - if set to `true` it will make the Arduino CLI skip network calls to Arduino
+    Cloud API to help detection of an unknown board.
 
 ### Default directories
 

Diff for: internal/cli/configuration/configuration.schema.json

@@ -38,16 +38,8 @@
         },
         "ttl": {
           "description": "cache expiration time of build folders. If the cache is hit by a compilation the corresponding build files lifetime is renewed. The value format must be a valid input for time.ParseDuration(), defaults to `720h` (30 days)",
-          "oneOf": [
-            {
-              "type": "integer",
-              "minimum": 0
-            },
-            {
-              "type": "string",
-              "pattern": "^\\+?([0-9]?\\.?[0-9]+(([nuµm]?s)|m|h))+$"
-            }
-          ]
+          "type": "string",
+          "pattern": "^[+-]?(([0-9]+(\\.[0-9]*)?|(\\.[0-9]+))(ns|us|µs|μs|ms|s|m|h))+$"
         }
       },
       "type": "object"
@@ -146,6 +138,35 @@
       },
       "type": "object"
     },
+    "network": {
+      "description": "settings related to network connections.",
+      "type": "object",
+      "properties": {
+        "proxy": {
+          "description": "proxy settings for network connections.",
+          "type": "string"
+        },
+        "user_agent_ext": {
+          "description": "extra string to append to the user agent string in HTTP requests.",
+          "type": "string"
+        },
+        "connection_timeout": {
+          "description": "timeout for network connections, defaults to '30s'",
+          "type": "string",
+          "pattern": "^[+-]?(([0-9]+(\\.[0-9]*)?|(\\.[0-9]+))(ns|us|µs|μs|ms|s|m|h))+$"
+        },
+        "cloud_api": {
+          "description": "settings related to the Arduino Cloud API.",
+          "type": "object",
+          "properties": {
+            "skip_board_detection_calls": {
+              "description": "do not call the Arduino Cloud API to detect an unknown board",
+              "type": "boolean"
+            }
+          }
+        }
+      }
+    },
     "output": {
       "description": "settings related to text output.",
       "properties": {

Diff for: internal/cli/configuration/defaults.go

@@ -71,6 +71,9 @@ func SetDefaults(settings *Settings) {
 	// network settings
 	setKeyTypeSchema("network.proxy", "")
 	setKeyTypeSchema("network.user_agent_ext", "")
+	setDefaultValueAndKeyTypeSchema("network.connection_timeout", (time.Second * 60).String())
+	// network: Arduino Cloud API settings
+	setKeyTypeSchema("network.cloud_api.skip_board_detection_calls", false)
 
 	// locale
 	setKeyTypeSchema("locale", "")

Diff for: internal/cli/configuration/network.go

@@ -22,6 +22,7 @@ import (
 	"net/url"
 	"os"
 	"runtime"
+	"time"
 
 	"github.com/arduino/arduino-cli/commands/cmderrors"
 	"github.com/arduino/arduino-cli/internal/i18n"
@@ -58,6 +59,19 @@ func (settings *Settings) ExtraUserAgent() string {
 	return settings.GetString("network.user_agent_ext")
 }
 
+// ConnectionTimeout returns the network connection timeout
+func (settings *Settings) ConnectionTimeout() time.Duration {
+	if timeout, ok, _ := settings.GetDurationOk("network.connection_timeout"); ok {
+		return timeout
+	}
+	return settings.Defaults.GetDuration("network.connection_timeout")
+}
+
+// SkipCloudApiForBoardDetection returns whether the cloud API should be ignored for board detection
+func (settings *Settings) SkipCloudApiForBoardDetection() bool {
+	return settings.GetBool("network.cloud_api.skip_board_detection_calls")
+}
+
 // NetworkProxy returns the proxy configuration (mainly used by HTTP clients)
 func (settings *Settings) NetworkProxy() (*url.URL, error) {
 	if proxyConfig, ok, _ := settings.GetStringOk("network.proxy"); !ok {
@@ -82,6 +96,7 @@ func (settings *Settings) NewHttpClient() (*http.Client, error) {
 			},
 			userAgent: settings.UserAgent(),
 		},
+		Timeout: settings.ConnectionTimeout(),
 	}, nil
 }
 

Diff for: internal/cli/configuration/network_test.go

@@ -21,6 +21,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"testing"
+	"time"
 
 	"github.com/arduino/arduino-cli/internal/cli/configuration"
 	"github.com/stretchr/testify/require"
@@ -68,3 +69,41 @@ func TestProxy(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, http.StatusNoContent, response.StatusCode)
 }
+
+func TestConnectionTimeout(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		time.Sleep(5 * time.Second)
+		w.WriteHeader(http.StatusNoContent)
+	}))
+	defer ts.Close()
+
+	doRequest := func(timeout int) (*http.Response, time.Duration, error) {
+		settings := configuration.NewSettings()
+		require.NoError(t, settings.Set("network.proxy", ts.URL))
+		if timeout != 0 {
+			require.NoError(t, settings.Set("network.connection_timeout", "2s"))
+		}
+		client, err := settings.NewHttpClient()
+		require.NoError(t, err)
+
+		request, err := http.NewRequest("GET", "http://arduino.cc", nil)
+		require.NoError(t, err)
+
+		start := time.Now()
+		resp, err := client.Do(request)
+		duration := time.Since(start)
+
+		return resp, duration, err
+	}
+
+	// Using default timeout (0), will wait indefinitely (in our case up to 5s)
+	response, elapsed, err := doRequest(0)
+	require.NoError(t, err)
+	require.Equal(t, http.StatusNoContent, response.StatusCode)
+	require.True(t, elapsed.Seconds() >= 4 && elapsed.Seconds() <= 6) // Adding some tolerance just in case...
+
+	// Setting a timeout of 1 seconds, will drop the connection after 1s
+	_, elapsed, err = doRequest(1)
+	require.Error(t, err)
+	require.True(t, elapsed.Seconds() >= 0.5 && elapsed.Seconds() <= 3) // Adding some tolerance just in case...
+}