Gregory Szorc <gregory.szorc@gmail.com> [Sat, 14 Apr 2018 15:38:11 -0700] rev 37727
wireprotov2: remove support for sending bytes response
We recently declared that all responses must be CBOR. So remove
support for sending a type that isn't CBOR data.
Differential Revision: https://phab.mercurial-scm.org/D3387
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 14 Apr 2018 15:36:12 -0700] rev 37726
wireprotov2: change behavior of error frame
Now that we have a leading CBOR map in command response frames
to indicate overall command result status, we don't need to use
the error response frame to represent command errors. Instead,
we can reserve it for protocol and server level errors. And for the
special case of a command error that occurred after command response
frames were emitted.
The code for error handling still needs a ton of work. But we're
slowly going in the right direction...
Differential Revision: https://phab.mercurial-scm.org/D3386
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 14 Apr 2018 15:19:36 -0700] rev 37725
wireprotov2: change command response protocol to include a leading map
The error handling mechanism for the new wire protocol isn't very
well-defined. This commit takes us a step in the right direction
by introducing a leading CBOR map for command responses. This map
will contain an overall result of the command.
Currently, the map indicates whether the command was overall
successful or if an error occurred. And if an error occurred, that
error is present in the map.
There is still a dedicated error frame. My intent is to use that
for protocol-level errors and for errors that are encountered after
the initial response frame has been sent. This will be clarified in a
later commit.
Differential Revision: https://phab.mercurial-scm.org/D3385
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 14 Apr 2018 14:37:23 -0700] rev 37724
wireprotov2: change frame type and name for command response
There was hole at frame type value 3. And the frame is better
named as a command response.
Differential Revision: https://phab.mercurial-scm.org/D3384
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 14 Apr 2018 12:11:24 -0700] rev 37723
wireprotov2: change frame type value for command data
When we dropped the dedicated command argument frame type, this left
a hole in our frame type numbering. Let's start plugging that hole.
The command data frame is now type value 2 instead of 3.
There was limited test fallout because a) we do a good job of using
the constants to refer to frame types b) not many tests are sending
command data frames.
Bumping the media type will be performed in a later commit, once all
type value adjustment has been performed.
Differential Revision: https://phab.mercurial-scm.org/D3383
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 14 Apr 2018 12:07:31 -0700] rev 37722
wireprotov2: define response data as CBOR
Previously, response data was defined as a stream of bytes. We had
the option to declare it as CBOR using a frame flag.
We've converged all wire protocol commands exposed on version 2 to
CBOR. I think consistency is important. The overhead to encoding
things with CBOR is minimal. Even a very large bytestring can be
efficiently encoded using an indefinite length bytestring. Now,
there are limitations with consumers not being able to efficiently
stream large CBOR values. But these feel like solvable problems.
This commit removes the "is CBOR" frame flag from command response
frames and defines the frame as always consisting of a stream of
CBOR values.
The framing protocol media type has been bumped to reflect this
BC change.
Differential Revision: https://phab.mercurial-scm.org/D3382
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 14 Apr 2018 11:49:06 -0700] rev 37721
wireprotov2: decode responses to their expected types
Callers of established wire protocol commands expect the
response from that command to be decoded into a data structure.
It's not very useful if callers get back a stream of bytes and
don't know how they should be interpreted - especially since that
stream of bytes varies by wire protocol and even the transport
within that protocol version.
This commit establishes decoding functions for various command
responses so callers of those commands get the response type
they expect.
In theory, this should make the version 2 HTTP peer usable for
various operations. But I haven't tested to confirm.
Differential Revision: https://phab.mercurial-scm.org/D3381
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 14 Apr 2018 11:46:08 -0700] rev 37720
wireprotov2: establish a type for representing command response
It will be desirable to have a higher-level type for representing
command responses. This will allow us to do nicer things.
For now, the instance encapsulates existing logic. It is still
a bit primitive. But we're slowly making things better.
Version 1 protocols have a wrapping layer that decodes the raw
string data into a data structure and that data structure is
sent to the future. Version 2 doesn't yet have this layer and
the future is receiving the raw wire response. Hence why
debugcommands needed to be taught about the response type.
Differential Revision: https://phab.mercurial-scm.org/D3380
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 14 Apr 2018 11:50:19 -0700] rev 37719
wireprotov2: move response handling out of httppeer
And fix some bugs while we're here.
The code for processing response data from the unified framing
protocol is mostly peer agnostic. The peer-specific bits are the
configuration of the client reactor and how I/O is performed. I
initially implemented things in httppeer for expediency.
This commit establishes a module for holding the peer API level
code for the framing based protocol. Inside this module we have
a class to help coordinate higher-level activities, such as managing
response object.
The client handler bits could be rolled into clientreactor. However,
I want clientreactor to be sans I/O and I want it to only be
concerned with protocol-level details, not higher-level concepts
like how protocol events are converted into peer API concepts. I
want clientreactor to receive a frame and then tell the caller what
should probably be done about it. If we start putting things like
future resolution into clientreactor, we'll constrain how the protocol
can be used (e.g. by requiring futures).
The new code is loosely based on what was in httppeer before. I
changed things a bit around response handling. We now buffer the
entire response "body" and then handle it as one atomic unit. This
fixed a bug around decoding CBOR data that spanned multiple frames.
I also fixed an off-by-one bug where we failed to read a single byte
CBOR value at the end of the stream. That's why tests have changed.
The new state of httppeer is much cleaner. It is largely agnostic
about framing protocol implementation details. That's how it should
be: the framing protocol is designed to be largely transport
agnostic. We want peers merely putting bytes on the wire and telling
the framing protocol where to read response data from.
There's still a bit of work to be done here, especially for
representing responses. But at least we're a step closer to having a
higher-level peer interface that can be plugged into the SSH peer
someday.
I initially added this class to wireprotoframing. However, we'll
eventually need version 2 specific functions to convert CBOR responses
into data structures expected by the code calling commands. This
needs to live somewhere. Since that code would be shared across peers,
we need a common module. We have wireprotov1peer for the equivalent
version 1 code. So I decided to establish wireprotov2peer.
Differential Revision: https://phab.mercurial-scm.org/D3379
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 14 Apr 2018 11:49:57 -0700] rev 37718
debugcommands: ability to suppress logging of handshake
The tests for calling wire protocol commands were getting quite
verbose because they included the results of the capabilities
request. Furthermore, it was annoying to have to update several
tests every time the capabilities response changed.
The only tests that really care about the low-level details of
the capabilities requests are those testing the protocol
handshake. And those are mostly not instantiating peer instances
or are contained to limited files.
This commit adds an option to `hg debugwireproto` to suppress logging
of the handshake. The shell helper function to perform HTTP tests
has been updated to use this by default. Lots of excessive test
output has gone away.
Differential Revision: https://phab.mercurial-scm.org/D3378