Second `readBuffer` call hangs forever

[paluh]

It seems that second call to readBuffer hangs forever. Minimal example:

 main :: forall e. Eff (avar ∷ AVAR, buffer ∷ BUFFER, console :: CONSOLE, http :: HTTP | e) Unit
 main =
   let app = writeStatus statusOK
             :*> (readBody :: Middleware _ _ _ String)
             :*> (readBody :: Middleware _ _ _ String)
             :*> closeHeaders
             :*> respond "Hello, Hyper!"
   in runServer defaultOptionsWithLogging {} app

I'm not sure, but maybe body state also should be tracked in types... What do you think?

[owickstrom]

Yeah, I recall having thought about this before. It would make sense for readBody to make a request state transition. The problem is streamBody, which currently "leaks" the underlying Stream. Maybe that operation could also transition to BodyRead, but you'd be on your own in terms of safety.

[owickstrom]

Just sketching:

data HeadersRead
data BodyRead

-- | A middleware transitioning from one `Request` state to another.
type RequestStateTransition m req from to a =
  forall res c.
  Middleware
  m
  (Conn (req from) res c)
  (Conn (req to) res c)
  a

class Request (req :: Type -> Type) m where
  getRequestData
    :: forall state res c
     . Middleware
       m
       (Conn (req state) res c)
       (Conn (req state) res c)
       RequestData

class Request req m <= BaseRequest req m

-- | A `ReadableBody` instance reads the complete request body as a
-- | value of type `b`. For streaming the request body, see the
-- | [StreamableBody](#streamablebody) class.
class (Request req m) <= ReadableBody req m b where
  readBody
    :: RequestStateTransition m req HeadersRead BodyRead b

[owickstrom]

Maybe the StreamableBody method could take the streaming consumer function as an argument, and directly changing the state of the request to BodyRead:

class (Request req m) <= StreamableBody req m stream | req -> stream where
  streamBody :: (stream -> m Unit) -> RequestStateTransition m req HeadersRead BodyRead b

So you'd get access to the underlying stream only inside the supplied function.

[JordanMartinez]

I'd like to pick up this issue following the same approach I proposed in #86.

I think the HeadersRead and BodyRead should be renamed to HeadersReadable and BodyReadable. Both convey the idea that those aspects are "readable" in that current state. The final transition could be FullyRead or something like that.

Just to clarify, but should there also be a type for reading the status line?

[owickstrom]

I think the HeadersRead and BodyRead should be renamed to HeadersReadable and BodyReadable. Both convey the idea that those aspects are "readable" in that current state. The final transition could be FullyRead or something like that.

IIRC (it was some time ago since I worked on this), the status line and headers are always read. The only state transition currently represented is not having read the body (HeadersRead) and (BodyRead). You could explicitly represent the prior states, too, but I'm not sure how useful it is for a user of Hyper. Only when implementing the underlying HTTP server would you care about reading status/headers (and I don't even know if the NodeJS HTTP API supports that level of control). Does that make sense?

[JordanMartinez]

Makes sense. I was thinking of parallels to the request state, so this came to mind.