Second readBuffer call hangs forever #48

Activity

1. owickstrom commented on Sep 29, 2017

   

   owickstrom

   on Sep 29, 2017

   Collaborator

   More actions

   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.
2. owickstrom commented on Sep 29, 2017

   

   owickstrom

   on Sep 29, 2017

   Collaborator

   More actions

   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
3. owickstrom commented on Oct 1, 2017

   

   owickstrom

   on Oct 1, 2017

   Collaborator

   More actions

   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.
4. JordanMartinez commented on Aug 6, 2019

   

   JordanMartinez

   on Aug 6, 2019

   Contributor

   More actions

   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?
5. owickstrom commented on Aug 15, 2019

   

   owickstrom

   on Aug 15, 2019

   Collaborator

   More actions

   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?
6. JordanMartinez commented on Aug 15, 2019

   

   JordanMartinez

   on Aug 15, 2019

   Contributor

   More actions

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