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

Update http2, wai. #3911

Merged
merged 8 commits into from
Mar 13, 2024
Merged

Update http2, wai. #3911

merged 8 commits into from
Mar 13, 2024

Conversation

elland
Copy link
Contributor

@elland elland commented Feb 28, 2024

may also fix https://wearezeta.atlassian.net/browse/WPB-5357

This cleans up the ConnectionIsClosed noise.

@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Feb 28, 2024
@elland elland force-pushed the connection-is-closed branch from 44fb140 to e516c0d Compare February 29, 2024 09:22
@elland elland marked this pull request as ready for review March 4, 2024 12:32
Copy link
Contributor

@MangoIV MangoIV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not very fond of the string-conversions change 👀

@elland elland force-pushed the connection-is-closed branch 2 times, most recently from 265e853 to a8e8d08 Compare March 5, 2024 08:45
@elland elland force-pushed the connection-is-closed branch 4 times, most recently from 95dde02 to 789c2a3 Compare March 5, 2024 14:24
Copy link
Contributor

@MangoIV MangoIV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me!

Comment on lines +271 to +274
allocServerConfig :: (Socket, Maybe SSL.SSL) -> IO Server.Config
allocServerConfig (sock, Nothing) =
HTTP2.allocSimpleConfig sock 4096
allocServerConfig (sock, Just ssl) = do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: perhaps this would be nicer curried.

| chunkLen > n ->
error "openssl: SSL.read returned more bytes than asked for, this is probably a bug"
| otherwise ->
readData (prevChunk <> chunk) (n - chunkLen)

testServerOnSocket :: Maybe SSL.SSLContext -> Socket -> IORef Int -> IORef (Map Unique (Async ())) -> IO ()
testServerOnSocket mCtx listenSock connsCounter conns = do
listen listenSock 1024
forever $ do
(sock, _) <- accept listenSock
serverCfgParam <- case mCtx of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in that case you could just match here directly and then only pass the snd to the case

@elland elland force-pushed the connection-is-closed branch 6 times, most recently from c28ca13 to 872ccb7 Compare March 7, 2024 12:42
@elland elland force-pushed the connection-is-closed branch 3 times, most recently from ee9965a to b92cdb0 Compare March 7, 2024 15:04
@elland elland force-pushed the connection-is-closed branch from 4ec8059 to 3b076e4 Compare March 13, 2024 09:21
@elland elland merged commit 8f823f2 into develop Mar 13, 2024
8 checks passed
@elland elland deleted the connection-is-closed branch March 13, 2024 12:04
fisx added a commit that referenced this pull request Mar 20, 2024
fisx added a commit that referenced this pull request Mar 20, 2024
* Remove trailing whitespace.

* Revert "Update http2, wai. (#3911)"

This reverts commit 8f823f2.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants