Merge pull request #84 from dahlia/rpc-client-exceptions

Make Client to process method error types properly

CHANGES.rst

@@ -7,8 +7,13 @@ Version 0.5.1
 To be released.
 
 - Added Python 3.6 support.
+- Fixed a bug that service client methods hadn't raised the proper error
+  type but ``nirum.exc.UnexpectedNirumResponseError`` instead.  [`#71`_]
 - Wheel distributions (``nirum-*.whl``) are now universal between Python 2
   and 3.  [`#78`_]
+- ``nirum.rpc.Service`` had been an old-style class on Python 2, but now
+  it became a new-style class also on Python 2.  (As Python 3 has only new-style
+  class, there's no change on Python 3.)  [`#83`_]
 - ``nirum.rpc.Client`` and its subtype became to raise ``TypeError`` with
   a better error message when its ``make_request()`` method is overridden and
   it returns a wrong artity of tuple.  [`#80`_]
@@ -17,8 +22,11 @@ To be released.
   it returns a wrong artity of tuple.  [`#80`_]
 - Fixed a bug that ``Client.ping()`` method had always raised ``TypeError``.
   [`#80`_]
+- Corrected a typo ``Accepts`` on request headers ``Client`` makes to
+  ``Accept``.
 
 .. _#78: https://github.com/spoqa/nirum-python/pull/78
+.. _#83: https://github.com/spoqa/nirum-python/issues/83
 .. _#80: https://github.com/spoqa/nirum-python/pull/80
 
 

nirum/rpc.py

@@ -32,7 +32,7 @@
 ]
 
 
-class Service:
+class Service(object):
     """Nirum RPC service."""
 
     __nirum_service_methods__ = {}
@@ -320,16 +320,37 @@ def __init__(self, url, opener=urllib.request.build_opener()):
         self.opener = opener
 
     def ping(self):
-        r = self.do_request(urllib.parse.urljoin(self.url, './ping/'), {})
-        return json.loads(r) == 'Ok'
+        status, _, __ = self.do_request(
+            urllib.parse.urljoin(self.url, './ping/'),
+            {}
+        )
+        return 200 <= status < 300
 
     def remote_call(self, method_name, payload={}):
         qs = urllib.parse.urlencode({'method': method_name})
         scheme, netloc, path, _, _ = urllib.parse.urlsplit(self.url)
         request_url = urllib.parse.urlunsplit((
             scheme, netloc, path, qs, ''
         ))
-        return self.do_request(request_url, payload)
+        status, headers, content = self.do_request(request_url, payload)
+        content_type = headers.get('Content-Type', '').split(';', 1)[0].strip()
+        if content_type == 'application/json':
+            text = content.decode('utf-8')
+            if 200 <= status < 300:
+                return text
+            elif 400 <= status < 500:
+                error_types = getattr(type(self),
+                                      '__nirum_method_error_types__',
+                                      {})
+                try:
+                    error_type = error_types[method_name]
+                except KeyError:
+                    pass
+                else:
+                    error_data = json.loads(text)
+                    raise deserialize_meta(error_type, error_data)
+            raise UnexpectedNirumResponseError(text)
+        raise UnexpectedNirumResponseError(repr(text))
 
     def make_request(self, method, request_url, headers, payload):
         return (
@@ -342,7 +363,7 @@ def do_request(self, request_url, payload):
             request_url,
             [
                 ('Content-type', 'application/json;charset=utf-8'),
-                ('Accepts', 'application/json'),
+                ('Accept', 'application/json'),
             ],
             payload
         )
@@ -394,11 +415,7 @@ def do_request(self, request_url, payload):
         for header_name, header_content in headers:
             request.add_header(header_name, header_content)
         response = self.opener.open(request, None)
-        response_text = response.read().decode('utf-8')
-        if 200 <= response.code < 300:
-            return response_text
-        else:
-            raise UnexpectedNirumResponseError(response_text)
+        return response.code, response.headers, response.read()
 
 
 # To eliminate imported vars from being overridden by

nirum/test.py

@@ -1,3 +1,4 @@
+import email
 import socket
 
 from six import PY3
@@ -18,6 +19,9 @@ class MockHttpResponse(HTTPResponse):
     def __init__(self, body, status_code):
         self.body = body
         self.code = status_code
+        self.headers = email.message_from_string(
+            'Content-Type: application/json\n'
+        )
         if PY3:
             self.status = status_code
 

tests/rpc_test.py

@@ -1,4 +1,3 @@
-import contextlib
 import json
 
 from pytest import fixture, raises, mark
@@ -8,8 +7,7 @@
 
 from .nirum_schema import import_nirum_fixture
 from nirum.exc import (InvalidNirumServiceMethodTypeError,
-                       InvalidNirumServiceMethodNameError,
-                       UnexpectedNirumResponseError)
+                       InvalidNirumServiceMethodNameError)
 from nirum.rpc import Client, WsgiApp
 from nirum.test import MockOpener
 
@@ -354,21 +352,10 @@ def make_request(self, method, request_url, headers, payload):
     )
 
 
-@contextlib.contextmanager
-def assert_error(error_type):
-    try:
-        yield
-    except UnexpectedNirumResponseError as e:
-        response_json = json.loads(str(e))
-        assert response_json == error_type().__nirum_serialize__()
-    else:
-        assert False  # MUST error raised
-
-
 def test_rpc_error_types():
     url = u'http://foobar.com/rpc/'
     client = nf.MusicServiceClient(url, MockOpener(url, MusicServiceImpl))
-    with assert_error(nf.Unknown):
+    with raises(nf.Unknown):
         client.get_music_by_artist_name('error')
-    with assert_error(nf.BadRequest):
+    with raises(nf.BadRequest):
         client.get_music_by_artist_name('adele')