RPC functions in server should not catch errors
Closed this issue · 7 comments
This kind of thing causes type errors if the CLI module is expecting anything besides a [str]. The error is swallowed and then the return value (with the precious error message!) causes another error with a stack trace that's entirely inside Flask and the type checker. My opinion is that RPC functions should not catch errors, they should throw and that error should bubble all the way back out to the caller.
except Exception as e:
return [f"Exception {e}"]My typechecker doesn't like it either
The only issue with this change is, if there is an error during rpc processing, then the error will only bubble back to the server, and not to the RPC caller.
IMO it's probably best practice to log the error to the server, and return a "server error" to the RPC caller?
Actually by removing the try/catch that is what happens. I had a syntax error in a scenario and when I called scenarios list I got useless stack dump. By removing the try catch in scenarios_list() I got "Server error" back on the cli, but the server log was finally helpful:
File "/Users/matthewzipkin/Desktop/work/warnet/src/scenarios/block_relay.py", line 43
self.generatetoaddress(self.nodes[node], 1 addr)
^^^^^^
SyntaxError: invalid syntax. Perhaps you forgot a comma?
PERHAPS I DID FORGET A COMMA
I'd still prefer to simplify with a decorator, always return a generic message to caller, and log any error and params to the server logs. Something like this?
main...willcl-ark:warnet:rpc-error-handler
I got "Server error" back on the cli
this is very interesting; I wonder if FlaskJSONRPC just handles all this automagically -- no need for decorators? Probably...
Perhaps that's coming from here: https://flask.palletsprojects.com/en/2.3.x/api/#flask.Flask.handle_exception
(and is all we need for this project...)
Hello team.
I am looking at giving a try at this issue.
Was going to ask, are we proposing using flask exception handler or the decorators. Its not really clear