D2869: wireproto: add request IDs to frames
indygreg (Gregory Szorc)
phabricator at mercurial-scm.org
Wed Mar 21 21:42:07 EDT 2018
This revision was automatically updated to reflect the committed changes.
Closed by commit rHG2ec1fb9de638: wireproto: add request IDs to frames (authored by indygreg, committed by ).
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D2869?vs=7145&id=7233
REVISION DETAIL
https://phab.mercurial-scm.org/D2869
AFFECTED FILES
mercurial/debugcommands.py
mercurial/help/internals/wireprotocol.txt
mercurial/wireprotoframing.py
mercurial/wireprotoserver.py
tests/test-http-api-httpv2.t
tests/test-wireproto-serverreactor.py
CHANGE DETAILS
diff --git a/tests/test-wireproto-serverreactor.py b/tests/test-wireproto-serverreactor.py
--- a/tests/test-wireproto-serverreactor.py
+++ b/tests/test-wireproto-serverreactor.py
@@ -18,52 +18,53 @@
Emits a generator of results from ``onframerecv()`` calls.
"""
for frame in gen:
- frametype, frameflags, framelength = framing.parseheader(frame)
+ rid, frametype, frameflags, framelength = framing.parseheader(frame)
payload = frame[framing.FRAME_HEADER_SIZE:]
assert len(payload) == framelength
- yield reactor.onframerecv(frametype, frameflags, payload)
+ yield reactor.onframerecv(rid, frametype, frameflags, payload)
-def sendcommandframes(reactor, cmd, args, datafh=None):
+def sendcommandframes(reactor, rid, cmd, args, datafh=None):
"""Generate frames to run a command and send them to a reactor."""
- return sendframes(reactor, framing.createcommandframes(cmd, args, datafh))
+ return sendframes(reactor,
+ framing.createcommandframes(rid, cmd, args, datafh))
class FrameTests(unittest.TestCase):
def testdataexactframesize(self):
data = util.bytesio(b'x' * framing.DEFAULT_MAX_FRAME_SIZE)
- frames = list(framing.createcommandframes(b'command', {}, data))
+ frames = list(framing.createcommandframes(1, b'command', {}, data))
self.assertEqual(frames, [
- ffs(b'command-name have-data command'),
- ffs(b'command-data continuation %s' % data.getvalue()),
- ffs(b'command-data eos ')
+ ffs(b'1 command-name have-data command'),
+ ffs(b'1 command-data continuation %s' % data.getvalue()),
+ ffs(b'1 command-data eos ')
])
def testdatamultipleframes(self):
data = util.bytesio(b'x' * (framing.DEFAULT_MAX_FRAME_SIZE + 1))
- frames = list(framing.createcommandframes(b'command', {}, data))
+ frames = list(framing.createcommandframes(1, b'command', {}, data))
self.assertEqual(frames, [
- ffs(b'command-name have-data command'),
- ffs(b'command-data continuation %s' % (
+ ffs(b'1 command-name have-data command'),
+ ffs(b'1 command-data continuation %s' % (
b'x' * framing.DEFAULT_MAX_FRAME_SIZE)),
- ffs(b'command-data eos x'),
+ ffs(b'1 command-data eos x'),
])
def testargsanddata(self):
data = util.bytesio(b'x' * 100)
- frames = list(framing.createcommandframes(b'command', {
+ frames = list(framing.createcommandframes(1, b'command', {
b'key1': b'key1value',
b'key2': b'key2value',
b'key3': b'key3value',
}, data))
self.assertEqual(frames, [
- ffs(b'command-name have-args|have-data command'),
- ffs(br'command-argument 0 \x04\x00\x09\x00key1key1value'),
- ffs(br'command-argument 0 \x04\x00\x09\x00key2key2value'),
- ffs(br'command-argument eoa \x04\x00\x09\x00key3key3value'),
- ffs(b'command-data eos %s' % data.getvalue()),
+ ffs(b'1 command-name have-args|have-data command'),
+ ffs(br'1 command-argument 0 \x04\x00\x09\x00key1key1value'),
+ ffs(br'1 command-argument 0 \x04\x00\x09\x00key2key2value'),
+ ffs(br'1 command-argument eoa \x04\x00\x09\x00key3key3value'),
+ ffs(b'1 command-data eos %s' % data.getvalue()),
])
class ServerReactorTests(unittest.TestCase):
@@ -86,10 +87,11 @@
def test1framecommand(self):
"""Receiving a command in a single frame yields request to run it."""
reactor = makereactor()
- results = list(sendcommandframes(reactor, b'mycommand', {}))
+ results = list(sendcommandframes(reactor, 1, b'mycommand', {}))
self.assertEqual(len(results), 1)
self.assertaction(results[0], 'runcommand')
self.assertEqual(results[0][1], {
+ 'requestid': 1,
'command': b'mycommand',
'args': {},
'data': None,
@@ -100,50 +102,53 @@
def test1argument(self):
reactor = makereactor()
- results = list(sendcommandframes(reactor, b'mycommand',
+ results = list(sendcommandframes(reactor, 41, b'mycommand',
{b'foo': b'bar'}))
self.assertEqual(len(results), 2)
self.assertaction(results[0], 'wantframe')
self.assertaction(results[1], 'runcommand')
self.assertEqual(results[1][1], {
+ 'requestid': 41,
'command': b'mycommand',
'args': {b'foo': b'bar'},
'data': None,
})
def testmultiarguments(self):
reactor = makereactor()
- results = list(sendcommandframes(reactor, b'mycommand',
+ results = list(sendcommandframes(reactor, 1, b'mycommand',
{b'foo': b'bar', b'biz': b'baz'}))
self.assertEqual(len(results), 3)
self.assertaction(results[0], 'wantframe')
self.assertaction(results[1], 'wantframe')
self.assertaction(results[2], 'runcommand')
self.assertEqual(results[2][1], {
+ 'requestid': 1,
'command': b'mycommand',
'args': {b'foo': b'bar', b'biz': b'baz'},
'data': None,
})
def testsimplecommanddata(self):
reactor = makereactor()
- results = list(sendcommandframes(reactor, b'mycommand', {},
+ results = list(sendcommandframes(reactor, 1, b'mycommand', {},
util.bytesio(b'data!')))
self.assertEqual(len(results), 2)
self.assertaction(results[0], 'wantframe')
self.assertaction(results[1], 'runcommand')
self.assertEqual(results[1][1], {
+ 'requestid': 1,
'command': b'mycommand',
'args': {},
'data': b'data!',
})
def testmultipledataframes(self):
frames = [
- ffs(b'command-name have-data mycommand'),
- ffs(b'command-data continuation data1'),
- ffs(b'command-data continuation data2'),
- ffs(b'command-data eos data3'),
+ ffs(b'1 command-name have-data mycommand'),
+ ffs(b'1 command-data continuation data1'),
+ ffs(b'1 command-data continuation data2'),
+ ffs(b'1 command-data eos data3'),
]
reactor = makereactor()
@@ -153,25 +158,27 @@
self.assertaction(results[i], 'wantframe')
self.assertaction(results[3], 'runcommand')
self.assertEqual(results[3][1], {
+ 'requestid': 1,
'command': b'mycommand',
'args': {},
'data': b'data1data2data3',
})
def testargumentanddata(self):
frames = [
- ffs(b'command-name have-args|have-data command'),
- ffs(br'command-argument 0 \x03\x00\x03\x00keyval'),
- ffs(br'command-argument eoa \x03\x00\x03\x00foobar'),
- ffs(b'command-data continuation value1'),
- ffs(b'command-data eos value2'),
+ ffs(b'1 command-name have-args|have-data command'),
+ ffs(br'1 command-argument 0 \x03\x00\x03\x00keyval'),
+ ffs(br'1 command-argument eoa \x03\x00\x03\x00foobar'),
+ ffs(b'1 command-data continuation value1'),
+ ffs(b'1 command-data eos value2'),
]
reactor = makereactor()
results = list(sendframes(reactor, frames))
self.assertaction(results[-1], 'runcommand')
self.assertEqual(results[-1][1], {
+ 'requestid': 1,
'command': b'command',
'args': {
b'key': b'val',
@@ -183,34 +190,34 @@
def testunexpectedcommandargument(self):
"""Command argument frame when not running a command is an error."""
result = self._sendsingleframe(makereactor(),
- b'command-argument 0 ignored')
+ b'1 command-argument 0 ignored')
self.assertaction(result, 'error')
self.assertEqual(result[1], {
'message': b'expected command frame; got 2',
})
def testunexpectedcommanddata(self):
"""Command argument frame when not running a command is an error."""
result = self._sendsingleframe(makereactor(),
- b'command-data 0 ignored')
+ b'1 command-data 0 ignored')
self.assertaction(result, 'error')
self.assertEqual(result[1], {
'message': b'expected command frame; got 3',
})
def testmissingcommandframeflags(self):
"""Command name frame must have flags set."""
result = self._sendsingleframe(makereactor(),
- b'command-name 0 command')
+ b'1 command-name 0 command')
self.assertaction(result, 'error')
self.assertEqual(result[1], {
'message': b'missing frame flags on command frame',
})
def testmissingargumentframe(self):
frames = [
- ffs(b'command-name have-args command'),
- ffs(b'command-name 0 ignored'),
+ ffs(b'1 command-name have-args command'),
+ ffs(b'1 command-name 0 ignored'),
]
results = list(sendframes(makereactor(), frames))
@@ -224,8 +231,8 @@
def testincompleteargumentname(self):
"""Argument frame with incomplete name."""
frames = [
- ffs(b'command-name have-args command1'),
- ffs(br'command-argument eoa \x04\x00\xde\xadfoo'),
+ ffs(b'1 command-name have-args command1'),
+ ffs(br'1 command-argument eoa \x04\x00\xde\xadfoo'),
]
results = list(sendframes(makereactor(), frames))
@@ -239,8 +246,8 @@
def testincompleteargumentvalue(self):
"""Argument frame with incomplete value."""
frames = [
- ffs(b'command-name have-args command'),
- ffs(br'command-argument eoa \x03\x00\xaa\xaafoopartialvalue'),
+ ffs(b'1 command-name have-args command'),
+ ffs(br'1 command-argument eoa \x03\x00\xaa\xaafoopartialvalue'),
]
results = list(sendframes(makereactor(), frames))
@@ -253,8 +260,8 @@
def testmissingcommanddataframe(self):
frames = [
- ffs(b'command-name have-data command1'),
- ffs(b'command-name eos command2'),
+ ffs(b'1 command-name have-data command1'),
+ ffs(b'1 command-name eos command2'),
]
results = list(sendframes(makereactor(), frames))
self.assertEqual(len(results), 2)
@@ -266,8 +273,8 @@
def testmissingcommanddataframeflags(self):
frames = [
- ffs(b'command-name have-data command1'),
- ffs(b'command-data 0 data'),
+ ffs(b'1 command-name have-data command1'),
+ ffs(b'1 command-data 0 data'),
]
results = list(sendframes(makereactor(), frames))
self.assertEqual(len(results), 2)
@@ -280,68 +287,87 @@
def testsimpleresponse(self):
"""Bytes response to command sends result frames."""
reactor = makereactor()
- list(sendcommandframes(reactor, b'mycommand', {}))
+ list(sendcommandframes(reactor, 1, b'mycommand', {}))
- result = reactor.onbytesresponseready(b'response')
+ result = reactor.onbytesresponseready(1, b'response')
self.assertaction(result, 'sendframes')
self.assertframesequal(result[1]['framegen'], [
- b'bytes-response eos response',
+ b'1 bytes-response eos response',
])
def testmultiframeresponse(self):
"""Bytes response spanning multiple frames is handled."""
first = b'x' * framing.DEFAULT_MAX_FRAME_SIZE
second = b'y' * 100
reactor = makereactor()
- list(sendcommandframes(reactor, b'mycommand', {}))
+ list(sendcommandframes(reactor, 1, b'mycommand', {}))
- result = reactor.onbytesresponseready(first + second)
+ result = reactor.onbytesresponseready(1, first + second)
self.assertaction(result, 'sendframes')
self.assertframesequal(result[1]['framegen'], [
- b'bytes-response continuation %s' % first,
- b'bytes-response eos %s' % second,
+ b'1 bytes-response continuation %s' % first,
+ b'1 bytes-response eos %s' % second,
])
def testapplicationerror(self):
reactor = makereactor()
- list(sendcommandframes(reactor, b'mycommand', {}))
+ list(sendcommandframes(reactor, 1, b'mycommand', {}))
- result = reactor.onapplicationerror(b'some message')
+ result = reactor.onapplicationerror(1, b'some message')
self.assertaction(result, 'sendframes')
self.assertframesequal(result[1]['framegen'], [
- b'error-response application some message',
+ b'1 error-response application some message',
])
def test1commanddeferresponse(self):
"""Responses when in deferred output mode are delayed until EOF."""
reactor = makereactor(deferoutput=True)
- results = list(sendcommandframes(reactor, b'mycommand', {}))
+ results = list(sendcommandframes(reactor, 1, b'mycommand', {}))
self.assertEqual(len(results), 1)
self.assertaction(results[0], 'runcommand')
- result = reactor.onbytesresponseready(b'response')
+ result = reactor.onbytesresponseready(1, b'response')
self.assertaction(result, 'noop')
result = reactor.oninputeof()
self.assertaction(result, 'sendframes')
self.assertframesequal(result[1]['framegen'], [
- b'bytes-response eos response',
+ b'1 bytes-response eos response',
])
def testmultiplecommanddeferresponse(self):
reactor = makereactor(deferoutput=True)
- list(sendcommandframes(reactor, b'command1', {}))
- list(sendcommandframes(reactor, b'command2', {}))
+ list(sendcommandframes(reactor, 1, b'command1', {}))
+ list(sendcommandframes(reactor, 3, b'command2', {}))
- result = reactor.onbytesresponseready(b'response1')
+ result = reactor.onbytesresponseready(1, b'response1')
self.assertaction(result, 'noop')
- result = reactor.onbytesresponseready(b'response2')
+ result = reactor.onbytesresponseready(3, b'response2')
self.assertaction(result, 'noop')
result = reactor.oninputeof()
self.assertaction(result, 'sendframes')
self.assertframesequal(result[1]['framegen'], [
- b'bytes-response eos response1',
- b'bytes-response eos response2'
+ b'1 bytes-response eos response1',
+ b'3 bytes-response eos response2'
+ ])
+
+ def testrequestidtracking(self):
+ reactor = makereactor(deferoutput=True)
+ list(sendcommandframes(reactor, 1, b'command1', {}))
+ list(sendcommandframes(reactor, 3, b'command2', {}))
+ list(sendcommandframes(reactor, 5, b'command3', {}))
+
+ # Register results for commands out of order.
+ reactor.onbytesresponseready(3, b'response3')
+ reactor.onbytesresponseready(1, b'response1')
+ reactor.onbytesresponseready(5, b'response5')
+
+ result = reactor.oninputeof()
+ self.assertaction(result, 'sendframes')
+ self.assertframesequal(result[1]['framegen'], [
+ b'3 bytes-response eos response3',
+ b'1 bytes-response eos response1',
+ b'5 bytes-response eos response5',
])
if __name__ == '__main__':
diff --git a/tests/test-http-api-httpv2.t b/tests/test-http-api-httpv2.t
--- a/tests/test-http-api-httpv2.t
+++ b/tests/test-http-api-httpv2.t
@@ -1,5 +1,5 @@
$ HTTPV2=exp-http-v2-0001
- $ MEDIATYPE=application/mercurial-exp-framing-0001
+ $ MEDIATYPE=application/mercurial-exp-framing-0002
$ send() {
> hg --verbose debugwireproto --peer raw http://$LOCALIP:$HGPORT/
@@ -122,7 +122,7 @@
s> Content-Type: text/plain\r\n
s> Content-Length: 85\r\n
s> \r\n
- s> client MUST specify Accept header with value: application/mercurial-exp-framing-0001\n
+ s> client MUST specify Accept header with value: application/mercurial-exp-framing-0002\n
Bad Accept header results in 406
@@ -145,7 +145,7 @@
s> Content-Type: text/plain\r\n
s> Content-Length: 85\r\n
s> \r\n
- s> client MUST specify Accept header with value: application/mercurial-exp-framing-0001\n
+ s> client MUST specify Accept header with value: application/mercurial-exp-framing-0002\n
Bad Content-Type header results in 415
@@ -158,7 +158,7 @@
using raw connection to peer
s> POST /api/exp-http-v2-0001/ro/customreadonly HTTP/1.1\r\n
s> Accept-Encoding: identity\r\n
- s> accept: application/mercurial-exp-framing-0001\r\n
+ s> accept: application/mercurial-exp-framing-0002\r\n
s> content-type: badmedia\r\n
s> user-agent: test\r\n
s> host: $LOCALIP:$HGPORT\r\n (glob)
@@ -170,36 +170,36 @@
s> Content-Type: text/plain\r\n
s> Content-Length: 88\r\n
s> \r\n
- s> client MUST send Content-Type header with value: application/mercurial-exp-framing-0001\n
+ s> client MUST send Content-Type header with value: application/mercurial-exp-framing-0002\n
Request to read-only command works out of the box
$ send << EOF
> httprequest POST api/$HTTPV2/ro/customreadonly
> accept: $MEDIATYPE
> content-type: $MEDIATYPE
> user-agent: test
- > frame command-name eos customreadonly
+ > frame 1 command-name eos customreadonly
> EOF
using raw connection to peer
s> POST /api/exp-http-v2-0001/ro/customreadonly HTTP/1.1\r\n
s> Accept-Encoding: identity\r\n
- s> accept: application/mercurial-exp-framing-0001\r\n
- s> content-type: application/mercurial-exp-framing-0001\r\n
+ s> accept: application/mercurial-exp-framing-0002\r\n
+ s> content-type: application/mercurial-exp-framing-0002\r\n
s> user-agent: test\r\n
- s> content-length: 18\r\n
+ s> *\r\n (glob)
s> host: $LOCALIP:$HGPORT\r\n (glob)
s> \r\n
- s> \x0e\x00\x00\x11customreadonly
+ s> \x0e\x00\x00\x01\x00\x11customreadonly
s> makefile('rb', None)
s> HTTP/1.1 200 OK\r\n
s> Server: testing stub value\r\n
s> Date: $HTTP_DATE$\r\n
- s> Content-Type: application/mercurial-exp-framing-0001\r\n
+ s> Content-Type: application/mercurial-exp-framing-0002\r\n
s> Transfer-Encoding: chunked\r\n
s> \r\n
- s> 21\r\n
- s> \x1d\x00\x00Bcustomreadonly bytes response
+ s> 23\r\n
+ s> \x1d\x00\x00\x01\x00Bcustomreadonly bytes response
s> \r\n
s> 0\r\n
s> \r\n
@@ -290,27 +290,27 @@
> user-agent: test
> accept: $MEDIATYPE
> content-type: $MEDIATYPE
- > frame command-name eos customreadonly
+ > frame 1 command-name eos customreadonly
> EOF
using raw connection to peer
s> POST /api/exp-http-v2-0001/rw/customreadonly HTTP/1.1\r\n
s> Accept-Encoding: identity\r\n
- s> accept: application/mercurial-exp-framing-0001\r\n
- s> content-type: application/mercurial-exp-framing-0001\r\n
+ s> accept: application/mercurial-exp-framing-0002\r\n
+ s> content-type: application/mercurial-exp-framing-0002\r\n
s> user-agent: test\r\n
- s> content-length: 18\r\n
+ s> content-length: 20\r\n
s> host: $LOCALIP:$HGPORT\r\n (glob)
s> \r\n
- s> \x0e\x00\x00\x11customreadonly
+ s> \x0e\x00\x00\x01\x00\x11customreadonly
s> makefile('rb', None)
s> HTTP/1.1 200 OK\r\n
s> Server: testing stub value\r\n
s> Date: $HTTP_DATE$\r\n
- s> Content-Type: application/mercurial-exp-framing-0001\r\n
+ s> Content-Type: application/mercurial-exp-framing-0002\r\n
s> Transfer-Encoding: chunked\r\n
s> \r\n
- s> 21\r\n
- s> \x1d\x00\x00Bcustomreadonly bytes response
+ s> 23\r\n
+ s> \x1d\x00\x00\x01\x00Bcustomreadonly bytes response
s> \r\n
s> 0\r\n
s> \r\n
@@ -325,7 +325,7 @@
using raw connection to peer
s> POST /api/exp-http-v2-0001/rw/badcommand HTTP/1.1\r\n
s> Accept-Encoding: identity\r\n
- s> accept: application/mercurial-exp-framing-0001\r\n
+ s> accept: application/mercurial-exp-framing-0002\r\n
s> user-agent: test\r\n
s> host: $LOCALIP:$HGPORT\r\n (glob)
s> \r\n
@@ -382,33 +382,33 @@
> accept: $MEDIATYPE
> content-type: $MEDIATYPE
> user-agent: test
- > frame command-name have-args command1
- > frame command-argument 0 \x03\x00\x04\x00fooval1
- > frame command-argument eoa \x04\x00\x03\x00bar1val
+ > frame 1 command-name have-args command1
+ > frame 1 command-argument 0 \x03\x00\x04\x00fooval1
+ > frame 1 command-argument eoa \x04\x00\x03\x00bar1val
> EOF
using raw connection to peer
s> POST /api/exp-http-v2-0001/ro/debugreflect HTTP/1.1\r\n
s> Accept-Encoding: identity\r\n
- s> accept: application/mercurial-exp-framing-0001\r\n
- s> content-type: application/mercurial-exp-framing-0001\r\n
+ s> accept: application/mercurial-exp-framing-0002\r\n
+ s> content-type: application/mercurial-exp-framing-0002\r\n
s> user-agent: test\r\n
- s> content-length: 42\r\n
+ s> content-length: 48\r\n
s> host: $LOCALIP:$HGPORT\r\n (glob)
s> \r\n
- s> \x08\x00\x00\x12command1\x0b\x00\x00 \x03\x00\x04\x00fooval1\x0b\x00\x00"\x04\x00\x03\x00bar1val
+ s> \x08\x00\x00\x01\x00\x12command1\x0b\x00\x00\x01\x00 \x03\x00\x04\x00fooval1\x0b\x00\x00\x01\x00"\x04\x00\x03\x00bar1val
s> makefile('rb', None)
s> HTTP/1.1 200 OK\r\n
s> Server: testing stub value\r\n
s> Date: $HTTP_DATE$\r\n
s> Content-Type: text/plain\r\n
- s> Content-Length: 310\r\n
+ s> Content-Length: 332\r\n
s> \r\n
- s> received: 1 2 command1\n
+ s> received: 1 2 1 command1\n
s> ["wantframe", {"state": "command-receiving-args"}]\n
- s> received: 2 0 \x03\x00\x04\x00fooval1\n
+ s> received: 2 0 1 \x03\x00\x04\x00fooval1\n
s> ["wantframe", {"state": "command-receiving-args"}]\n
- s> received: 2 2 \x04\x00\x03\x00bar1val\n
- s> ["runcommand", {"args": {"bar1": "val", "foo": "val1"}, "command": "command1", "data": null}]\n
+ s> received: 2 2 1 \x04\x00\x03\x00bar1val\n
+ s> ["runcommand", {"args": {"bar1": "val", "foo": "val1"}, "command": "command1", "data": null, "requestid": 1}]\n
s> received: <no frame>\n
s> {"action": "noop"}
diff --git a/mercurial/wireprotoserver.py b/mercurial/wireprotoserver.py
--- a/mercurial/wireprotoserver.py
+++ b/mercurial/wireprotoserver.py
@@ -33,7 +33,7 @@
HGTYPE = 'application/mercurial-0.1'
HGTYPE2 = 'application/mercurial-0.2'
HGERRTYPE = 'application/hg-error'
-FRAMINGTYPE = b'application/mercurial-exp-framing-0001'
+FRAMINGTYPE = b'application/mercurial-exp-framing-0002'
HTTPV2 = wireprototypes.HTTPV2
SSHV1 = wireprototypes.SSHV1
@@ -394,10 +394,12 @@
states.append(b'received: <no frame>')
break
- frametype, frameflags, payload = frame
- states.append(b'received: %d %d %s' % (frametype, frameflags, payload))
+ requestid, frametype, frameflags, payload = frame
+ states.append(b'received: %d %d %d %s' % (frametype, frameflags,
+ requestid, payload))
- action, meta = reactor.onframerecv(frametype, frameflags, payload)
+ action, meta = reactor.onframerecv(requestid, frametype, frameflags,
+ payload)
states.append(json.dumps((action, meta), sort_keys=True,
separators=(', ', ': ')))
@@ -517,7 +519,8 @@
res.headers[b'Content-Type'] = FRAMINGTYPE
if isinstance(rsp, wireprototypes.bytesresponse):
- action, meta = reactor.onbytesresponseready(rsp.data)
+ action, meta = reactor.onbytesresponseready(command['requestid'],
+ rsp.data)
else:
action, meta = reactor.onapplicationerror(
_('unhandled response type from wire proto command'))
diff --git a/mercurial/wireprotoframing.py b/mercurial/wireprotoframing.py
--- a/mercurial/wireprotoframing.py
+++ b/mercurial/wireprotoframing.py
@@ -19,7 +19,7 @@
util,
)
-FRAME_HEADER_SIZE = 4
+FRAME_HEADER_SIZE = 6
DEFAULT_MAX_FRAME_SIZE = 32768
FRAME_TYPE_COMMAND_NAME = 0x01
@@ -89,28 +89,43 @@
ARGUMENT_FRAME_HEADER = struct.Struct(r'<HH')
-def makeframe(frametype, frameflags, payload):
+def makeframe(requestid, frametype, frameflags, payload):
"""Assemble a frame into a byte array."""
# TODO assert size of payload.
frame = bytearray(FRAME_HEADER_SIZE + len(payload))
+ # 24 bits length
+ # 16 bits request id
+ # 4 bits type
+ # 4 bits flags
+
l = struct.pack(r'<I', len(payload))
frame[0:3] = l[0:3]
- frame[3] = (frametype << 4) | frameflags
- frame[4:] = payload
+ struct.pack_into(r'<H', frame, 3, requestid)
+ frame[5] = (frametype << 4) | frameflags
+ frame[6:] = payload
return frame
def makeframefromhumanstring(s):
- """Given a string of the form: <type> <flags> <payload>, creates a frame.
+ """Create a frame from a human readable string
+
+ Strings have the form:
+
+ <request-id> <type> <flags> <payload>
This can be used by user-facing applications and tests for creating
frames easily without having to type out a bunch of constants.
+ Request ID is an integer.
+
Frame type and flags can be specified by integer or named constant.
+
Flags can be delimited by `|` to bitwise OR them together.
"""
- frametype, frameflags, payload = s.split(b' ', 2)
+ requestid, frametype, frameflags, payload = s.split(b' ', 3)
+
+ requestid = int(requestid)
if frametype in FRAME_TYPES:
frametype = FRAME_TYPES[frametype]
@@ -127,7 +142,7 @@
payload = util.unescapestr(payload)
- return makeframe(frametype, finalflags, payload)
+ return makeframe(requestid, frametype, finalflags, payload)
def parseheader(data):
"""Parse a unified framing protocol frame header from a buffer.
@@ -140,12 +155,13 @@
# 4 bits frame flags
# ... payload
framelength = data[0] + 256 * data[1] + 16384 * data[2]
- typeflags = data[3]
+ requestid = struct.unpack_from(r'<H', data, 3)[0]
+ typeflags = data[5]
frametype = (typeflags & 0xf0) >> 4
frameflags = typeflags & 0x0f
- return frametype, frameflags, framelength
+ return requestid, frametype, frameflags, framelength
def readframe(fh):
"""Read a unified framing protocol frame from a file object.
@@ -165,16 +181,16 @@
raise error.Abort(_('received incomplete frame: got %d bytes: %s') %
(readcount, header))
- frametype, frameflags, framelength = parseheader(header)
+ requestid, frametype, frameflags, framelength = parseheader(header)
payload = fh.read(framelength)
if len(payload) != framelength:
raise error.Abort(_('frame length error: expected %d; got %d') %
(framelength, len(payload)))
- return frametype, frameflags, payload
+ return requestid, frametype, frameflags, payload
-def createcommandframes(cmd, args, datafh=None):
+def createcommandframes(requestid, cmd, args, datafh=None):
"""Create frames necessary to transmit a request to run a command.
This is a generator of bytearrays. Each item represents a frame
@@ -189,7 +205,7 @@
if not flags:
flags |= FLAG_COMMAND_NAME_EOS
- yield makeframe(FRAME_TYPE_COMMAND_NAME, flags, cmd)
+ yield makeframe(requestid, FRAME_TYPE_COMMAND_NAME, flags, cmd)
for i, k in enumerate(sorted(args)):
v = args[k]
@@ -205,7 +221,7 @@
payload[offset:offset + len(v)] = v
flags = FLAG_COMMAND_ARGUMENT_EOA if last else 0
- yield makeframe(FRAME_TYPE_COMMAND_ARGUMENT, flags, payload)
+ yield makeframe(requestid, FRAME_TYPE_COMMAND_ARGUMENT, flags, payload)
if datafh:
while True:
@@ -219,21 +235,21 @@
assert datafh.read(1) == b''
done = True
- yield makeframe(FRAME_TYPE_COMMAND_DATA, flags, data)
+ yield makeframe(requestid, FRAME_TYPE_COMMAND_DATA, flags, data)
if done:
break
-def createbytesresponseframesfrombytes(data,
+def createbytesresponseframesfrombytes(requestid, data,
maxframesize=DEFAULT_MAX_FRAME_SIZE):
"""Create a raw frame to send a bytes response from static bytes input.
Returns a generator of bytearrays.
"""
# Simple case of a single frame.
if len(data) <= maxframesize:
- yield makeframe(FRAME_TYPE_BYTES_RESPONSE,
+ yield makeframe(requestid, FRAME_TYPE_BYTES_RESPONSE,
FLAG_BYTES_RESPONSE_EOS, data)
return
@@ -248,12 +264,12 @@
else:
flags = FLAG_BYTES_RESPONSE_CONTINUATION
- yield makeframe(FRAME_TYPE_BYTES_RESPONSE, flags, chunk)
+ yield makeframe(requestid, FRAME_TYPE_BYTES_RESPONSE, flags, chunk)
if done:
break
-def createerrorframe(msg, protocol=False, application=False):
+def createerrorframe(requestid, msg, protocol=False, application=False):
# TODO properly handle frame size limits.
assert len(msg) <= DEFAULT_MAX_FRAME_SIZE
@@ -263,7 +279,7 @@
if application:
flags |= FLAG_ERROR_RESPONSE_APPLICATION
- yield makeframe(FRAME_TYPE_ERROR_RESPONSE, flags, msg)
+ yield makeframe(requestid, FRAME_TYPE_ERROR_RESPONSE, flags, msg)
class serverreactor(object):
"""Holds state of a server handling frame-based protocol requests.
@@ -326,15 +342,16 @@
self._deferoutput = deferoutput
self._state = 'idle'
self._bufferedframegens = []
+ self._activerequestid = None
self._activecommand = None
self._activeargs = None
self._activedata = None
self._expectingargs = None
self._expectingdata = None
self._activeargname = None
self._activeargchunks = None
- def onframerecv(self, frametype, frameflags, payload):
+ def onframerecv(self, requestid, frametype, frameflags, payload):
"""Process a frame that has been received off the wire.
Returns a dict with an ``action`` key that details what action,
@@ -351,14 +368,14 @@
if not meth:
raise error.ProgrammingError('unhandled state: %s' % self._state)
- return meth(frametype, frameflags, payload)
+ return meth(requestid, frametype, frameflags, payload)
- def onbytesresponseready(self, data):
+ def onbytesresponseready(self, requestid, data):
"""Signal that a bytes response is ready to be sent to the client.
The raw bytes response is passed as an argument.
"""
- framegen = createbytesresponseframesfrombytes(data)
+ framegen = createbytesresponseframesfrombytes(requestid, data)
if self._deferoutput:
self._bufferedframegens.append(framegen)
@@ -387,9 +404,9 @@
'framegen': makegen(),
}
- def onapplicationerror(self, msg):
+ def onapplicationerror(self, requestid, msg):
return 'sendframes', {
- 'framegen': createerrorframe(msg, application=True),
+ 'framegen': createerrorframe(requestid, msg, application=True),
}
def _makeerrorresult(self, msg):
@@ -399,6 +416,7 @@
def _makeruncommandresult(self):
return 'runcommand', {
+ 'requestid': self._activerequestid,
'command': self._activecommand,
'args': self._activeargs,
'data': self._activedata.getvalue() if self._activedata else None,
@@ -409,14 +427,15 @@
'state': self._state,
}
- def _onframeidle(self, frametype, frameflags, payload):
+ def _onframeidle(self, requestid, frametype, frameflags, payload):
# The only frame type that should be received in this state is a
# command request.
if frametype != FRAME_TYPE_COMMAND_NAME:
self._state = 'errored'
return self._makeerrorresult(
_('expected command frame; got %d') % frametype)
+ self._activerequestid = requestid
self._activecommand = payload
self._activeargs = {}
self._activedata = None
@@ -439,7 +458,7 @@
return self._makeerrorresult(_('missing frame flags on '
'command frame'))
- def _onframereceivingargs(self, frametype, frameflags, payload):
+ def _onframereceivingargs(self, requestid, frametype, frameflags, payload):
if frametype != FRAME_TYPE_COMMAND_ARGUMENT:
self._state = 'errored'
return self._makeerrorresult(_('expected command argument '
@@ -492,7 +511,7 @@
else:
return self._makewantframeresult()
- def _onframereceivingdata(self, frametype, frameflags, payload):
+ def _onframereceivingdata(self, requestid, frametype, frameflags, payload):
if frametype != FRAME_TYPE_COMMAND_DATA:
self._state = 'errored'
return self._makeerrorresult(_('expected command data frame; '
@@ -512,5 +531,5 @@
return self._makeerrorresult(_('command data frame without '
'flags'))
- def _onframeerrored(self, frametype, frameflags, payload):
+ def _onframeerrored(self, requestid, frametype, frameflags, payload):
return self._makeerrorresult(_('server already errored'))
diff --git a/mercurial/help/internals/wireprotocol.txt b/mercurial/help/internals/wireprotocol.txt
--- a/mercurial/help/internals/wireprotocol.txt
+++ b/mercurial/help/internals/wireprotocol.txt
@@ -469,22 +469,26 @@
The protocol is request-response based: the client issues requests to
the server, which issues replies to those requests. Server-initiated
-messaging is not supported.
+messaging is not currently supported, but this specification carves
+out room to implement it.
All data is read and written in atomic units called *frames*. These
are conceptually similar to TCP packets. Higher-level functionality
is built on the exchange and processing of frames.
-Frames begin with a 4 octet header followed by a variable length
+All frames are associated with a numbered request. Frames can thus
+be logically grouped by their request ID.
+
+Frames begin with a 6 octet header followed by a variable length
payload::
+-----------------------------------------------+
| Length (24) |
- +-----------+-----------------------------------+
- | Type (4) |
- +-----------+
- | Flags (4) |
- +===========+===================================================|
+ +---------------------------------+-------------+
+ | Request ID (16) |
+ +----------+-----------+----------+
+ | Type (4) | Flags (4) |
+ +==========+===========+========================================|
| Frame Payload (0...) ...
+---------------------------------------------------------------+
@@ -494,6 +498,15 @@
during the handshake. The frame header is not part of the advertised
frame length.
+The 16-bit ``Request ID`` field denotes the integer request identifier,
+stored as an unsigned little endian integer. Odd numbered requests are
+client-initiated. Even numbered requests are server-initiated. This
+refers to where the *request* was initiated - not where the *frame* was
+initiated, so servers will send frames with odd ``Request ID`` in
+response to client-initiated requests. Implementations are advised to
+start ordering request identifiers at ``1`` and ``0``, increment by
+``2``, and wrap around if all available numbers have been exhausted.
+
The 4-bit ``Type`` field denotes the type of message being sent.
The 4-bit ``Flags`` field defines special, per-type attributes for
@@ -633,6 +646,28 @@
1 ``Command Request`` frame, 0 or more ``Command Argument`` frames,
and 0 or more ``Command Data`` frames.
+All frames composing a single command request MUST be associated with
+the same ``Request ID``.
+
+Clients MAY send additional command requests without waiting on the
+response to a previous command request. If they do so, they MUST ensure
+that the ``Request ID`` field of outbound frames does not conflict
+with that of an active ``Request ID`` whose response has not yet been
+fully received.
+
+Servers MAY respond to commands in a different order than they were
+sent over the wire. Clients MUST be prepared to deal with this. Servers
+also MAY start executing commands in a different order than they were
+received, or MAY execute multiple commands concurrently.
+
+If there is a dependency between commands or a race condition between
+commands executing (e.g. a read-only command that depends on the results
+of a command that mutates the repository), then clients MUST NOT send
+frames issuing a command until a response to all dependent commands has
+been received.
+TODO think about whether we should express dependencies between commands
+to avoid roundtrip latency.
+
Argument frames are the recommended mechanism for transferring fixed
sets of parameters to a command. Data frames are appropriate for
transferring variable data. A similar comparison would be to HTTP:
diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py
--- a/mercurial/debugcommands.py
+++ b/mercurial/debugcommands.py
@@ -2765,12 +2765,14 @@
syntax.
A frame is composed as a type, flags, and payload. These can be parsed
- from a string of the form ``<type> <flags> <payload>``. That is, 3
- space-delimited strings.
+ from a string of the form ``<requestid> <type> <flags> <payload>``. That is,
+ 4 space-delimited strings.
``payload`` is the simplest: it is evaluated as a Python byte string
literal.
+ ``requestid`` is an integer defining the request identifier.
+
``type`` can be an integer value for the frame type or the string name
of the type. The strings are defined in ``wireprotoframing.py``. e.g.
``command-name``.
To: indygreg, #hg-reviewers, durin42
Cc: mercurial-devel
More information about the Mercurial-devel
mailing list