Skip to content
This repository has been archived by the owner on Dec 27, 2022. It is now read-only.

Server-side exceptions can be difficult to debug due to lack of error detail #58

Open
fake-name opened this issue Feb 15, 2021 · 0 comments

Comments

@fake-name
Copy link

fake-name commented Feb 15, 2021

This is a issue to inquire about a potential PR.

Currently, I'm dealing with trying to track down an exception in a number of calls provided by a mprpc.RPCServer subclass.

Basically, these calls fail only when passed certain arguments. The RPCServer server I'm working against has a number of clients using it concurrently, and I'd really like to avoid having to reboot it if at all possible because it would interrupt a number of running tasks which I'd then have to also restart.

Right now, a server-side exception is handled, but it's flattened to str(e), where e is the exception. This discards the entire server-side traceback and even a lot of the information the actual exception name may contain.

It'd be very nice to propagate the traceback from the server to the client in some form.

I can see a number of ways to do this, though they all have some caveats:

Protocol-compatible:

  1. The str(e) could be simply replaced with traceback.format_exc(). This would be drop-in compatible with the existing protocol, but the client side output would be a bit messy since the actual client-side exception is raised as simply raise RPCError(str(error))
  2. The way the client handles errors, if error is True, result is ignored (and server-side, it's set to None when sending an exception). This means it should be possible to simply place the results of traceback.format_exc() in the place where results go in a non-exception context. This will interoperate with older clients (since the results data is just ignored when if error:, but can be used to provide more information if present (this means that old server -> new client & new server -> old client can both be made to work)
  3. Instead of just returning a string of the error as the error member, the error member could be replaced with a tuple containing (str(e), traceback.format_exc()). This should interoperate correctly (though the stdout would be a bit messy) since error is explicitly stringified in the client, so new server -> old client will work, and old server -> new client can have a type check on error.

Protocol Incompatible:

  1. I could conceivably add a additional message type ( MSGPACKRPC_REQUEST, MSGPACKRPC_RESPONSE, MSGPACKRPC_EXCEPTION) with a explicit error indication and handle that differently. The downside of this is that a new server -> old client will fail when the server sends an exception, and a old server -> new client will still need to maintain the existing error handling code in addition to the new message type.

I think the second option would be nice, and the fact that it can be added without breaking the existing API is a bonus.

So basically, with everything above said, would a PR that adds the above error information be accepted?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant