mirror of
https://github.com/facebook/sapling.git
synced 2024-10-06 23:07:18 +03:00
fbconduit: fix error handling in gitnode() revset
Summary: Don't let errors propagate out of the gitnode() revset. Always report errors in gitnode() as a translation failure, rather than letting exceptions propagate up and crash mercurial. The code was previously only catching internally generated ConduitError exceptions, but it can also throw HttpError exceptions, and the underlying httplib code can throw its own exceptions as well as socket.error exceptions. This also fixes the test code to use an ephemeral TCP port rather than assuming port 8543 will always be available. Using a fixed TCP port in test code is a very common way to cause bogus failures if the tests are run in parallel. (For instance, testing multiple repositories in parallel on the same build host.) Test Plan: Added unit tests that check the behavior when the server returns a 500 error, and when the server refuses the connection entirely. Reviewers: quark, durham, rmcelroy Reviewed By: rmcelroy Subscribers: net-systems-diffs@fb.com, yogeshwer, mjpieters Differential Revision: https://phabricator.intern.facebook.com/D4556871 Signature: t1:4556871:1487062142:b58d770d46c975d44933bec08cfce8acb25ff16b
This commit is contained in:
Adam Simpkins
parent
f5752faf6f
commit
33b2fbc0bc
@ -191,7 +191,8 @@ def gitnode(repo, subset, x):
|
||||
backingrepos = repo.ui.configlist('fbconduit', 'backingrepos',
|
||||
default=[reponame])
|
||||
|
||||
translationerror = False
|
||||
lasterror = None
|
||||
hghash = None
|
||||
for backingrepo in backingrepos:
|
||||
try:
|
||||
result = call_conduit('scmquery.get.mirrored.revs',
|
||||
@ -204,16 +205,18 @@ def gitnode(repo, subset, x):
|
||||
hghash = result[n]
|
||||
if hghash != '':
|
||||
break
|
||||
except ConduitError:
|
||||
pass
|
||||
else:
|
||||
translationerror = True
|
||||
except Exception as ex:
|
||||
lasterror = ex
|
||||
|
||||
if translationerror or result[n] == "":
|
||||
repo.ui.warn(("Could not translate revision {0}.\n".format(n)))
|
||||
if not hghash:
|
||||
if lasterror:
|
||||
repo.ui.warn(("Could not translate revision {0}: {1}\n".format(
|
||||
n, lasterror)))
|
||||
else:
|
||||
repo.ui.warn(("Could not translate revision {0}\n".format(n)))
|
||||
return subset.filter(lambda r: False)
|
||||
|
||||
rn = repo[node.bin(result[n])].rev()
|
||||
rn = repo[node.bin(hghash)].rev()
|
||||
return subset.filter(lambda r: r == rn)
|
||||
|
||||
def overridestringset(orig, repo, subset, x):
|
||||
|
||||
@ -15,6 +15,7 @@ except ImportError:
|
||||
from mercurial.cmdutil import service as runservice
|
||||
|
||||
known_translations = {}
|
||||
next_error_message = []
|
||||
|
||||
class RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
|
||||
def handle_request(self, param):
|
||||
@ -24,6 +25,11 @@ class RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
|
||||
to_scm = param['to_scm']
|
||||
revs = param['revs']
|
||||
|
||||
if next_error_message:
|
||||
self.send_response(500, next_error_message[0])
|
||||
self.end_headers()
|
||||
del next_error_message[0]
|
||||
|
||||
translations = known_translations.get(
|
||||
(from_repo, from_scm, to_repo, to_scm), {})
|
||||
translated_revs = {}
|
||||
@ -89,6 +95,12 @@ class RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
|
||||
if len(path_comps) == 6:
|
||||
self.update("PUT", path_comps)
|
||||
return
|
||||
elif len(path_comps) == 2 and path_comps[0] == 'fail_next':
|
||||
# This allows tests to ask us to fail the next HTTP request
|
||||
next_error_message.append(path_comps[1])
|
||||
self.send_response(200)
|
||||
self.end_headers()
|
||||
return
|
||||
|
||||
self.send_response(500)
|
||||
self.end_headers()
|
||||
@ -111,17 +123,23 @@ class RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
|
||||
|
||||
|
||||
class simplehttpservice(object):
|
||||
def __init__(self, host, port):
|
||||
def __init__(self, host, port, port_file):
|
||||
self.address = (host, port)
|
||||
self.port_file = port_file
|
||||
def init(self):
|
||||
self.httpd = BaseHTTPServer.HTTPServer(self.address, RequestHandler)
|
||||
if self.port_file:
|
||||
with open(self.port_file, 'w') as f:
|
||||
f.write('%d\n' % self.httpd.server_port)
|
||||
def run(self):
|
||||
self.httpd.serve_forever()
|
||||
|
||||
if __name__ == '__main__':
|
||||
parser = OptionParser()
|
||||
parser.add_option('-p', '--port', dest='port', type='int', default=8000,
|
||||
parser.add_option('-p', '--port', dest='port', type='int', default=0,
|
||||
help='TCP port to listen on', metavar='PORT')
|
||||
parser.add_option('--port-file', dest='port_file',
|
||||
help='file name where the server port should be written')
|
||||
parser.add_option('-H', '--host', dest='host', default='localhost',
|
||||
help='hostname or IP to listen on', metavar='HOST')
|
||||
parser.add_option('--pid', dest='pid',
|
||||
@ -141,6 +159,6 @@ if __name__ == '__main__':
|
||||
opts = {'pid_file': options.pid,
|
||||
'daemon': not options.foreground,
|
||||
'daemon_postexec': options.daemon_postexec}
|
||||
service = simplehttpservice(options.host, options.port)
|
||||
service = simplehttpservice(options.host, options.port, options.port_file)
|
||||
runservice(opts, initfn=service.init, runfn=service.run,
|
||||
runargs=[sys.executable, __file__] + sys.argv[1:])
|
||||
|
||||
@ -1,7 +1,8 @@
|
||||
Start up translation service.
|
||||
|
||||
$ python "$TESTDIR/conduithttp.py" -p 8543 --pid conduit.pid
|
||||
$ python "$TESTDIR/conduithttp.py" --port-file conduit.port --pid conduit.pid
|
||||
$ cat conduit.pid >> $DAEMON_PIDS
|
||||
$ CONDUIT_PORT=`cat conduit.port`
|
||||
|
||||
Basic functionality.
|
||||
|
||||
@ -11,7 +12,7 @@ Basic functionality.
|
||||
$ echo "fbconduit = $TESTDIR/../hgext3rd/fbconduit.py" >> .hg/hgrc
|
||||
$ echo "[fbconduit]" >> .hg/hgrc
|
||||
$ echo "reponame = basic" >> .hg/hgrc
|
||||
$ echo "host = localhost:8543" >> .hg/hgrc
|
||||
$ echo "host = localhost:$CONDUIT_PORT" >> .hg/hgrc
|
||||
$ echo "path = /intern/conduit/" >> .hg/hgrc
|
||||
$ echo "protocol = http" >> .hg/hgrc
|
||||
$ touch file
|
||||
@ -19,7 +20,7 @@ Basic functionality.
|
||||
$ hg ci -m "initial commit"
|
||||
$ commitid=`hg log -T "{label('custom.fullrev',node)}"`
|
||||
$ hg phase -p $commitid
|
||||
$ curl -s -X PUT http://localhost:8543/basic/hg/basic/git/$commitid/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
|
||||
$ curl -s -X PUT http://localhost:$CONDUIT_PORT/basic/hg/basic/git/$commitid/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
|
||||
$ hg log -T '{gitnode}\n'
|
||||
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
|
||||
$ hg log -T '{mirrornode("git")}\n'
|
||||
@ -27,10 +28,17 @@ Basic functionality.
|
||||
$ hg log -T '{mirrornode("basic", "git")}\n'
|
||||
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
|
||||
$ hg log -r 'gitnode("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")'
|
||||
Could not translate revision aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.
|
||||
$ curl -s -X PUT http://localhost:8543/basic/git/basic/hg/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/$commitid
|
||||
Could not translate revision aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
|
||||
$ curl -s -X PUT http://localhost:$CONDUIT_PORT/basic/git/basic/hg/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/$commitid
|
||||
$ hg log -r 'gitnode("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")' -T '{desc}\n'
|
||||
initial commit
|
||||
|
||||
Make sure that we fail gracefully if the translation server returns an
|
||||
HTTP error code.
|
||||
|
||||
$ curl -s -X PUT http://localhost:$CONDUIT_PORT/fail_next/whoops
|
||||
$ hg log -r 'gitnode("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")' -T '{desc}\n'
|
||||
Could not translate revision aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: whoops
|
||||
$ cd ..
|
||||
|
||||
Test with one backing repos specified.
|
||||
@ -42,7 +50,7 @@ Test with one backing repos specified.
|
||||
$ echo "[fbconduit]" >> .hg/hgrc
|
||||
$ echo "reponame = single" >> .hg/hgrc
|
||||
$ echo "backingrepos = single_src" >> .hg/hgrc
|
||||
$ echo "host = localhost:8543" >> .hg/hgrc
|
||||
$ echo "host = localhost:$CONDUIT_PORT" >> .hg/hgrc
|
||||
$ echo "path = /intern/conduit/" >> .hg/hgrc
|
||||
$ echo "protocol = http" >> .hg/hgrc
|
||||
$ touch file
|
||||
@ -50,12 +58,12 @@ Test with one backing repos specified.
|
||||
$ hg ci -m "initial commit"
|
||||
$ commitid=`hg log -T "{label('custom.fullrev',node)}"`
|
||||
$ hg phase -p $commitid
|
||||
$ curl -s -X PUT http://localhost:8543/single/hg/single_src/git/$commitid/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
|
||||
$ curl -s -X PUT http://localhost:$CONDUIT_PORT/single/hg/single_src/git/$commitid/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
|
||||
$ hg log -T '{gitnode}\n'
|
||||
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
|
||||
$ hg log -r 'gitnode("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")'
|
||||
Could not translate revision aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.
|
||||
$ curl -s -X PUT http://localhost:8543/single_src/git/single/hg/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/$commitid
|
||||
Could not translate revision aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
|
||||
$ curl -s -X PUT http://localhost:$CONDUIT_PORT/single_src/git/single/hg/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/$commitid
|
||||
$ hg log -r 'gitnode("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")' -T '{desc}\n'
|
||||
initial commit
|
||||
$ cd ..
|
||||
@ -69,7 +77,7 @@ Test with multiple backing repos specified.
|
||||
$ echo "[fbconduit]" >> .hg/hgrc
|
||||
$ echo "reponame = multiple" >> .hg/hgrc
|
||||
$ echo "backingrepos = src_a src_b src_c" >> .hg/hgrc
|
||||
$ echo "host = localhost:8543" >> .hg/hgrc
|
||||
$ echo "host = localhost:$CONDUIT_PORT" >> .hg/hgrc
|
||||
$ echo "path = /intern/conduit/" >> .hg/hgrc
|
||||
$ echo "protocol = http" >> .hg/hgrc
|
||||
$ touch file_a
|
||||
@ -87,10 +95,10 @@ Test with multiple backing repos specified.
|
||||
$ hg phase -p $commit_a_id
|
||||
$ hg phase -p $commit_b_id
|
||||
$ hg phase -p $commit_c_id
|
||||
$ curl -s -X PUT http://localhost:8543/multiple/hg/src_a/git/$commit_a_id/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
|
||||
$ curl -s -X PUT http://localhost:8543/multiple/hg/src_b/git/$commit_b_id/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
|
||||
$ curl -s -X PUT http://localhost:8543/multiple/hg/src_c/git/$commit_c_id/cccccccccccccccccccccccccccccccccccccccc
|
||||
$ curl -s -X PUT http://localhost:8543/multiple/hg/src_b/git/$commit_c_id/dddddddddddddddddddddddddddddddddddddddd
|
||||
$ curl -s -X PUT http://localhost:$CONDUIT_PORT/multiple/hg/src_a/git/$commit_a_id/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
|
||||
$ curl -s -X PUT http://localhost:$CONDUIT_PORT/multiple/hg/src_b/git/$commit_b_id/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
|
||||
$ curl -s -X PUT http://localhost:$CONDUIT_PORT/multiple/hg/src_c/git/$commit_c_id/cccccccccccccccccccccccccccccccccccccccc
|
||||
$ curl -s -X PUT http://localhost:$CONDUIT_PORT/multiple/hg/src_b/git/$commit_c_id/dddddddddddddddddddddddddddddddddddddddd
|
||||
$ hg log -T '{gitnode}\n' -r ".^^"
|
||||
src_a: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
|
||||
$ hg log -T '{gitnode}\n' -r ".^"
|
||||
@ -98,11 +106,11 @@ Test with multiple backing repos specified.
|
||||
$ hg log -T '{gitnode}\n' -r .
|
||||
src_b: dddddddddddddddddddddddddddddddddddddddd; src_c: cccccccccccccccccccccccccccccccccccccccc
|
||||
$ hg log -r 'gitnode("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")'
|
||||
Could not translate revision aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.
|
||||
$ curl -s -X PUT http://localhost:8543/src_a/git/multiple/hg/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/$commit_a_id
|
||||
$ curl -s -X PUT http://localhost:8543/src_b/git/multiple/hg/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/$commit_b_id
|
||||
$ curl -s -X PUT http://localhost:8543/src_c/git/multiple/hg/cccccccccccccccccccccccccccccccccccccccc/$commit_c_id
|
||||
$ curl -s -X PUT http://localhost:8543/src_b/git/multiple/hg/dddddddddddddddddddddddddddddddddddddddd/$commit_c_id
|
||||
Could not translate revision aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
|
||||
$ curl -s -X PUT http://localhost:$CONDUIT_PORT/src_a/git/multiple/hg/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/$commit_a_id
|
||||
$ curl -s -X PUT http://localhost:$CONDUIT_PORT/src_b/git/multiple/hg/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/$commit_b_id
|
||||
$ curl -s -X PUT http://localhost:$CONDUIT_PORT/src_c/git/multiple/hg/cccccccccccccccccccccccccccccccccccccccc/$commit_c_id
|
||||
$ curl -s -X PUT http://localhost:$CONDUIT_PORT/src_b/git/multiple/hg/dddddddddddddddddddddddddddddddddddddddd/$commit_c_id
|
||||
$ hg log -r 'gitnode("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")' -T '{desc}\n'
|
||||
commit 1
|
||||
$ hg log -r 'gitnode("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb")' -T '{desc}\n'
|
||||
@ -119,9 +127,31 @@ Test with multiple backing repos specified.
|
||||
commit 3
|
||||
$ hg log -r gdddddddddddddddddddddddddddddddddddddddd -T '{desc}\n'
|
||||
commit 3
|
||||
$ cd ..
|
||||
|
||||
Test with a bad server port, where we get connection refused errors.
|
||||
|
||||
$ hg init errortest
|
||||
$ cd errortest
|
||||
$ echo "[extensions]" >> .hg/hgrc
|
||||
$ echo "fbconduit = $TESTDIR/../hgext3rd/fbconduit.py" >> .hg/hgrc
|
||||
$ echo "[fbconduit]" >> .hg/hgrc
|
||||
$ echo "reponame = errortest" >> .hg/hgrc
|
||||
$ echo "host = localhost:9" >> .hg/hgrc
|
||||
$ echo "path = /intern/conduit/" >> .hg/hgrc
|
||||
$ echo "protocol = http" >> .hg/hgrc
|
||||
$ touch file
|
||||
$ hg add file
|
||||
$ hg ci -m "initial commit"
|
||||
$ commitid=`hg log -T "{label('custom.fullrev',node)}"`
|
||||
$ hg phase -p $commitid
|
||||
$ hg log -r 'gitnode("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")'
|
||||
Could not translate revision aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: * (glob)
|
||||
$ cd ..
|
||||
|
||||
Make sure the template keywords are documented correctly
|
||||
|
||||
$ cd basic
|
||||
$ hg help templates | grep gitnode
|
||||
gitnode Return the git revision corresponding to a given hg rev
|
||||
$ cd ..
|
||||
|
||||
Loading…
Reference in New Issue
Block a user