Fixed some error messages/warnings during index download #2257

cmaglie · 2023-08-02T15:40:37Z

Please check if the PR fulfills these requirements

See how to contribute

The PR has no duplicates (please search among the Pull Requests
before creating one)
The PR follows
our contributing guidelines
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)
UPGRADING.md has been updated with a migration guide (for breaking changes)
configuration.schema.json updated if new parameters are added.

What kind of change does this PR introduce?

Some error messages are adjusted to better represent the actual error.

During index download if the server is not available.
During index loading if the URL is wrong.

What is the current behavior?

Following the steps in #2254 I get:

$ arduino-cli core update-index --format json
{
  "error": "Some indexes could not be updated.",
  "warnings": [
    "Error initializing instance: Loading index file: loading json index file /home/megabug/.arduino15: read /home/megabug/.arduino15: is a directory"
  ]
}

What is the new behavior?

$ arduino-cli core update-index --format json
{
  "error": "Some indexes could not be updated.",
  "warnings": [
    "Error initializing instance: Error downloading index 'https://invalidurl': Invalid URL",
    "Error initializing instance: Loading index file: Invalid URL: https://invalidurl"
  ]
}

Does this PR introduce a breaking change, and is titled accordingly?

No

Other information

Fix #2254

…rrors

codecov · 2023-08-02T15:55:38Z

Codecov Report

Patch coverage: 48.14% and project coverage change: -0.04% ⚠️

Comparison is base (df12786) 62.97% compared to head (b39c45b) 62.93%.
Report is 7 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2257      +/-   ##
==========================================
- Coverage   62.97%   62.93%   -0.04%     
==========================================
  Files         220      220              
  Lines       19501    19521      +20     
==========================================
+ Hits        12280    12286       +6     
- Misses       6137     6147      +10     
- Partials     1084     1088       +4

Flag	Coverage Δ
unit	`62.93% <48.14%> (-0.04%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
arduino/cores/packagemanager/package_manager.go	`76.61% <0.00%> (-0.44%)`	⬇️
arduino/resources/index.go	`46.29% <33.33%> (-2.73%)`	⬇️
commands/instances.go	`64.43% <64.28%> (-0.27%)`	⬇️
internal/cli/monitor/monitor.go	`13.07% <100.00%> (ø)`

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kittaakos

✅ I get the error message if the invalid 3rd party URL is https://invalidUrl. Thank you! 👍

% ./arduino-cli version
arduino-cli  Version: git-snapshot Commit: b39c45b9 Date: 2023-08-02T17:26:45Z
% cat ~/.arduinoIDE/arduino-cli.yaml | yq .directories
{
  "builtin": {
    "libraries": "/Users/a.kitta/Library/Arduino15/libraries"
  },
  "data": "/Users/a.kitta/Library/Arduino15",
  "downloads": "/Users/a.kitta/Library/Arduino15/staging",
  "user": "/Users/a.kitta/Documents/Arduino"
}
% cat ~/.arduinoIDE/arduino-cli.yaml | yq .board_manager.additional_urls
[
  "https://invalidUrl"
]
% ls ~/Library/Arduino15
inventory.yaml
% ./arduino-cli core update-index --config-file ~/.arduinoIDE/arduino-cli.yaml --format json
{
  "error": "Some indexes could not be updated.",
  "warnings": [
    "Error initializing instance: Some indexes could not be updated.",
    "Error initializing instance: Loading index file: Invalid URL: https://invalidUrl"
  ]
}

kittaakos · 2023-08-02T17:48:01Z

During the review, I noticed something with the URL parsing. Is this a bug?

When I changed the 3rd party URL to https://http://file://notafile, I would have expected a different error message. Something like an invalid URL, but I got:

"Error initializing instance: Loading index file: loading json index file /Users/a.kitta/Library/Arduino15/notafile: open /Users/a.kitta/Library/Arduino15/notafile: no such file or directory"

% cat ~/.arduinoIDE/arduino-cli.yaml | yq .board_manager.additional_urls
[
  "https://http://file://notafile"
]
% ./arduino-cli core update-index --config-file ~/.arduinoIDE/arduino-cli.yaml --format json
{
  "error": "Some indexes could not be updated.",
  "warnings": [
    "Error initializing instance: Some indexes could not be updated.",
    "Error initializing instance: Loading index file: loading json index file /Users/a.kitta/Library/Arduino15/notafile: open /Users/a.kitta/Library/Arduino15/notafile: no such file or directory"
  ]
}

I thought it was because the last URL scheme was picked up; it was file, and the CLI tried to load a file. I changed the URL to https://http://ssfile://notafile, then I got the same error: Error initializing instance: Loading index file: loading json index file /Users/a.kitta/Library/Arduino15/notafile: open /Users/a.kitta/Library/Arduino15/notafile: no such file or directory. Changing the URL to https://http://ssfile://notafile or https://http://ssfilex://notafile produced the same error message. How does the URL parsing work? If I write file anywhere in the URL, does the CLI try to locate a file in the data folder? Thanks!

cmaglie · 2023-08-03T08:36:04Z

When I changed the 3rd party URL to https://http://file://notafile, I would have expected a different error message. Something like an invalid URL, but I got:

https://http://file://notafile it's a bit convoluted but it's still a valid URL, it breaks down as:

Scheme: https
Host: http: (hostname http with implicit port)
Path: //file://notafile (equivalent to /file:/notafile you can actually use // as a single / anywhere in a path...)

https://go.dev/play/p/barpaG3S9Q0

so basically the CLI is trying to download notafile as an index, and also it tries to load a (possibly) already downloaded index called notafile.

Maybe we should enforce the .json extension or at least the package_[*_]index.* name format?

cmaglie · 2023-08-03T13:03:13Z

Maybe we should enforce the .json extension or at least the package_[_]index. name format?

while we decide this, I'll move forward this one.

cmaglie added 2 commits August 2, 2023 17:34

Added FAILED_INSTANCE_INIT_REASON_INDEX_DOWNLOAD_ERROR in gRPC Init e…

e6ddb09

…rrors

Improved error reporting during Init and first-index-update

b39c45b

cmaglie added type: imperfection Perceived defect in any part of project topic: CLI Related to the command line interface topic: gRPC Related to the gRPC interface labels Aug 2, 2023

cmaglie self-assigned this Aug 2, 2023

cmaglie requested review from kittaakos, alessio-perugini, umbynos and MatteoPologruto August 2, 2023 15:41

umbynos approved these changes Aug 2, 2023

View reviewed changes

kittaakos approved these changes Aug 2, 2023

View reviewed changes

alessio-perugini approved these changes Aug 3, 2023

View reviewed changes

cmaglie merged commit f46cf1e into arduino:master Aug 3, 2023

cmaglie deleted the fix_error_message branch August 3, 2023 13:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed some error messages/warnings during index download #2257

Fixed some error messages/warnings during index download #2257

cmaglie commented Aug 2, 2023

codecov bot commented Aug 2, 2023

kittaakos left a comment

kittaakos commented Aug 2, 2023

cmaglie commented Aug 3, 2023

cmaglie commented Aug 3, 2023

Fixed some error messages/warnings during index download #2257

Fixed some error messages/warnings during index download #2257

Conversation

cmaglie commented Aug 2, 2023

Please check if the PR fulfills these requirements

What kind of change does this PR introduce?

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change, and is titled accordingly?

Other information

codecov bot commented Aug 2, 2023

Codecov Report

kittaakos left a comment

Choose a reason for hiding this comment

kittaakos commented Aug 2, 2023

cmaglie commented Aug 3, 2023

cmaglie commented Aug 3, 2023