clnrest: don't try to handle plugin.rpc.call results.

It already throws an exception or error, or decodes the JSON response:
we can simply pass it through.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

plugins/clnrest/utilities/rpc_routes.py

@@ -1,7 +1,8 @@
+from pyln.client import RpcError
 import json5
 from flask import request, make_response
 from flask_restx import Namespace, Resource
-from .shared import call_rpc_method, verify_rune, process_help_response
+from .shared import call_rpc_method, verify_rune, process_help_response, RuneError
 from .rpc_plugin import plugin
 
 methods_list = []
@@ -42,20 +43,19 @@ class RpcMethodResource(Resource):
             rpc_params = request.form.to_dict() if not request.is_json else request.get_json() if len(request.data) != 0 else {}
 
             try:
-                is_valid_rune = verify_rune(plugin, rune, rpc_method, rpc_params)
-                if "error" in is_valid_rune:
-                    plugin.log(f"Error: {is_valid_rune}", "error")
-                    raise Exception(is_valid_rune)
-
-            except Exception as err:
-                return json5.loads(str(err)), 401
+                verify_rune(plugin, rune, rpc_method, rpc_params)
+            except RpcError as rpc_err:
+                plugin.log(f"RPC Error: {str(rpc_err.error)}", "info")
+                return rpc_err.error, 401
+            except RuneError as rune_err:
+                plugin.log(f"Rune Error: {str(rune_err)}", "info")
+                return rune_err.error, 403
 
             try:
                 return call_rpc_method(plugin, rpc_method, rpc_params), 201
-
-            except Exception as err:
-                plugin.log(f"Error: {err}", "info")
-                return json5.loads(str(err)), 500
+            except RpcError as rpc_err:
+                plugin.log(f"RPC Error: {str(rpc_err.error)}", "info")
+                return (rpc_err.error, 404) if rpc_err.error["code"] == -32601 else (rpc_err.error, 500)
 
         except Exception as err:
             return f"Unable to parse request: {err}", 500

plugins/clnrest/utilities/shared.py

@@ -1,12 +1,16 @@
 import json5
-import re
-import json
 import ipaddress
 
 
 CERTS_PATH, REST_PROTOCOL, REST_HOST, REST_PORT, REST_CSP, REST_CORS_ORIGINS = "", "", "", "", "", []
 
 
+class RuneError(Exception):
+    def __init__(self, error=str({"code": 1501, "message": "Not authorized: Missing or invalid rune"})):
+        self.error = error
+        super().__init__(self.error)
+
+
 def validate_ip4(ip_str):
     try:
         # Create an IPv4 address object.
@@ -61,34 +65,12 @@ def set_config(options):
 
 
 def call_rpc_method(plugin, rpc_method, payload):
-    try:
-        response = plugin.rpc.call(rpc_method, payload)
-        if '"error":' in str(response).lower():
-            raise Exception(response)
-        else:
-            plugin.log(f"{response}", "debug")
-            if '"result":' in str(response).lower():
-                # Use json5.loads ONLY when necessary, as it increases processing time
-                return json.loads(response)["result"]
-            else:
-                return response
-
-    except Exception as err:
-        plugin.log(f"Error: {err}", "info")
-        if "error" in str(err).lower():
-            match_err_obj = re.search(r'"error":\{.*?\}', str(err))
-            if match_err_obj is not None:
-                err = "{" + match_err_obj.group() + "}"
-            else:
-                match_err_str = re.search(r"error: \{.*?\}", str(err))
-                if match_err_str is not None:
-                    err = "{" + match_err_str.group() + "}"
-        raise Exception(err)
+    return plugin.rpc.call(rpc_method, payload)
 
 
 def verify_rune(plugin, rune, rpc_method, rpc_params):
     if rune is None:
-        raise Exception('{ "error": {"code": 403, "message": "Not authorized: Missing rune"} }')
+        raise RuneError({"code": 1501, "message": "Not authorized: Missing rune"})
 
     return call_rpc_method(plugin, "checkrune",
                            {"rune": rune,

tests/test_clnrest.py

@@ -1,6 +1,7 @@
 from ephemeral_port_reserve import reserve
 from fixtures import *  # noqa: F401,F403
 from pyln.testing.utils import env, TEST_NETWORK
+from pyln.client import Millisatoshi
 import unittest
 import requests
 from pathlib import Path
@@ -164,9 +165,9 @@ def test_clnrest_unknown_method(node_factory):
     """Test POST request error on `/v1/unknown-post` end point."""
     rune = l1.rpc.createrune()['rune']
     response = http_session.post(base_url + '/v1/unknown-post', headers={'Rune': rune}, verify=ca_cert)
-    assert response.status_code == 500
-    assert response.json()['error']['code'] == -32601
-    assert response.json()['error']['message'] == "Unknown command 'unknown-post'"
+    assert response.status_code == 404
+    assert response.json()['code'] == -32601
+    assert response.json()['message'] == "Unknown command 'unknown-post'"
 
 
 def test_clnrest_rpc_method(node_factory):
@@ -177,17 +178,17 @@ def test_clnrest_rpc_method(node_factory):
 
     # /v1/getinfo no rune provided in header of the request
     response = http_session.post(base_url + '/v1/getinfo', verify=ca_cert)
-    assert response.status_code == 401
-    assert response.json()['error']['code'] == 403
-    assert response.json()['error']['message'] == 'Not authorized: Missing rune'
+    assert response.status_code == 403
+    assert response.json()['code'] == 1501
+    assert response.json()['message'] == 'Not authorized: Missing rune'
 
     # /v1/getinfo with a rune which doesn't authorized getinfo method
     rune_no_getinfo = l1.rpc.createrune(restrictions=[["method/getinfo"]])['rune']
     response = http_session.post(base_url + '/v1/getinfo', headers={'Rune': rune_no_getinfo},
                                  verify=ca_cert)
     assert response.status_code == 401
-    assert response.json()['error']['code'] == 1502
-    assert response.json()['error']['message'] == 'Not permitted: method is equal to getinfo'
+    assert response.json()['code'] == 1502
+    assert response.json()['message'] == 'Not permitted: method is equal to getinfo'
 
     # /v1/getinfo with a correct rune
     rune_getinfo = l1.rpc.createrune(restrictions=[["method=getinfo"]])['rune']
@@ -201,7 +202,7 @@ def test_clnrest_rpc_method(node_factory):
     response = http_session.post(base_url + '/v1/invoice', headers={'Rune': rune_invoice},
                                  verify=ca_cert)
     assert response.status_code == 500
-    assert response.json()['error']['code'] == -32602
+    assert response.json()['code'] == -32602
 
     # /v1/invoice with a correct rune but wrong parameters
     rune_invoice = l1.rpc.createrune(restrictions=[["method=invoice"]])['rune']
@@ -211,7 +212,7 @@ def test_clnrest_rpc_method(node_factory):
                                        'description': 'description'},
                                  verify=ca_cert)
     assert response.status_code == 500
-    assert response.json()['error']['code'] == -32602
+    assert response.json()['code'] == -32602
 
     # l2 pays l1's invoice where the invoice is created with /v1/invoice
     rune_invoice = l1.rpc.createrune(restrictions=[["method=invoice"]])['rune']
@@ -282,7 +283,7 @@ def test_clnrest_websocket_wrong_rune(node_factory):
 
     notifications = notifications_received_via_websocket(l1, base_url, http_session)
     l1.daemon.logsearch_start = 0
-    assert l1.daemon.is_in_log(r"plugin-clnrest.py: {error: {'code': 1501, 'message': 'Not authorized: Not derived from master'}}")
+    assert l1.daemon.is_in_log(r"error: {'code': 1501, 'message': 'Not authorized: Not derived from master'}")
     assert len(notifications) == 0