D3386: wireprotov2: change behavior of error frame
indygreg (Gregory Szorc)
phabricator at mercurial-scm.org
Mon Apr 16 19:10:54 EDT 2018
This revision was automatically updated to reflect the committed changes.
Closed by commit rHG0c184ca594bb: wireprotov2: change behavior of error frame (authored by indygreg, committed by ).
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D3386?vs=8298&id=8329
REVISION DETAIL
https://phab.mercurial-scm.org/D3386
AFFECTED FILES
mercurial/help/internals/wireprotocol.txt
mercurial/wireprotoframing.py
mercurial/wireprotov2server.py
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
@@ -373,16 +373,18 @@
b'1 2 0 command-response eos %s' % second,
])
- def testapplicationerror(self):
+ def testservererror(self):
reactor = makereactor()
instream = framing.stream(1)
list(sendcommandframes(reactor, instream, 1, b'mycommand', {}))
outstream = reactor.makeoutputstream()
- result = reactor.onapplicationerror(outstream, 1, b'some message')
+ result = reactor.onservererror(outstream, 1, b'some message')
self.assertaction(result, b'sendframes')
self.assertframesequal(result[1][b'framegen'], [
- b'1 2 stream-begin error-response application some message',
+ b"1 2 stream-begin error-response 0 "
+ b"cbor:{b'type': b'server', "
+ b"b'message': [{b'msg': b'some message'}]}",
])
def test1commanddeferresponse(self):
diff --git a/mercurial/wireprotov2server.py b/mercurial/wireprotov2server.py
--- a/mercurial/wireprotov2server.py
+++ b/mercurial/wireprotov2server.py
@@ -311,7 +311,7 @@
command['requestid'],
encoded)
else:
- action, meta = reactor.onapplicationerror(
+ action, meta = reactor.onservererror(
_('unhandled response type from wire proto command'))
if action == 'sendframes':
diff --git a/mercurial/wireprotoframing.py b/mercurial/wireprotoframing.py
--- a/mercurial/wireprotoframing.py
+++ b/mercurial/wireprotoframing.py
@@ -87,20 +87,12 @@
b'eos': FLAG_COMMAND_RESPONSE_EOS,
}
-FLAG_ERROR_RESPONSE_PROTOCOL = 0x01
-FLAG_ERROR_RESPONSE_APPLICATION = 0x02
-
-FLAGS_ERROR_RESPONSE = {
- b'protocol': FLAG_ERROR_RESPONSE_PROTOCOL,
- b'application': FLAG_ERROR_RESPONSE_APPLICATION,
-}
-
# Maps frame types to their available flags.
FRAME_TYPE_FLAGS = {
FRAME_TYPE_COMMAND_REQUEST: FLAGS_COMMAND_REQUEST,
FRAME_TYPE_COMMAND_DATA: FLAGS_COMMAND_DATA,
FRAME_TYPE_COMMAND_RESPONSE: FLAGS_COMMAND_RESPONSE,
- FRAME_TYPE_ERROR_RESPONSE: FLAGS_ERROR_RESPONSE,
+ FRAME_TYPE_ERROR_RESPONSE: {},
FRAME_TYPE_TEXT_OUTPUT: {},
FRAME_TYPE_PROGRESS: {},
FRAME_TYPE_STREAM_SETTINGS: {},
@@ -394,20 +386,19 @@
if done:
break
-def createerrorframe(stream, requestid, msg, protocol=False, application=False):
+def createerrorframe(stream, requestid, msg, errtype):
# TODO properly handle frame size limits.
assert len(msg) <= DEFAULT_MAX_FRAME_SIZE
- flags = 0
- if protocol:
- flags |= FLAG_ERROR_RESPONSE_PROTOCOL
- if application:
- flags |= FLAG_ERROR_RESPONSE_APPLICATION
+ payload = cbor.dumps({
+ b'type': errtype,
+ b'message': [{b'msg': msg}],
+ }, canonical=True)
yield stream.makeframe(requestid=requestid,
typeid=FRAME_TYPE_ERROR_RESPONSE,
- flags=flags,
- payload=msg)
+ flags=0,
+ payload=payload)
def createtextoutputframe(stream, requestid, atoms,
maxframesize=DEFAULT_MAX_FRAME_SIZE):
@@ -664,12 +655,12 @@
'framegen': makegen(),
}
- def onapplicationerror(self, stream, requestid, msg):
+ def onservererror(self, stream, requestid, msg):
ensureserverstream(stream)
return 'sendframes', {
'framegen': createerrorframe(stream, requestid, msg,
- application=True),
+ errtype='server'),
}
def makeoutputstream(self):
@@ -1051,6 +1042,7 @@
handlers = {
FRAME_TYPE_COMMAND_RESPONSE: self._oncommandresponseframe,
+ FRAME_TYPE_ERROR_RESPONSE: self._onerrorresponseframe,
}
meth = handlers.get(frame.typeid)
@@ -1071,3 +1063,16 @@
'eos': frame.flags & FLAG_COMMAND_RESPONSE_EOS,
'data': frame.payload,
}
+
+ def _onerrorresponseframe(self, request, frame):
+ request.state = 'errored'
+ del self._activerequests[request.requestid]
+
+ # The payload should be a CBOR map.
+ m = cbor.loads(frame.payload)
+
+ return 'error', {
+ 'request': request,
+ 'type': m['type'],
+ 'message': m['message'],
+ }
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
@@ -688,23 +688,44 @@
The ``0x01`` flag is mutually exclusive with the ``0x02`` flag.
-Error Response (``0x05``)
+Error Occurred (``0x05``)
-------------------------
-An error occurred when processing a request. This could indicate
-a protocol-level failure or an application level failure depending
-on the flags for this message type.
+Some kind of error occurred.
+
+There are 3 general kinds of failures that can occur:
-The payload for this type is an error message that should be
-displayed to the user.
+* Command error encountered before any response issued
+* Command error encountered after a response was issued
+* Protocol or stream level error
+
+This frame type is used to capture the latter cases. (The general
+command error case is handled by the leading CBOR map in
+``Command Response`` frames.)
+
+The payload of this frame contains a CBOR map detailing the error. That
+map has the following bytestring keys:
-The following flag values are defined for this type:
+type
+ (bytestring) The overall type of error encountered. Can be one of the
+ following values:
+
+ protocol
+ A protocol-level error occurred. This typically means someone
+ is violating the framing protocol semantics and the server is
+ refusing to proceed.
-0x01
- The error occurred at the transport/protocol level. If set, the
- connection should be closed.
-0x02
- The error occurred at the application level. e.g. invalid command.
+ server
+ A server-level error occurred. This typically indicates some kind of
+ logic error on the server, likely the fault of the server.
+
+ command
+ A command-level error, likely the fault of the client.
+
+message
+ (array of maps) A richly formatted message that is intended for
+ human consumption. See the ``Human Output Side-Channel`` frame
+ section for a description of the format of this data structure.
Human Output Side-Channel (``0x06``)
------------------------------------
To: indygreg, #hg-reviewers, durin42
Cc: mercurial-devel
More information about the Mercurial-devel
mailing list