Improve error message due to missing FQBN in upload/compile commands

[federicobond]

No description provided.

[federicobond]

@masci this is the PR I mentioned in #545

[rsora]

Hi @federicobond, I gave a look at your PR, good job!

I was wondering if you want to modify your PR to try another approach for giving better error info to our users:

We moved recently the fqbn / port / sketch folder checks a level below in the commands module , to better support both the CLI and the gRPC interface.

To maintain a "Single Responsibility Principle" we think it could be better to:

1. create an error type alias in the commands package that represents the MissingFqbnError (see this for example
2. replace your error check in the cli/compile/compile.go and cli/upload/upload.go with an error check that leverages type checking to inspect the error returned from the compile.Compile() and upload.Upload() functions, for example using a switch err.(type) { construct

WDYT?

Thanks for your contribution and happy coding!

[federicobond]

Hi, thank you for the feedback, @rsora! One problem I see with that approach is that the error type would not be serialized through the gRPC interface, right? The error message string would, but its type would be lost in the translation.

[federicobond]

I do get your point about the Single Responsibility Principle though, and I am similarly bothered by the duplication we see here, but it's not at all uncommon to have this sort of duplication in other client/server architectures.

Sometimes clients are able to provide richer feedback and developers can take advantage of that, which usually requires some duplication of logic. Sometimes they are not, but it's important that users still receive some feedback from the backend.

Edit: we could do something like this instead.

[rsora]

Hi @federicobond,
I gave a look to the document you posted (thanks for suggesting!) and tried to bind errors generated in the commands packages with codes from the package mentioned in the blog post you shared, to imagine how a possible implementation could be.

I ended up having a lot of Internal Code = 13 errors, because commands modules are fairly high level modules, that wraps different errors coming from low level modules that cannot be identified at commands level.

Additionally, we cannot use the status package outside the commands because we want to avoid leaks of gRPC specific structures in the arduino-cli internals as much as possible.

So, if we need to continue on this path, I guess we should define specific error type alias for both commands and lower level modules and use them to:

• create status errors in case of gRPC to send back to client with all the details
• identify the specific error type at command line interface level and generate a proper CLI message for the user.

What do you think about implementing a POC for this approach? This way we can continue the discussion having some code to look at.

Thanks for your suggestion!

[federicobond]

Sure! I can try a POC of that. It will probably happen in the weekend though, feeling exhausted these days during the weeks.