Merge branch 'jsonrpc-fix-errors'

* jsonrpc-fix-errors:
  add todo
  format response and error properly
  fixes, refactors
  changelog
  hack for lbrynet-cli for now
  fix error handling in jsonrpc

CHANGELOG.md

@@ -11,6 +11,8 @@ at anytime.
 ### Added
   *
   *
+  * Add `wallet_list` command
+  * Add checks for missing/extraneous params when calling jsonrpc commands
   *
 
 ### Changed
@@ -22,6 +24,12 @@ at anytime.
   * Fix restart procedure in DaemonControl
   *
   * Create download directory if it doesn't exist
+  * Fixed descriptor_get
+  * Fixed jsonrpc_reflect()
+  * Fixed api help return
+  * Fixed API command descriptor_get
+  * Fixed API command transaction_show
+  * Fixed error handling for jsonrpc commands
   *
 
 ## [0.9.2rc1] - 2017-03-21

lbrynet/core/Wallet.py

@@ -20,11 +20,10 @@ from lbrynet.core.sqlite_helpers import rerun_if_locked
 from lbrynet.interfaces import IRequestCreator, IQueryHandlerFactory, IQueryHandler, IWallet
 from lbrynet.core.client.ClientRequest import ClientRequest
 from lbrynet.core.Error import (UnknownNameError, InvalidStreamInfoError, RequestCanceledError,
-                            InsufficientFundsError)
+                                InsufficientFundsError)
 from lbrynet.db_migrator.migrate1to2 import UNSET_NOUT
 from lbrynet.metadata.Metadata import Metadata
 
-
 log = logging.getLogger(__name__)
 
 
@@ -1030,13 +1029,15 @@ class LBRYumWallet(Wallet):
         d.addCallback(lambda claim_out: self._broadcast_claim_transaction(claim_out))
         return d
 
+    @defer.inlineCallbacks
     def _abandon_claim(self, claim_outpoint):
         log.debug("Abandon %s %s" % (claim_outpoint['txid'], claim_outpoint['nout']))
         broadcast = False
-        d = self._run_cmd_as_defer_succeed('abandon', claim_outpoint['txid'],
-                                           claim_outpoint['nout'], broadcast)
-        d.addCallback(lambda claim_out: self._broadcast_claim_transaction(claim_out))
-        return d
+        abandon_tx = yield self._run_cmd_as_defer_succeed(
+            'abandon', claim_outpoint['txid'], claim_outpoint['nout'], broadcast
+        )
+        claim_out = yield self._broadcast_claim_transaction(abandon_tx)
+        defer.returnValue(claim_out)
 
     def _support_claim(self, name, claim_id, amount):
         log.debug("Support %s %s %f" % (name, claim_id, amount))

lbrynet/lbrynet_daemon/Daemon.py

@@ -1444,9 +1444,8 @@ class Daemon(AuthJSONRPCServer):
 
     @AuthJSONRPCServer.auth_required
     @defer.inlineCallbacks
-    def jsonrpc_get(
-            self, name, file_name=None, stream_info=None, timeout=None,
-            download_directory=None, wait_for_write=True):
+    def jsonrpc_get(self, name, file_name=None, stream_info=None, timeout=None,
+                    download_directory=None, wait_for_write=True):
         """
         Download stream from a LBRY name.
 
@@ -1632,8 +1631,6 @@ class Daemon(AuthJSONRPCServer):
         cost = yield self.get_est_cost(name, size)
         defer.returnValue(cost)
 
-
-
     @AuthJSONRPCServer.auth_required
     @defer.inlineCallbacks
     def jsonrpc_publish(self, name, bid, metadata=None, file_path=None, fee=None, title=None,
@@ -1725,7 +1722,6 @@ class Daemon(AuthJSONRPCServer):
                     metadata['fee'][currency]['address'] = new_address
             metadata['fee'] = FeeValidator(metadata['fee'])
 
-
         log.info("Publish: %s", {
             'name': name,
             'file_path': file_path,
@@ -1765,9 +1761,13 @@ class Daemon(AuthJSONRPCServer):
         try:
             abandon_claim_tx = yield self.session.wallet.abandon_claim(txid, nout)
             response = yield self._render_response(abandon_claim_tx)
-        except Exception as err:
+        except BaseException as err:
             log.warning(err)
-            response = yield self._render_response(err)
+            # pylint: disable=unsubscriptable-object
+            if len(err.args) and err.args[0] == "txid was not found in wallet":
+                raise Exception("This transaction was not found in your wallet")
+            else:
+                response = yield self._render_response(err)
         defer.returnValue(response)
 
     @AuthJSONRPCServer.auth_required
@@ -2113,7 +2113,6 @@ class Daemon(AuthJSONRPCServer):
         """
         return self.jsonrpc_blob_get(sd_hash, timeout, 'json', payment_rate_manager)
 
-
     @AuthJSONRPCServer.auth_required
     @defer.inlineCallbacks
     def jsonrpc_blob_get(self, blob_hash, timeout=None, encoding=None, payment_rate_manager=None):
@@ -2639,6 +2638,3 @@ def format_json_out_amount_as_float(obj):
     elif isinstance(obj, list):
         obj = [format_json_out_amount_as_float(o) for o in obj]
     return obj
-
-
-

lbrynet/lbrynet_daemon/auth/server.py

@@ -1,5 +1,7 @@
 import logging
 import urlparse
+import inspect
+import json
 
 from decimal import Decimal
 from zope.interface import implements
@@ -8,11 +10,13 @@ from twisted.internet import defer
 from twisted.python.failure import Failure
 from twisted.internet.error import ConnectionDone, ConnectionLost
 from txjsonrpc import jsonrpclib
+from traceback import format_exc
 
 from lbrynet import conf
 from lbrynet.core.Error import InvalidAuthenticationToken
 from lbrynet.core import utils
-from lbrynet.lbrynet_daemon.auth.util import APIKey, get_auth_message, jsonrpc_dumps_pretty
+from lbrynet.undecorated import undecorated
+from lbrynet.lbrynet_daemon.auth.util import APIKey, get_auth_message
 from lbrynet.lbrynet_daemon.auth.client import LBRY_SECRET
 
 log = logging.getLogger(__name__)
@@ -20,24 +24,65 @@ log = logging.getLogger(__name__)
 EMPTY_PARAMS = [{}]
 
 
+class JSONRPCError(object):
+    # http://www.jsonrpc.org/specification#error_object
+    CODE_PARSE_ERROR = -32700  # Invalid JSON. Error while parsing the JSON text.
+    CODE_INVALID_REQUEST = -32600  # The JSON sent is not a valid Request object.
+    CODE_METHOD_NOT_FOUND = -32601  # The method does not exist / is not available.
+    CODE_INVALID_PARAMS = -32602  # Invalid method parameter(s).
+    CODE_INTERNAL_ERROR = -32603  # Internal JSON-RPC error (I think this is like a 500?)
+    CODE_APPLICATION_ERROR = -32500  # Generic error with our app??
+    CODE_AUTHENTICATION_ERROR = -32501  # Authentication failed
+
+    MESSAGES = {
+        CODE_PARSE_ERROR: "Parse Error. Data is not valid JSON.",
+        CODE_INVALID_REQUEST: "JSON data is not a valid Request",
+        CODE_METHOD_NOT_FOUND: "Method Not Found",
+        CODE_INVALID_PARAMS: "Invalid Params",
+        CODE_INTERNAL_ERROR: "Internal Error",
+        CODE_AUTHENTICATION_ERROR: "Authentication Failed",
+    }
+
+    HTTP_CODES = {
+        CODE_INVALID_REQUEST: 400,
+        CODE_PARSE_ERROR: 400,
+        CODE_INVALID_PARAMS: 400,
+        CODE_METHOD_NOT_FOUND: 404,
+        CODE_INTERNAL_ERROR: 500,
+        CODE_APPLICATION_ERROR: 500,
+        CODE_AUTHENTICATION_ERROR: 401,
+    }
+
+    def __init__(self, message, code=CODE_APPLICATION_ERROR, traceback=None, data=None):
+        assert isinstance(code, (int, long)), "'code' must be an int"
+        assert (data is None or isinstance(data, dict)), "'data' must be None or a dict"
+        self.code = code
+        if message is None:
+            message = self.MESSAGES[code] if code in self.MESSAGES else "Error"
+        self.message = message
+        self.data = {} if data is None else data
+        if traceback is not None:
+            self.data['traceback'] = traceback.split("\n")
+
+    def to_dict(self):
+        ret = {
+            'code': self.code,
+            'message': self.message,
+        }
+        if len(self.data):
+            ret['data'] = self.data
+        return ret
+
+    @classmethod
+    def create_from_exception(cls, exception, code=CODE_APPLICATION_ERROR, traceback=None):
+        return cls(exception.message, code=code, traceback=traceback)
+
+
 def default_decimal(obj):
     if isinstance(obj, Decimal):
         return float(obj)
 
 
-class JSONRPCException(Exception):
-    def __init__(self, err, code):
-        self.faultCode = code
-        self.err = err
-
-    @property
-    def faultString(self):
-        try:
-            return self.err.getTraceback()
-        except AttributeError:
-            return str(self.err)
-
-
 class UnknownAPIMethodError(Exception):
     pass
 
@@ -50,6 +95,21 @@ def trap(err, *to_trap):
     err.trap(*to_trap)
 
 
+def jsonrpc_dumps_pretty(obj, **kwargs):
+    try:
+        id_ = kwargs.pop("id")
+    except KeyError:
+        id_ = None
+
+    if isinstance(obj, JSONRPCError):
+        data = {"jsonrpc": "2.0", "error": obj.to_dict(), "id": id_}
+    else:
+        data = {"jsonrpc": "2.0", "result": obj, "id": id_}
+
+    return json.dumps(data, cls=jsonrpclib.JSONRPCEncoder, sort_keys=True, indent=2,
+                      separators=(',', ': '), **kwargs) + "\n"
+
+
 class AuthorizedBase(object):
     def __init__(self):
         self.authorized_functions = []
@@ -93,11 +153,6 @@ class AuthJSONRPCServer(AuthorizedBase):
     implements(resource.IResource)
 
     isLeaf = True
-    OK = 200
-    UNAUTHORIZED = 401
-    # TODO: codes should follow jsonrpc spec: http://www.jsonrpc.org/specification#error_object
-    NOT_FOUND = 8001
-    FAILURE = 8002
 
     def __init__(self, use_authentication=None):
         AuthorizedBase.__init__(self)
@@ -121,30 +176,50 @@ class AuthJSONRPCServer(AuthorizedBase):
             session_id = request.getSession().uid
             request.setHeader(LBRY_SECRET, self.sessions.get(session_id).secret)
 
-    def _render_message(self, request, message):
+    @staticmethod
+    def _render_message(request, message):
         request.write(message)
         request.finish()
 
-    def _render_error(self, failure, request, id_,
-                      version=jsonrpclib.VERSION_2, response_code=FAILURE):
-        # TODO: is it necessary to wrap the failure in a Failure? if not, merge this with next fn
-        self._render_error_string(Failure(failure), request, id_, version, response_code)
+    def _render_error(self, failure, request, id_):
+        if isinstance(failure, JSONRPCError):
+            error = failure
+        elif isinstance(failure, Failure):
+            # maybe failure is JSONRPCError wrapped in a twisted Failure
+            error = failure.check(JSONRPCError)
+            if error is None:
+                # maybe its a twisted Failure with another type of error
+                error = JSONRPCError(failure.getErrorMessage(), traceback=failure.getTraceback())
+        else:
+            # last resort, just cast it as a string
+            error = JSONRPCError(str(failure))
 
-    def _render_error_string(self, error_string, request, id_, version=jsonrpclib.VERSION_2,
-                             response_code=FAILURE):
-        err = JSONRPCException(error_string, response_code)
-        fault = jsonrpc_dumps_pretty(err, id=id_, version=version)
-        self._set_headers(request, fault)
-        if response_code != AuthJSONRPCServer.FAILURE:
-            request.setResponseCode(response_code)
-        self._render_message(request, fault)
+        response_content = jsonrpc_dumps_pretty(error, id=id_)
 
-    def _handle_dropped_request(self, result, d, function_name):
+        self._set_headers(request, response_content)
+        # TODO: uncomment this after fixing lbrynet-cli to handle error code responses correctly
+        # try:
+        #     request.setResponseCode(JSONRPCError.HTTP_CODES[error.code])
+        # except KeyError:
+        #     request.setResponseCode(JSONRPCError.HTTP_CODES[JSONRPCError.CODE_INTERNAL_ERROR])
+        self._render_message(request, response_content)
+
+    @staticmethod
+    def _handle_dropped_request(result, d, function_name):
         if not d.called:
             log.warning("Cancelling dropped api request %s", function_name)
             d.cancel()
 
     def render(self, request):
+        try:
+            return self._render(request)
+        except BaseException as e:
+            log.error(e)
+            error = JSONRPCError.create_from_exception(e, traceback=format_exc())
+            self._render_error(error, request, None)
+            return server.NOT_DONE_YET
+
+    def _render(self, request):
         time_in = utils.now()
         # assert self._check_headers(request), InvalidHeaderError
         session = request.getSession()
@@ -161,7 +236,7 @@ class AuthJSONRPCServer(AuthorizedBase):
                 session.startCheckingExpiration()
                 session.notifyOnExpire(expire_session)
                 message = "OK"
-                request.setResponseCode(self.OK)
+                request.setResponseCode(200)
                 self._set_headers(request, message, True)
                 self._render_message(request, message)
                 return server.NOT_DONE_YET
@@ -174,14 +249,21 @@ class AuthJSONRPCServer(AuthorizedBase):
             parsed = jsonrpclib.loads(content)
         except ValueError:
             log.warning("Unable to decode request json")
-            self._render_error_string('Invalid JSON', request, None)
+            self._render_error(JSONRPCError(None, JSONRPCError.CODE_PARSE_ERROR), request, None)
             return server.NOT_DONE_YET
 
-        function_name = parsed.get('method')
-        args = parsed.get('params', {})
-        id_ = parsed.get('id')
-        token = parsed.pop('hmac', None)
-        version = self._get_jsonrpc_version(parsed.get('jsonrpc'), id_)
+        id_ = None
+        try:
+            function_name = parsed.get('method')
+            args = parsed.get('params', {})
+            id_ = parsed.get('id', None)
+            token = parsed.pop('hmac', None)
+        except AttributeError as err:
+            log.warning(err)
+            self._render_error(
+                JSONRPCError(None, code=JSONRPCError.CODE_INVALID_REQUEST), request, id_
+            )
+            return server.NOT_DONE_YET
 
         reply_with_next_secret = False
         if self._use_authentication:
@@ -191,49 +273,100 @@ class AuthJSONRPCServer(AuthorizedBase):
                 except InvalidAuthenticationToken as err:
                     log.warning("API validation failed")
                     self._render_error(
-                        err, request, id_, version=version,
-                        response_code=AuthJSONRPCServer.UNAUTHORIZED)
+                        JSONRPCError.create_from_exception(
+                            err.message, code=JSONRPCError.CODE_AUTHENTICATION_ERROR,
+                            traceback=format_exc()
+                        ),
+                        request, id_
+                    )
                     return server.NOT_DONE_YET
                 self._update_session_secret(session_id)
                 reply_with_next_secret = True
 
         try:
             function = self._get_jsonrpc_method(function_name)
-        except (UnknownAPIMethodError, NotAllowedDuringStartupError) as err:
+        except UnknownAPIMethodError as err:
             log.warning('Failed to get function %s: %s', function_name, err)
-            self._render_error(err, request, version)
+            self._render_error(
+                JSONRPCError(None, JSONRPCError.CODE_METHOD_NOT_FOUND),
+                request
+            )
+            return server.NOT_DONE_YET
+        except NotAllowedDuringStartupError as err:
+            log.warning('Function not allowed during startup %s: %s', function_name, err)
+            self._render_error(
+                JSONRPCError("This method is unavailable until the daemon is fully started",
+                             code=JSONRPCError.CODE_INVALID_REQUEST),
+                request
+            )
             return server.NOT_DONE_YET
 
         if args == EMPTY_PARAMS or args == []:
-            d = defer.maybeDeferred(function)
+            args_dict = {}
         elif isinstance(args, dict):
-            d = defer.maybeDeferred(function, **args)
+            args_dict = args
         elif len(args) == 1 and isinstance(args[0], dict):
             # TODO: this is for backwards compatibility. Remove this once API and UI are updated
             # TODO: also delete EMPTY_PARAMS then
-            d = defer.maybeDeferred(function, **args[0])
+            args_dict = args[0]
         else:
             # d = defer.maybeDeferred(function, *args)  # if we want to support positional args too
             raise ValueError('Args must be a dict')
 
+        params_error, erroneous_params = self._check_params(function, args_dict)
+        if params_error is not None:
+            params_error_message = '{} for {} command: {}'.format(
+                params_error, function_name, ', '.join(erroneous_params)
+            )
+            log.warning(params_error_message)
+            self._render_error(
+                JSONRPCError(params_error_message, code=JSONRPCError.CODE_INVALID_PARAMS),
+                request, id_
+            )
+            return server.NOT_DONE_YET
+
+        d = defer.maybeDeferred(function, **args_dict)
+
         # finished_deferred will callback when the request is finished
         # and errback if something went wrong. If the errback is
         # called, cancel the deferred stack. This is to prevent
         # request.finish() from being called on a closed request.
         finished_deferred.addErrback(self._handle_dropped_request, d, function_name)
 
-        d.addCallback(self._callback_render, request, id_, version, reply_with_next_secret)
+        d.addCallback(self._callback_render, request, id_, reply_with_next_secret)
         # TODO: don't trap RuntimeError, which is presently caught to
         # handle deferredLists that won't peacefully cancel, namely
         # get_lbry_files
         d.addErrback(trap, ConnectionDone, ConnectionLost, defer.CancelledError, RuntimeError)
-        d.addErrback(log.fail(self._render_error, request, id_, version=version),
+        d.addErrback(log.fail(self._render_error, request, id_),
                      'Failed to process %s', function_name)
         d.addBoth(lambda _: log.debug("%s took %f",
                                       function_name,
                                       (utils.now() - time_in).total_seconds()))
         return server.NOT_DONE_YET
 
+    @staticmethod
+    def _check_params(function, args_dict):
+        argspec = inspect.getargspec(undecorated(function))
+        num_optional_params = 0 if argspec.defaults is None else len(argspec.defaults)
+        missing_required_params = [
+            required_param
+            for required_param in argspec.args[1:-num_optional_params]
+            if required_param not in args_dict
+            ]
+        if len(missing_required_params):
+            return 'Missing required parameters', missing_required_params
+
+        extraneous_params = [] if argspec.keywords is not None else [
+            extra_param
+            for extra_param in args_dict
+            if extra_param not in argspec.args[1:]
+            ]
+        if len(extraneous_params):
+            return 'Extraneous parameters', extraneous_params
+
+        return None, None
+
     def _register_user_session(self, session_id):
         """
         Add or update a HMAC secret for a session
@@ -313,38 +446,25 @@ class AuthJSONRPCServer(AuthorizedBase):
         return False
 
     def _verify_token(self, session_id, message, token):
-        assert token is not None, InvalidAuthenticationToken
+        if token is None:
+            raise InvalidAuthenticationToken('Authentication token not found')
         to_auth = get_auth_message(message)
         api_key = self.sessions.get(session_id)
-        assert api_key.compare_hmac(to_auth, token), InvalidAuthenticationToken
+        if not api_key.compare_hmac(to_auth, token):
+            raise InvalidAuthenticationToken('Invalid authentication token')
 
     def _update_session_secret(self, session_id):
         self.sessions.update({session_id: APIKey.new(name=session_id)})
 
-    @staticmethod
-    def _get_jsonrpc_version(version=None, id_=None):
-        if version:
-            return int(float(version))
-        elif id_:
-            return jsonrpclib.VERSION_1
-        else:
-            return jsonrpclib.VERSION_PRE1
-
-    def _callback_render(self, result, request, id_, version, auth_required=False):
-        result_for_return = result
-
-        if version == jsonrpclib.VERSION_PRE1:
-            if not isinstance(result, jsonrpclib.Fault):
-                result_for_return = (result_for_return,)
-
+    def _callback_render(self, result, request, id_, auth_required=False):
         try:
-            encoded_message = jsonrpc_dumps_pretty(
-                result_for_return, id=id_, version=version, default=default_decimal)
+            encoded_message = jsonrpc_dumps_pretty(result, id=id_, default=default_decimal)
+            request.setResponseCode(200)
             self._set_headers(request, encoded_message, auth_required)
             self._render_message(request, encoded_message)
         except Exception as err:
             log.exception("Failed to render API response: %s", result)
-            self._render_error(err, request, id_, version)
+            self._render_error(err, request, id_)
 
     @staticmethod
     def _render_response(result):

lbrynet/lbrynet_daemon/auth/util.py

@@ -24,10 +24,6 @@ def generate_key(x=None):
         return sha(x)
 
 
-def jsonrpc_dumps_pretty(obj, **kwargs):
-    return jsonrpclib.dumps(obj, sort_keys=True, indent=2, separators=(',', ': '), **kwargs) + "\n"
-
-
 class APIKey(object):
     def __init__(self, secret, name, expiration=None):
         self.secret = secret

lbrynet/undecorated.py

@@ -0,0 +1,68 @@
+# -*- coding: utf-8 -*-
+# Copyright 2016-2017 Ionuț Arțăriși <ionut@artarisi.eu>
+
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+
+#     http://www.apache.org/licenses/LICENSE-2.0
+
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+
+# This came from https://github.com/mapleoin/undecorated
+
+from inspect import isfunction, ismethod, isclass
+
+__version__ = '0.3.0'
+
+
+def undecorated(o):
+    """Remove all decorators from a function, method or class"""
+    # class decorator
+    if type(o) is type:
+        return o
+
+    try:
+        # python2
+        closure = o.func_closure
+    except AttributeError:
+        pass
+
+    # try:
+    #     # python3
+    #     closure = o.__closure__
+    # except AttributeError:
+    #     return
+
+    if closure:
+        for cell in closure:
+            # avoid infinite recursion
+            if cell.cell_contents is o:
+                continue
+
+            # check if the contents looks like a decorator; in that case
+            # we need to go one level down into the dream, otherwise it
+            # might just be a different closed-over variable, which we
+            # can ignore.
+
+            # Note: this favors supporting decorators defined without
+            # @wraps to the detriment of function/method/class closures
+            if looks_like_a_decorator(cell.cell_contents):
+                undecd = undecorated(cell.cell_contents)
+                if undecd:
+                    return undecd
+        else:
+            return o
+    else:
+        return o
+
+
+def looks_like_a_decorator(a):
+    return (
+        isfunction(a) or ismethod(a) or isclass(a)
+    )