Improve the error shown when an RPC return value fails serialisation

[mbirtwell]

ARTIQ Pull Request

Description of Changes

I've encountered failures with RPC return values failing to serialise from time to time. e.g. returning a different type than annotated from the RPC function. Making sure that the name of the RPC function called is included in the error makes tracking down the problem much easier.

Related Issue

Type of Changes

	Type
✓	🔨 Refactoring

Steps (Choose relevant, delete irrelevant before submitting)

All Pull Requests

• Use correct spelling and grammar.
• Update RELEASE_NOTES.rst if there are noteworthy changes, especially if there are changes to existing APIs.
• Close/update issues.
• Check the copyright situation of your changes and sign off your patches (git commit --signoff, see copyright).

Code Changes

• Run flake8 to check code style (follow PEP-8 style). flake8 has issues with parsing Migen/gateware code, ignore as necessary.
• Test your changes or have someone test them. Mention what was tested and how.

Git Logistics

• Split your contribution into logically separate changes (git rebase --interactive). Merge/squash/fixup commits that just fix or amend previous commits. Remove unintended changes & cleanup. See tutorial.
• Write short & meaningful commit messages. Review each commit for messages (git show). Format:

  topic: description. < 50 characters total.
  
  Longer description. < 70 characters per line
  

Licensing

See copyright & licensing for more info.
ARTIQ files that do not contain a license header are copyrighted by M-Labs Limited and are licensed under LGPLv3+.

[sbourdeauducq]

I would at least narrow down the try block to only the function susceptible to fail, which is only this line AFAICT?
Can't the code that raises the error initially simply include the function information in its exception? Why is it not in the traceback anyway?

[mbirtwell]

Why is it not in the traceback anyway?

This is after the function being called by the RPC has returned so it's not on the call stack anymore.

Can't the code that raises the error initially simply include the function information in its exception?

We could do, where ever it's an explicitly raised exception. In fact we already do in one instance, but most of the raised exceptions don't include this information. I like centralising these kind of things so that as more raises get added you don't have to remember to add it each time. And this also has the advantage of adding this information to any unexpected errors.

I would at least narrow down the try block to only the function susceptible to fail, which is only this line AFAICT?

I agree that is the most likely function to fail. But I was feeling defensive when I wrote this and I can't see any harm in increasing the scope of this try block. The original exception is preserved as the cause of the exception raised here so we're not losing any information and the context might just be useful.