Xud: Don't throw assertion on xucli argument error

[kilrau]

We should try catching all of these kind of xucli argement error so that it doesn't throw an assertion. Example:

testnet > openchannel 0.01 BTC BrassMarble 0.005
xucli openchannel <currency> <amount> [node_identifier] [push_amount]

open a payment channel with a peer

Positionals:
  currency  the ticker symbol for the currency               [string] [required]
  amount    the amount to be deposited into the channel      [number] [required]

Options:
  --help             Show help                                         [boolean]
  --version          Show version number                               [boolean]
  --rpcport, -p      RPC service port                                   [number]
  --rpchost, -h      RPC service hostname                               [string]
  --tlscertpath, -c  Path to the TLS certificate of xud                 [string]
  --json, -j         Display output in json format    [boolean] [default: false]
  --xudir, -x        Data directory for xud                             [string]
  --node_identifier  the node key or alias of the connected peer to open the
                     channel with                                       [string]
  --push_amount      the amount to be pushed to the remote side of the channel
                                                           [number] [default: 0]

Examples:
  xucli openchannel BTC 0.1                 open an 0.1 BTC channel by node key
  028599d05b18c0c3f8028915a17d603416f7276c
  822b6b2d20e71a3502bd0f9e0b
  xucli openchannel BTC 0.1 CheeseMonkey    open an 0.1 BTC channel by alias
  xucli openchannel BTC 0.1 CheeseMonkey    open an 0.1 BTC channel by alias and
  0.05                                      push 0.05 to remote side
  xucli openchannel ETH 0.5                 deposit 0.5 into an ETH Connext
                                            channel without specifying a remote
                                            node

Error [AssertionError]: Assertion failed
    at new goog.asserts.AssertionError (/app/node_modules/google-protobuf/google-protobuf.js:81:876)
    at Object.goog.asserts.doAssertFailure_ (/app/node_modules/google-protobuf/google-protobuf.js:82:257)
    at Object.goog.asserts.assert (/app/node_modules/google-protobuf/google-protobuf.js:83:83)
    at jspb.BinaryWriter.writeUint64 (/app/node_modules/google-protobuf/google-protobuf.js:483:419)
    at Function.proto.xudrpc.OpenChannelRequest.serializeBinaryToWriter (/app/dist/proto/xudrpc_pb.js:6591:12)
    at proto.xudrpc.OpenChannelRequest.serializeBinary (/app/dist/proto/xudrpc_pb.js:6561:35)
    at serialize_xudrpc_OpenChannelRequest (/app/dist/proto/xudrpc_grpc_pb.js:374:26)
    at Object.final_requester.sendMessage (/app/node_modules/grpc/src/client_interceptors.js:807:37)
    at InterceptingCall._callNext (/app/node_modules/grpc/src/client_interceptors.js:419:43)
    at InterceptingCall.sendMessage (/app/node_modules/grpc/src/client_interceptors.js:464:8) {
  reportErrorToServer: true,
  messagePattern: 'Assertion failed'
}

[kilrau]

Could you try reproducing this one and catch this error if it still exists? @rsercano

[rsercano]

This still occurs, I'll check this one.

[rsercano]

First I implemented a similar catching mechanism as Service layer and @sangaman had a comment about it (couldn't find) so currently I fixed this one, just controlling for openchannel, if there's any other one like this let me know, or if you want me to implemente a generic error catching mechanism we can discuss it too.

@sangaman @kilrau @raladev

[kilrau]

if you want me to implemente a generic error catching mechanism

That'd be good. An idea how to do that?

[rsercano]

There's a function called check for yargs, in there we can check arguments and write a small utility like in the Service layer to check arguments. https://yargs.js.org/docs/#api-reference-checkfn-globaltrue

What do you think? @sangaman @kilrau @raladev

[kilrau]

If it's performant enough so that one can't feel a difference to now - sure.

[rsercano]

@sangaman wdyt?

[sangaman]

@rsercano I'm sure performance wouldn't be an issue, I can't imagine any checks taking more than a couple seconds. Seems like a worthwhile idea to me.