-
-
Notifications
You must be signed in to change notification settings - Fork 422
Fixed some error messages/warnings during index download #2257
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov ReportPatch coverage:
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
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ 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月02日T17: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" ] }
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!
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?
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.
Please check if the PR fulfills these requirements
See how to contribute
before creating one)
our contributing guidelines
UPGRADING.mdhas been updated with a migration guide (for breaking changes)configuration.schema.jsonupdated if new parameters are added.What kind of change does this PR introduce?
Some error messages are adjusted to better represent the actual error.
What is the current behavior?
Following the steps in #2254 I get:
What is the new behavior?
Does this PR introduce a breaking change, and is titled accordingly?
No
Other information
Fix #2254