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

Close the underlying tcp connection when rejecting #211

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

shterrett
Copy link

When using this library in conjunction with servant-websockets, we noticed that after rejecting the request, the connection was held open until it timed out, approximately one minute later. Our current workaround is to accept the connect and then immediately sendClose and exit.

This adds rejectAndCloseWith to reject the request and close the underlying tcp connection immediately to avoid the workaround above.

@shterrett
Copy link
Author

The build failure appears to be an issue with CircleCI fetching the stack lts

#!/bin/bash -eo pipefail
stack --no-terminal --copy-bins test --pedantic

Downloading lts-15.6 build plan ...
RedownloadInvalidResponse Request {
  host                 = "raw.githubusercontent.com"
  port                 = 443
  secure               = True
  requestHeaders       = []
  path                 = "/fpco/lts-haskell/master//lts-15.6.yaml"
  queryString          = ""
  method               = "GET"
  proxy                = Nothing
  rawBody              = False
  redirectCount        = 10
  responseTimeout      = ResponseTimeoutDefault
  requestVersion       = HTTP/1.1
}
 "/root/.stack/build-plan/lts-15.6.yaml" (Response {responseStatus = Status {statusCode = 404, statusMessage = "Not Found"}, responseVersion = HTTP/1.1, responseHeaders = [("Connection","keep-alive"),("Content-Length","14"),("Content-Security-Policy","default-src 'none'; style-src 'unsafe-inline'; sandbox"),("Strict-Transport-Security","max-age=31536000"),("X-Content-Type-Options","nosniff"),("X-Frame-Options","deny"),("X-XSS-Protection","1; mode=block"),("Content-Type","text/plain; charset=utf-8"),("Via","1.1 varnish (Varnish/6.0), 1.1 varnish"),("X-GitHub-Request-Id","E114:768A:F93F50:11C1623:5FA98AF4"),("Accept-Ranges","bytes"),("Date","Mon, 09 Nov 2020 18:31:17 GMT"),("X-Served-By","cache-wdc5554-WDC"),("X-Cache","MISS, MISS"),("X-Cache-Hits","0, 0"),("X-Timer","S1604946677.248789,VS0,VE137"),("Vary","Authorization,Accept-Encoding"),("Access-Control-Allow-Origin","*"),("X-Fastly-Request-ID","07d6d8c7a70261ff968d1cb3729faf52197f1876"),("Expires","Mon, 09 Nov 2020 18:36:17 GMT"),("Source-Age","0")], responseBody = (), responseCookieJar = CJ {expose = []}, responseClose' = ResponseClose})

Exited with code exit status 1

CircleCI received exit code 1

@domenkozar
Copy link
Collaborator

Is there any reason why rejectRequest wouldn't close the connection?

@domenkozar
Copy link
Collaborator

@shterrett if you rebase on top of master CI failures would go away.

@domenkozar
Copy link
Collaborator

@jaspervdj
Copy link
Owner

Is there any reason why rejectRequest wouldn't close the connection?

Historically, I was assuming it would be the task of the framework (WAI/Snap/...) to close the connection once the WebSockets application finishes. I wanted to avoid double closes, but it maybe depends on the specific framework on whether or not that is a bad thing or the second close would just do nothing.

But ever since we have the Streams abstraction, I guess it's fine to close that it in either case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants