Add outBodyCancel to OutBodyIface

[FinleyMcIlwaine]

Support stream cancellation by adding an outBodyCancel field to the OutBodyIface API.

[FinleyMcIlwaine]

@kazu-yamamoto Could you please add @edsko as a reviewer? Thank you!

[edsko]

LGTM, modulo a few minor suggestions for improving the docs.

[edsko]

I always get a bit confused about exactly how this code works, the interaction between http2 and http-semantics is a bit subtle. @FinleyMcIlwaine It would help me if we added the following comments to the code:

1. Definition of DynaNext:

   -- | Write part of a streaming response to the write buffer
   --
   -- In @http2@ this will be used to construct a single HTTP2 @DATA@ frame
   -- (see discussion of the maximum number of bytes, below).
   type DynaNext =
         Buffer
         -- ^ Write buffer
      -> Int
         -- ^ Maximum number of bytes we are allowed to write
         --
         -- In @http2@, this maximum will be set to the space left in the write
         -- buffer. Implicitly this also means that this maximum cannot exceed the
         -- maximum size of a HTTP2 frame, since in @http2@ the size of the write
         -- buffer is also used to set @SETTINGS_MAX_FRAME_SIZE@ (see
         -- @confBufferSize@).
      -> IO Next
         -- ^ Information the data written, and on how to continue if not all data
         -- was written

   (We can also remove the old definition of DynaNext that is still left in the code as a comment.)

2. Definition of NextWithTotal:

   -- | Like 'DynaNext', but with additional argument indicating total bytes written
   --
   -- Since @http2@ uses @DynaNext@ to construct a /single/ @DATA@ frame, the
   -- \"total number of bytes written\" refers to the current size of the payload
   -- of that @DATA@ frame.
   type NextWithTotal = Int -> DynaNext

3. In the documentation of OutBodyIface (which is user facing documentation, unlike DynaNext and NextWithTotal), let's clarify what outBodyPush and outBodyFlush do:

   , outBodyPush :: Builder -> IO ()
   -- ^ Push a new chunk
   --
   -- In @http2@, there is no direct correspondance between chunks and the
   -- resulting @DATA@ frames sent: the chunks are collected (written to an
   -- internal write buffer) until we can fill a frame.
   --
   -- See also 'outBodyFlush'.

   and

   , outBodyFlush :: IO ()
   -- ^ Flush
   --
   -- This can be used to emit a DATA frame with the data collected so far
   -- (using 'outBodyPush'), even if that DATA frame has not yet reached the
   -- maximum frame size. Calling 'outBodyFlush' unnecessarily can therefore
   -- result in excessive overhead from frame headers.
   --
   -- If no data is available to send, this is a no-op.

   (Having written this now, is this really what we want..? It means clients cannot send empty DATA frames, even if they ask nicely 🤔 )

4. Let's also clarify outBodyCancel (in addition to the docs you already added):

   , outBodyCancel :: Maybe SomeException -> IO ()
   -- ^ Cancel the stream
   --
   -- Sends a @RST_STREAM@ (.. as before ..)
   --
   -- If there is a partially constructed @DATA@ frame at the time of
   -- cancellation, this frame is discarded. If this is undesirable, you should
   -- call 'outBodyFlush' prior to cancelling.

5. Finally, in the definition of runStreamingChunk, the case for StreamingCancelled, let's make this its own local function, alongside finished and flush:

   -- Cancel streaming
   --
   -- The @_total@ number of bytes written refers to the @DATA@ frame currently
   -- under construction, but not yet sent (see discussion at 'DynaNext' and
   -- 'NextWithTotal'). Moreover, the documentation of 'outBodyCancel'
   -- explicitly states that such a partially constructed frame, if one exists,
   -- will be discarded on cancellation. We can therefore simply ignore
   -- @_total@ here.
   cancel :: Maybe SomeException -> NextWithTotal
   cancel mErr = \_total _buf _room -> pure $ CancelNext mErr

Now that I have written this actually I am a bit concerned, about point (4). On the http2 side, the call to resetStream results in a frame being added to the control queue. Could it be that due to an explicit flush, or perhaps simply a DATA frame being full and therefore enqueued, we have a race condition between that frame being sent and the RST_STREAM being sent, and therefore the possibility of the RST_STREAM being followed by a data frame on the stream that was reset? The spec doesn't explicitly disallow this (see discussion at kazu-yamamoto/http2#106), but I still worry that it might confuse some peers.

[edsko]

Let's consider the race conditioned mentioned in #11 (comment) before we consider this mergeable.

[FinleyMcIlwaine]

Great documentation, I've added it. I think not being able to send empty data frames is okay.

[edsko]

Nice, this all looks good to me now. Thanks @FinleyMcIlwaine !

@kazu-yamamoto , this PR, together with kazu-yamamoto/http2#142, adds the ability to cancel a streaming request. This is necessary, because although of course it is possible to simply stop streaming, this will result in http2 putting the connection in half-closed state, and the server will then not be able to distinguish between the client simply being finished and something having gone wrong on the client side.

[kazu-yamamoto]

The cancel part looks good to me.
Excellent improvement for the documentation!