Re: [PATCH net-next v3 2/6] tools/net/ynl: Report netlink errors without stacktrace

On Thu, 7 Mar 2024 at 09:31, Breno Leitao [off-list ref] wrote:

Hello Donald,

On Wed, Mar 06, 2024 at 11:10:42PM +0000, Donald Hunter wrote:

quoted

ynl does not handle NlError exceptions so they get reported like program
failures. Handle the NlError exceptions and report the netlink errors
more cleanly.

Example now:

Netlink error: No such file or directory
nl_len = 44 (28) nl_flags = 0x300 nl_type = 2
      error: -2       extack: {'bad-attr': '.op'}

Example before:

Traceback (most recent call last):
  File "/home/donaldh/net-next/./tools/net/ynl/cli.py", line 81, in <module>
    main()
  File "/home/donaldh/net-next/./tools/net/ynl/cli.py", line 69, in main
    reply = ynl.dump(args.dump, attrs)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/donaldh/net-next/tools/net/ynl/lib/ynl.py", line 906, in dump
    return self._op(method, vals, [], dump=True)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/donaldh/net-next/tools/net/ynl/lib/ynl.py", line 872, in _op
    raise NlError(nl_msg)
lib.ynl.NlError: Netlink error: No such file or directory
nl_len = 44 (28) nl_flags = 0x300 nl_type = 2
      error: -2       extack: {'bad-attr': '.op'}

Basically this is just hidding the stack, which may make it harder for
someone not used to the code to find the problem.

Usually fatal exception is handled to make the error more meaningful,
i.e, better than just the exception message + stack. Hidding the stack
and exitting may make the error less meaningful.

NlError is used to report a usage error reported by netlink as opposed
to a fatal exception. My thinking here is that it is better UX to
report netlink error responses without the stack trace precisely
because they are not exceptional. An NlError is not ynl program
breakage or subsystem breakage, it's e.g. nlctrl telling you that you
requested an op that does not exist.

On a different topic, I am wondering if we want to add type hitting for
these python program. They make the review process easier, and the
development a bit more structured. (Maybe that is what we expect from
upcoming new python code in netdev?!)

It's a good suggestion. I have never used python type hints so I'll
need to learn about them. I defer to the netdev maintainers about
whether this is something they want.

If that is where we want to go, this is *not*, by any mean, a blocker to
this code. Maybe something we can add to our public ToDo list (CC:
Jakub).