diff --git a/rietveld.py b/rietveld.py index 9adc9c5ca1..2107e2e13a 100644 --- a/rietveld.py +++ b/rietveld.py @@ -416,10 +416,7 @@ class Rietveld(object): for retry in xrange(self._maxtries): try: logging.debug('%s' % request_path) - result = self.rpc_server.Send(request_path, **kwargs) - # Sometimes GAE returns a HTTP 200 but with HTTP 500 as the content. - # How nice. - return result + return self.rpc_server.Send(request_path, **kwargs) except urllib2.HTTPError, e: if retry >= (self._maxtries - 1): raise @@ -528,6 +525,11 @@ class OAuthRpcServer(object): payload: request is a POST if not None, GET otherwise timeout: in seconds extra_headers: (dict) + + Returns: the HTTP response body as a string + + Raises: + urllib2.HTTPError """ # This method signature should match upload.py:AbstractRpcServer.Send() method = 'GET' @@ -543,7 +545,6 @@ class OAuthRpcServer(object): try: if timeout: self._http.timeout = timeout - # TODO(pgervais) implement some kind of retry mechanism (see upload.py). url = self.host + request_path if kwargs: url += "?" + urllib.urlencode(kwargs) @@ -572,6 +573,10 @@ class OAuthRpcServer(object): continue break + if ret[0].status != 200: + raise urllib2.HTTPError( + request_path, int(ret[0]['status']), ret[1], None, None) + return ret[1] finally: diff --git a/tests/rietveld_test.py b/tests/rietveld_test.py index 7bcb9bcbca..36d114e509 100755 --- a/tests/rietveld_test.py +++ b/tests/rietveld_test.py @@ -5,19 +5,23 @@ """Unit tests for rietveld.py.""" +import httplib import logging import os import socket import ssl +import StringIO import sys import time import traceback import unittest +import urllib2 sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) from testing_support.patches_data import GIT, RAW from testing_support import auto_stub +from third_party import httplib2 import patch import rietveld @@ -490,6 +494,30 @@ class DefaultTimeoutTest(auto_stub.TestCase): self.rietveld.post('/api/1234', [('key', 'data')]) self.assertNotEqual(self.sleep_time, 0) + +class OAuthRpcServerTest(auto_stub.TestCase): + def setUp(self): + super(OAuthRpcServerTest, self).setUp() + self.rpc_server = rietveld.OAuthRpcServer( + 'http://www.example.com', 'foo', 'bar') + + def set_mock_response(self, status): + def MockHttpRequest(*args, **kwargs): + return (httplib2.Response({'status': status}), 'body') + self.mock(self.rpc_server._http, 'request', MockHttpRequest) + + def test_404(self): + self.set_mock_response(404) + with self.assertRaises(urllib2.HTTPError) as ctx: + self.rpc_server.Send('/foo') + self.assertEquals(404, ctx.exception.code) + + def test_200(self): + self.set_mock_response(200) + ret = self.rpc_server.Send('/foo') + self.assertEquals('body', ret) + + if __name__ == '__main__': logging.basicConfig(level=[ logging.ERROR, logging.INFO, logging.DEBUG][min(2, sys.argv.count('-v'))])