Enhance error messages when updating indexes or local index files are missing

Author: silvanocerza

Diff for: cli/instance/instance.go

@@ -17,6 +17,8 @@ package instance
 
 import (
 	"context"
+	"fmt"
+	"strings"
 
 	"github.com/arduino/arduino-cli/cli/output"
 	"github.com/arduino/arduino-cli/commands"
@@ -126,8 +128,7 @@ func checkPlatformErrors(resp *rpc.InitResp) error {
 		for _, err := range resp.GetPlatformsIndexErrors() {
 			logrus.Errorf("Error loading platform index: %v", err)
 		}
-		// return
-		return errors.New("There were errors loading platform indexes")
+		return fmt.Errorf("error loading platform index: \n%v", strings.Join(resp.GetPlatformsIndexErrors(), "\n"))
 	}
 
 	return nil

Diff for: commands/download.go

@@ -16,6 +16,7 @@
 package commands
 
 import (
+	"errors"
 	"time"
 
 	"github.com/arduino/arduino-cli/httpclient"
@@ -60,6 +61,10 @@ func Download(d *downloader.Downloader, label string, downloadCB DownloadProgres
 	if d.Error() != nil {
 		return d.Error()
 	}
+	// The URL is not reachable for some reason
+	if d.Resp.StatusCode >= 400 && d.Resp.StatusCode <= 599 {
+		return errors.New(d.Resp.Status)
+	}
 	downloadCB(&rpc.DownloadProgress{Completed: true})
 	return nil
 }

Diff for: commands/instances.go

@@ -230,9 +230,9 @@ func UpdateIndex(ctx context.Context, req *rpc.UpdateIndexReq, downloadCB Downlo
 			return nil, fmt.Errorf("downloading index %s: %s", URL, err)
 		}
 		coreIndexPath := indexpath.Join(path.Base(URL.Path))
-		Download(d, "Updating index: "+coreIndexPath.Base(), downloadCB)
-		if d.Error() != nil {
-			return nil, fmt.Errorf("downloading index %s: %s", URL, d.Error())
+		err = Download(d, "Updating index: "+coreIndexPath.Base(), downloadCB)
+		if err != nil {
+			return nil, fmt.Errorf("downloading index %s: %s", URL, err)
 		}
 
 		// Check for signature

Diff for: go.mod

@@ -21,15 +21,12 @@ require (
 	github.com/h2non/filetype v1.0.8 // indirect
 	github.com/imjasonmiller/godice v0.1.2
 	github.com/juju/loggo v0.0.0-20190526231331-6e530bcce5d8 // indirect
-	github.com/kr/pretty v0.2.0 // indirect
 	github.com/kr/text v0.2.0 // indirect
 	github.com/leonelquinteros/gotext v1.4.0
-	github.com/marcinbor85/gohex v0.0.0-20200531163658-baab2527a9a2
 	github.com/mattn/go-colorable v0.1.2
 	github.com/mattn/go-isatty v0.0.8
 	github.com/mattn/go-runewidth v0.0.9 // indirect
 	github.com/miekg/dns v1.0.5 // indirect
-	github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e // indirect
 	github.com/oleksandr/bonjour v0.0.0-20160508152359-5dcf00d8b228 // indirect
 	github.com/pkg/errors v0.9.1
 	github.com/pmylund/sortutil v0.0.0-20120526081524-abeda66eb583
@@ -40,11 +37,10 @@ require (
 	github.com/sirupsen/logrus v1.4.2
 	github.com/spf13/cobra v1.0.1-0.20200710201246-675ae5f5a98c
 	github.com/spf13/jwalterweatherman v1.0.0
-	github.com/spf13/pflag v1.0.3
 	github.com/spf13/viper v1.6.2
 	github.com/stretchr/testify v1.6.1
 	go.bug.st/cleanup v1.0.0
-	go.bug.st/downloader/v2 v2.0.1
+	go.bug.st/downloader/v2 v2.1.0
 	go.bug.st/relaxed-semver v0.0.0-20190922224835-391e10178d18
 	go.bug.st/serial v1.1.1
 	go.bug.st/serial.v1 v0.0.0-20180827123349-5f7892a7bb45 // indirect
@@ -53,7 +49,6 @@ require (
 	golang.org/x/text v0.3.2
 	google.golang.org/grpc v1.27.0
 	google.golang.org/protobuf v1.25.0
-	gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect
 	gopkg.in/mgo.v2 v2.0.0-20180705113604-9856a29383ce // indirect
 	gopkg.in/src-d/go-git.v4 v4.13.1
 	gopkg.in/yaml.v2 v2.3.0

Diff for: go.sum

@@ -557,7 +557,10 @@ def test_compile_with_invalid_url(run_command, data_dir):
     # Create settings with custom invalid URL
     assert run_command("config init --dest-dir . --additional-urls https://example.com/package_example_index.json")
 
-    # Verifies compilation fails cause of invalid URL
+    # Verifies compilation fails cause of missing local index file
     res = run_command(f'compile -b {fqbn} "{sketch_path}"')
     assert res.failed
-    assert "There were errors loading platform indexes" in res.stderr
+    lines = [l.strip() for l in res.stderr.splitlines()]
+    assert "Error creating instance: error loading platform index:" in lines
+    expected_index_file = Path(data_dir, "package_example_index.json")
+    assert f"loading json index file {expected_index_file}: " + f"open {expected_index_file}:" in lines[-1]

Diff for: test/test_compile.py

@@ -174,10 +174,30 @@ def test_core_search_no_args(run_command, httpserver):
     assert len(platforms) == num_platforms
 
 
-def test_core_updateindex_invalid_url(run_command):
-    url = "http://www.invalid-domain-asjkdakdhadjkh.com/package_example_index.json"
-    result = run_command("core update-index --additional-urls={}".format(url))
+def test_core_updateindex_url_not_found(run_command, httpserver):
+    assert run_command("core update-index")
+
+    # Brings up a local server to fake a failure
+    httpserver.expect_request("/test_index.json").respond_with_data(status=404)
+    url = httpserver.url_for("/test_index.json")
+
+    result = run_command(f"core update-index --additional-urls={url}")
+    assert result.failed
+    lines = [l.strip() for l in result.stderr.splitlines()]
+    assert f"Error updating index: downloading index {url}: 404 NOT FOUND" in lines
+
+
+def test_core_updateindex_internal_server_error(run_command, httpserver):
+    assert run_command("core update-index")
+
+    # Brings up a local server to fake a failure
+    httpserver.expect_request("/test_index.json").respond_with_data(status=500)
+    url = httpserver.url_for("/test_index.json")
+
+    result = run_command(f"core update-index --additional-urls={url}")
     assert result.failed
+    lines = [l.strip() for l in result.stderr.splitlines()]
+    assert f"Error updating index: downloading index {url}: 500 INTERNAL SERVER ERROR" in lines
 
 
 def test_core_install_without_updateindex(run_command):

Diff for: test/test_core.py

@@ -47,3 +47,29 @@ def test_update_showing_outdated(run_command):
     assert "Updating index: library_index.json downloaded" in lines
     assert lines[-5].startswith("Arduino AVR Boards")
     assert lines[-2].startswith("USBHost")
+
+
+def test_update_with_url_not_found(run_command, httpserver):
+    assert run_command("update")
+
+    # Brings up a local server to fake a failure
+    httpserver.expect_request("/test_index.json").respond_with_data(status=404)
+    url = httpserver.url_for("/test_index.json")
+
+    res = run_command(f"update --additional-urls={url}")
+    assert res.failed
+    lines = [l.strip() for l in res.stderr.splitlines()]
+    assert f"Error updating core and libraries index: downloading index {url}: 404 NOT FOUND" in lines
+
+
+def test_update_with_url_internal_server_error(run_command, httpserver):
+    assert run_command("update")
+
+    # Brings up a local server to fake a failure
+    httpserver.expect_request("/test_index.json").respond_with_data(status=500)
+    url = httpserver.url_for("/test_index.json")
+
+    res = run_command(f"update --additional-urls={url}")
+    assert res.failed
+    lines = [l.strip() for l in res.stderr.splitlines()]
+    assert f"Error updating core and libraries index: downloading index {url}: 500 INTERNAL SERVER ERROR" in lines