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