Check if connection is open in withConnection #18
Conversation
Change to the type of waitStream
| ensureOpen :: ApnSession -> IO () | ||
| ensureOpen s = do | ||
| open <- isOpen s |
There was a problem hiding this comment.
For backwards compat, it's nicer if we leave the existing function as an alias. The types indicate what sort of thing it is. Then we're in minor bump land. A DEPRECATED pragma could also point users to the ensureSessionOpen so that ensureOpen can be deleted in the next major version.
| @@ -639,7 +650,7 @@ sendApnRaw connection deviceToken mJwtBearerToken message = bracket_ | |||
| upload message (HTTP2.setEndHeader . HTTP2.setEndStream) client (_outgoingFlowControl client) stream osfc | |||
| let pph _hStreamId _hStream hHeaders _hIfc _hOfc = | |||
| lift $ print hHeaders | |||
| response <- waitStream stream isfc pph | |||
| response <- waitStream client stream isfc pph | |||
There was a problem hiding this comment.
PR can have permissive bounds on http2-client if this is guarded with CPP
| response <- waitStream client stream isfc pph | |
| #if MIN_VERSION_http2_client(?major, ?major, ?minor) | |
| response <- waitStream client stream isfc pph | |
| #else | |
| response <- waitStream stream isfc pph | |
| #endif |
src/Network/PushNotify/APN.hs
Outdated
| ensureConnectionOpen :: ApnConnection -> IO () | ||
| ensureConnectionOpen c = do | ||
| open <- isConnectionOpen c | ||
| unless open $ error "Connection is closed" |
There was a problem hiding this comment.
I'm curious about this error right here. The context is that we're calling this in an ExceptT . (try :: IO a -> IO (Either ClientError a)). Which means that it seems like we're intending to have a ClientError as the "known" error path.
I know the other ensureSessionIsOpen also uses error - may be worthwhile to change that too? Would be a major break to change exception types IMO.
matthewbauer
force-pushed
the
is-connection-open
branch
from
b84d801 to
7d4090c
Compare
|
Thanks for the PR. I see there are some merge conflicts, though? |
|
@matthewbauer @parsonsmatt this PR is included in v0.4.0.0 of |
No description provided.