Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inferno-vc shouldn't use Manager internals #101

Open
ngua opened this issue Feb 9, 2024 · 0 comments
Open

inferno-vc shouldn't use Manager internals #101

ngua opened this issue Feb 9, 2024 · 0 comments
Labels
correctness Make something more correct

Comments

@ngua
Copy link
Contributor

ngua commented Feb 9, 2024

mkVCClientEnv in inferno-vc currently uses the internals of Manager (from http-client) to modify request headers and body:

mkVCClientEnv :: Manager -> BaseUrl -> ClientEnv
mkVCClientEnv man@Manager {mModifyRequest = modReq} baseUrl =
mkClientEnv man {mModifyRequest = modReq'} baseUrl
where
modReq' :: Request -> IO Request
modReq' r = do
x <- modReq r
pure $
if ((hContentEncoding, "gzip") `elem` requestHeaders x)
then x
else
let new_hdrs = (hContentEncoding, "gzip") : requestHeaders x
(hrds, body) = case requestBody x of
RequestBodyBuilder _ _ -> (requestHeaders x, requestBody x)
RequestBodyStream _ _ -> (requestHeaders x, requestBody x)
RequestBodyStreamChunked _ -> (requestHeaders x, requestBody x)
b -> (new_hdrs, compressBody b)
in x {requestHeaders = hrds, requestBody = body}
compressBody :: RequestBody -> RequestBody
compressBody = \case
RequestBodyLBS bsl -> RequestBodyLBS $ compress bsl
RequestBodyBS bs -> RequestBodyLBS $ compress $ BSL.fromStrict bs
RequestBodyIO iob -> RequestBodyIO $ compressBody <$> iob
b -> b

I don't think we should rely on the internal implementation of Manager like this; the current docs for that Network.HTTP.Client.Internal state

No API stability is guaranteed for this module

which is generally the case for modules marked as internal.

We could probably just use a middleware to achieve the same thing. Or, if that's not possible, this code should document why it's necessary to use internal modules from another library

@ngua ngua added invalid This doesn't seem right correctness Make something more correct and removed invalid This doesn't seem right labels Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
correctness Make something more correct
Projects
None yet
Development

No branches or pull requests

1 participant