Raise exceptions properly on HTTP errors from OAuthRpcServer (which i…

…s only used on bots)

This will hopefully make Rietveld._send retry 500s like it promises to

BUG=

Review URL: https://codereview.chromium.org/1683603002

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@298688 0039d316-1c4b-4281-b951-d872f2087c98

rietveld.py

@@ -416,10 +416,7 @@ def trap_http_500(msg):
  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 @@ def Send(self,
  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 @@ def Send(self,
  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 @@ def Send(self,
  continue
  break
 
+ if ret[0].status != 200:
+ raise urllib2.HTTPError(
+ request_path, int(ret[0]['status']), ret[1], None, None)
+
  return ret[1]
 
  finally:

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 @@ def test_socket_connect_timeout_post(self):
  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'))])