feat: specialize init errors

[Bikappa]

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)

What kind of change does this PR introduce?

Makes some errors easier to recognize, with no text parsing, on the client side.

What is the current behavior?

Most of the errors follow the standard grpc error model which is limited around the structure of data it carries.

What is the new behavior?

More errors in the instance initialization logic now have a richer standard that allows them to be easily machine-recognizable on the client side

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

Other information

Fixes #1762 (at least in its original demand, a following initiative should be pursued to make all errors following a richer standard)

[codecov]

Codecov Report

Base: 36.56% // Head: 36.49% // Decreases project coverage by -0.07% ⚠️

Coverage data is based on head (d48eb6e) compared to base (c95f890).
Patch coverage: 6.97% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2076      +/-   ##
==========================================
- Coverage   36.56%   36.49%   -0.07%     
==========================================
  Files         229      229              
  Lines       19538    19570      +32     
==========================================
- Hits         7144     7143       -1     
- Misses      11555    11587      +32     
- Partials      839      840       +1     

Flag	Coverage Δ
unit	36.49% <6.97%> (-0.07%)	⬇️

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

Impacted Files	Coverage Δ
commands/instances.go	38.65% <0.00%> (-2.03%)	⬇️
arduino/errors.go	4.96% <23.07%> (+0.52%)	⬆️
arduino/cores/packagemanager/package_manager.go	67.46% <0.00%> (-0.73%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[cmaglie]

👍🏼

[kittaakos]

Hi, thanks for moving the request forward. IDE2 is eager to try it out (arduino/arduino-ide#1925 (comment)). I do not understand something. Can you please help?

There is no error when the library index is missing. I am unfamiliar with the go code, but maybe here. When I wipe my directories.data location and Create and Init a client. I get this error:

rpcurl \
  -plaintext \
  -import-path ./rpc \
  -proto cc/arduino/cli/commands/v1/commands.proto \
  127.0.0.1:50051 \
  cc.arduino.cli.commands.v1.ArduinoCoreService.Create

{
  "instance": {
    "id": 1
  }
}

grpcurl \
  -plaintext \
  -import-path ./rpc \
  -proto cc/arduino/cli/commands/v1/commands.proto \
  -d '{"instance": {"id": 1}}' \
  127.0.0.1:50051 \
  cc.arduino.cli.commands.v1.ArduinoCoreService.Init

{
  "error": {
    "code": 9,
    "message": "Loading index file: loading json index file /Users/a.kitta/Library/Arduino15/package_index.json: open /Users/a.kitta/Library/Arduino15/package_index.json: no such file or directory",
    "details": [
      {"@type":"type.googleapis.com/cc.arduino.cli.commands.v1.FailedInstanceInitError","message":"Loading index file: loading json index file /Users/a.kitta/Library/Arduino15/package_index.json: open /Users/a.kitta/Library/Arduino15/package_index.json: no such file or directory","reason":"FAILED_INSTANCE_INIT_REASON_INDEX_LOAD_ERROR"}
    ]
  }
}
{
  "error": {
    "code": 9,
    "message": "Error loading hardware platform: discovery builtin:serial-discovery not found",
    "details": [
      {"@type":"type.googleapis.com/cc.arduino.cli.commands.v1.PlatformLoadingError"}
    ]
  }
}
{
  "error": {
    "code": 9,
    "message": "Error loading hardware platform: discovery builtin:mdns-discovery not found",
    "details": [
      {"@type":"type.googleapis.com/cc.arduino.cli.commands.v1.PlatformLoadingError"}
    ]
  }
}
{
  "error": {
    "code": 9,
    "message": "Loading index file: reading library_index.json: open /Users/a.kitta/Library/Arduino15/library_index.json: no such file or directory"
  }
}

👆 Notice the missing details[] when the library_index.json file is missing.

Why does the CLI distinguish between FailedInstanceInitError and PlatformLoadingError? From the caller's point of view, it's still impossible to decide if the primary package index loading has failed, so the CLI/IDE2 is not functional, or loading a 3rd part platform has failed, which does not break the CLI/IDE2 functionality.

{
    "code": 9,
    "message": "Loading index file: loading json index file /Users/a.kitta/Library/Arduino15/package_index.json: open /Users/a.kitta/Library/Arduino15/package_index.json: no such file or directory",
    "detailsList": [
        {
            "typeUrl": "type.googleapis.com/cc.arduino.cli.commands.v1.FailedInstanceInitError",
            "value": "CAIStAFMb2FkaW5nIGluZGV4IGZpbGU6IGxvYWRpbmcganNvbiBpbmRleCBmaWxlIC9Vc2Vycy9hLmtpdHRhL0xpYnJhcnkvQXJkdWlubzE1L3BhY2thZ2VfaW5kZXguanNvbjogb3BlbiAvVXNlcnMvYS5raXR0YS9MaWJyYXJ5L0FyZHVpbm8xNS9wYWNrYWdlX2luZGV4Lmpzb246IG5vIHN1Y2ggZmlsZSBvciBkaXJlY3Rvcnk="
        }
    ]
}

{
    "code": 9,
    "message": "Loading index file: loading json index file /Users/a.kitta/Library/Arduino15/package_teensy_index.json: open /Users/a.kitta/Library/Arduino15/package_teensy_index.json: no such file or directory",
    "detailsList": [
        {
            "typeUrl": "type.googleapis.com/cc.arduino.cli.commands.v1.FailedInstanceInitError",
            "value": "CAISwgFMb2FkaW5nIGluZGV4IGZpbGU6IGxvYWRpbmcganNvbiBpbmRleCBmaWxlIC9Vc2Vycy9hLmtpdHRhL0xpYnJhcnkvQXJkdWlubzE1L3BhY2thZ2VfdGVlbnN5X2luZGV4Lmpzb246IG9wZW4gL1VzZXJzL2Eua2l0dGEvTGlicmFyeS9BcmR1aW5vMTUvcGFja2FnZV90ZWVuc3lfaW5kZXguanNvbjogbm8gc3VjaCBmaWxlIG9yIGRpcmVjdG9yeQ=="
        }
    ]
}

Can CLI have a more expressive name for the PlatformLoadingError; maybe hardware platform or (discovery) tool loading error? I think PlatformLoadingError is misleading for consumers.

[kittaakos]

When does the CLI provide the FAILED_INSTANCE_INIT_REASON_TOOL_LOAD_ERROR reason? I deleted the directories.data folder, but I get the following error:

{
  "error": {
    "code": 9,
    "message": "Error loading hardware platform: discovery builtin:serial-discovery not found",
    "details": [
      {"@type":"type.googleapis.com/cc.arduino.cli.commands.v1.PlatformLoadingError"}
    ]
  }
}

I would expect something like this:

{
  "error": {
    "code": 9,
    "message": "Error loading hardware platform: discovery builtin:serial-discovery not found",
    "details": [
      {"@type":"type.googleapis.com/cc.arduino.cli.commands.v1.FailedInstanceInitError","message":"Error loading hardware platform: discovery builtin:serial-discovery not found","reason":"FAILED_INSTANCE_INIT_REASON_TOOL_LOAD_ERROR"}
    ]
  }
}

From the docs:

// FAILED_INSTANCE_INIT_REASON_TOOL_LOAD_ERROR failure encountered while
// loading a tool

[Bikappa]

Hi @kittaakos,
PlatformLoadingError was added in details to all the errors that were already classified of type PlatformLoadingError in the code, but were missing a machine readable/fixed field. Those are basically the ones starting with "Error loading hardware platform: in english.

FailedInstanceInitError was added in details to those errors which were not classified in code and returned as generic rpc statuses. The reason field of these has been arbitrary created from the pre-existing free form error messages. Please see below if you have suggestions on this field.
Note regarding:

Notice the missing details[] when the library_index.json file is missing.

it was left behind in this category. I'm preparing a follow up pr for it.

If I understand correctly you are suggesting to wrap PlatformLoadingError into a FailedInstanceInitError and make it recognizable by reason instead of type in the detail element or perhaps dropping the "Platform" trait entirely like in

{"@type":"type.googleapis.com/cc.arduino.cli.commands.v1.FailedInstanceInitError","message":"Error loading hardware platform: discovery builtin:serial-discovery not found","reason":"FAILED_INSTANCE_INIT_REASON_TOOL_LOAD_ERROR"}

I like the approach and I'm including it in the aforementioned follow-up pr. But if further differentiation is desired from the CLI output: like different types of PlatformLoadingErrors or the information about the severity of the error for the CLI functioning I'd strongly suggest to prepare rfc/document so to align on all scenarios where we want error details and which these should be. It would help prevent indefinite and long iterations on the subject.

[kittaakos]

Thanks for the reply, @Bikappa!

classified of type PlatformLoadingError in the code,

From the API's perspective, it does not matter how you name the error type in Go, but there is an API type Platform, so I would prefer having a more expressive error type when loading a hardware tool failed. PlatformLoadingError is ambiguous, and checking only the proto API is neither used nor documented.

it was left behind in this category. I'm preparing a follow up pr for it.

Thank you!

If I understand correctly you are suggesting

I put it like this, IDE2 needs to detect two distinct errors during the InitRequest:

• fatal: loading the primary package index, the library index, or any hardware tool has failed: IDE2 must run an index update before the init request
• error: when loading any 3rd party package index has failed (invalid URL, no Internet, etc.): IDE2 can remain functional without running an index update before the init

IDE2 can try out any proposed CLI changes from a PR, so when such a changeset is ready, I highly recommend trying it out before the merge.