Improving CLI error messages

[lameferret]

Current Behaviour

1. ockam tcp-listener apple ✔
       
2. ockam tcp-listener create ✔
       
3. ockam tcp-listener create ip4apple ❌
       
4. ockam tcp-listener create "11:11:11" ❌
       
5. ockam tcp-listener create "127.0.0.1:7001" ❌
       
6. ockam tcp-listener create "127.0.0.1:7001" --node ✔
       
7. ./ockam tcp-listener create "127.0.0.1:7001" --node badnode ❌
       
8. ockam tcp-listener create "127.0.0.1:7001" --node nodewthlistener ❌
       
9. ockam tcp-listener create "127.0.0.1:7001" --node nodewithoutlistener ✔
       

 ## Suggested Behaviour
[NOTE]: I'm really new to the repo, so my understanding and solution can both be wrong.

• Issue 1 is fine but IMO rather than [OPTIONS] possible commands should be listed.
• Issue 2, rather than just format should also be specified, which in this case would be <BIND> : IPv4:PORT
• Issues like 3 and 4 are simple, with strict struct parsing i.e. std::net::Ipv4Addr::parse() in case of ipv4) and they are probably here because of refactoring.
• Issue 5 should print out, specifically what is required to create a tcp-listener
• Issue 6 clearly shows that a node is required, with it listing available nodes would be a bit better. IIUC every node can have only one tcp-listener if so, listing node without listener would be the best.
• Issue 7, same as
• Issue 8, can be much clearer about the error
• 9 Yay!! Working tcp-listener

@mrinalwadhwa can you please further elaborate this:

That's a great idea. I think each command should have examples defined in a const and then we can show them on error and we can add them to the HELP_TEMPLATE.

[mrinalwadhwa]

every node can have only one tcp-listener if so, listing node without listener would be the best.

There can be multiple listener on one node.

This works:

ockam node create n1 --api-address 127.0.0.1:6001
ockam tcp-listener create 127.0.0.1:7001 --node n1
ockam tcp-listener create 127.0.0.1:8001 --node n1

[mrinalwadhwa]

Issue 6 clearly shows that a node is required, with it listing available nodes would be a bit better.

that would be great 👍

[mrinalwadhwa]

Issue 1 is fine but IMO rather than [OPTIONS] possible commands should be listed.

Listing tcp-listener subcommands would be great 👍 if we can get clap to do that.

[mrinalwadhwa]

Issue 2, rather than just format should also be specified, which in this case would be : IPv4:PORT

That would be great maybe also an example value 127.0.0.1:7001 👍

[mrinalwadhwa]

Issues like 3 and 4 are simple, with strict struct parsing i.e. std::net::Ipv4Addr::parse() in case of ipv4) and they are probably here because of refactoring.

👍 agreed, we want to support both ipv4 & ipv6 values.

[mrinalwadhwa]

Issue 5 should print out, specifically what is required to create a tcp-listener

Agreed.
This one connects most to my comment:

I think each command should have examples defined in a const and then we can show them on error and we can add them to the HELP_TEMPLATE.

I think each command should come with a set of full use examples. For instance creating a TCP listener should show how to create a node and then how to create a listener in that node in a series of commands. This EXAMPLES section can be added to a command's help inserting it just before the LEARN MORE section in the HELP_TEMPLATE. We can modify it just before giving it to clap here.

[mrinalwadhwa]

Issue 8, can be much clearer about the error

Agree. Not sure what the error you encountered was.

[lameferret]

I tried creating tcp-listiner on the same address & port. And I don't think there can be two listeners on the same port. So listener already exists on port: xxxx would be the desired output.

[mrinalwadhwa]

@anuvratsingh all very good points. Feel free to create new issues for these. If you don't get a chance, I'll create some in my morning.