async, closures, procs, nimcalls, gcsafe, locks are too confusing to work with

[xmonader]

I was planning to do some websocket integration in nim-servy, but it was very painful because

• Had to change the interface because apparently async and var don't work which cannot be captured as it would violate memory so that required me to change to ref objects (thx @leorize)
• Finding the right combination of pragmas, async, closure, gcsafe, locks, nimcall and knowing the behavior is really annoying and too many are going by guess work until it works.Also, in the previous time @mratsim was kind enough to fix the combinations himself
• in the end I had to change the signuatre to type HandlerFunc* = proc(req: Request, res: Response) : Future[void] {.closure, gcsafe.}
• Very annoying part, is I can't use closures and nimcalls interchangebly and apparently If I use a seq I need to start it with a closure element ( thx @xflywind )

So this works

    var s = initServy(opts, router, @[serveTmpDir, serveHomeDir, loggingMiddleware, trimTrailingSlash])

but

    var s = initServy(opts, router, @[loggingMiddleware, trimTrailingSlash, serveTmpDir, serveHomeDir])

because this starts with a proc loggingMiddleware ... this is very annoying

Here's a diff of the migration process

diff --git a/servy.nimble b/servy.nimble
index 4341172..3d2bf1c 100644
--- a/servy.nimble
+++ b/servy.nimble
@@ -13,4 +13,4 @@ bin           = @["servy"]

 requires "nim >= 1.0.0"
 requires "terminaltables"
-
+requires "ws"
diff --git a/src/servy.nim b/src/servy.nim
index f70fc04..09cab96 100644
--- a/src/servy.nim
+++ b/src/servy.nim
@@ -6,7 +6,8 @@ from cgi import decodeUrl
 import terminaltables
 import mimetypes
 import base64
-
+import ws
+import std/sha1

 type
   HttpVersion* = enum
@@ -355,7 +356,7 @@ proc `$`(this: FormMultiPart): string =
   return fmt"parts: {this.parts}"

-type Request* = object
+type Request* = ref object
   httpMethod*: HTTPMethod
   requestURI*: string
   httpVersion*: HttpVersion
@@ -367,7 +368,7 @@ type Request* = object
   formData*: FormMultiPart
   urlParams*: Table[string, string]
   cookies*: Table[string, string]
-
+  asyncSock: AsyncSocket

 proc `$`*(r: Request): string =
   result.add "*******RequestInfo*******"
@@ -394,19 +395,21 @@ proc fullInfo*(r: Request) =
   echo r.body
   echo "***************************"

-type Response* = object
+type Response* = ref object
   headers: HttpHeaders
   httpver: HttpVersion
   code: HttpCode
   content: string

-proc initResponse*(): Response =
+proc newResponse*(): Response =
+  result.new
   result.httpver = HttpVer11
   result.headers = newHttpHeaders()

-type MiddlewareFunc* = proc(req: var Request, resp: var Response): bool {.closure, gcsafe, locks: 0.}
-type HandlerFunc* = proc(req: var Request, res: var Response): void {.nimcall.}
+type MiddlewareFunc* = proc(req: Request, res: Response): Future[bool]  {.closure, gcsafe.}
+type HandlerFunc* = proc(req: Request, res: Response) :  Future[void]   {.closure, gcsafe.}
+

 type RouterValue* = object
   handlerFunc: HandlerFunc
@@ -419,18 +422,18 @@ type Router* = object

-proc abortWith*(res: var Response, msg: string, code=Http404) =
+proc abortWith*(res: Response, msg: string, code=Http404) =
   res.code = code
   res.content = msg

-proc redirectTo*(res: var Response, url: string, code=Http301) =
+proc redirectTo*(res: Response, url: string, code=Http301) =
   res.code = code
   res.headers.add("Location", url)

-proc handle404*(req: var Request, res: var Response) =
+proc handle404*(req: Request, res: Response) : Future[void] {.async.} =
   res.code = Http404
   res.content = fmt"nothing at {req.path}"

@@ -622,6 +625,7 @@ received request from client: (httpMethod: HttpPost, requestURI: "", httpVersion
       # echo $result.parts

 proc parseRequestFromConnection(s: Servy, conn:AsyncSocket): Future[Request] {.async.} =
+    result = Request.new
     let requestline = $await conn.recvLine(maxLength=maxLine)
     var  meth, path, httpver: string
     var parts = requestLine.splitWhitespace()
@@ -635,7 +639,7 @@ proc parseRequestFromConnection(s: Servy, conn:AsyncSocket): Future[Request] {.a
         result.httpMethod = m.get()
     else:
         # echo meth
-        raise newException(OSError, "invalid httpmethod "& meth)
+        raise newException(OSError, "invalid httpmethod " & meth)
     if "1.1" in httpver:
         result.httpVersion = HttpVer11
     elif "1.0" in httpver:
@@ -735,11 +739,11 @@ template logMsg(m: string) : untyped =

 proc handleClient*(s: Servy, client: AsyncSocket) {.async.} =
   var req = await s.parseRequestFromConnection(client)
-  var res = initResponse()
+  var res = newResponse()
   res.headers = newHttpHeaders()

   for  m in s.middlewares:
-    let usenextmiddleware = m(req, res)
+    let usenextmiddleware = await m(req, res)
     if not usenextmiddleware:
       logMsg "early return from middleware..."
       await client.send(res.format())
@@ -755,13 +759,13 @@ proc handleClient*(s: Servy, client: AsyncSocket) {.async.} =

   for  m in middlewares:
-    let usenextmiddleware = m(req, res)
+    let usenextmiddleware = await m(req, res)
     if not usenextmiddleware:
       logMsg "early return from route middleware..."
       await client.send(res.format())
       return

-  handler(req,res)
+  await handler(req, res)

   logMsg "reached the handler safely.. and executing now."
   await client.send(res.format())
@@ -828,8 +832,8 @@ proc stripLeadingSlashes(s: string): string =
       break  
   s[idx..^1]

-proc newStaticMiddleware*(dir: string, onRoute="/public"): proc(request: var Request, response: var Response): bool {.closure, gcsafe, locks: 0.} =
-  result = proc(request: var Request, response: var Response): bool {.closure, gcsafe, locks: 0.} =
+proc newStaticMiddleware*(dir: string, onRoute="/public"): proc(request: Request, response: Response): Future[bool] {.async, closure, gcsafe.} =
+  result = proc(request: Request, response: Response): Future[bool] {.async, closure, gcsafe.} =

     # TODO:
     # check for method to be get/head
@@ -856,7 +860,7 @@ proc newStaticMiddleware*(dir: string, onRoute="/public"): proc(request: var Req
         return true

-let loggingMiddleware* = proc(request: var Request,  response: var Response): bool {.closure, gcsafe, locks: 0.} =
+proc loggingMiddleware*(request: Request,  response: Response): Future[bool] {.async.} =
   let path = request.path
   let headers = request.headers
   echo "==============================="
@@ -866,7 +870,7 @@ let loggingMiddleware* = proc(request: var Request,  response: var Response): bo
   echo "==============================="
   return true

-let trimTrailingSlash* = proc(request: var Request,  response: var Response): bool {.closure, gcsafe, locks: 0.} =
+proc trimTrailingSlash*(request: Request,  response: Response): Future[bool] {.async.} =
   let path = request.path
   if path.endswith("/"):
     request.path = path[0..^2]
@@ -880,10 +884,10 @@ let trimTrailingSlash* = proc(request: var Request,  response: var Response): bo

-proc basicAuth*(users: Table[string, string], realm="private", text="Access denied"): proc(request: var Request, response: var Response): bool {.closure, gcsafe, locks: 0.} =
+proc basicAuth*(users: Table[string, string], realm="private", text="Access denied"): proc(request: Request, response: Response): Future[bool] {.async, closure, gcsafe.} =

-  result = proc(request: var Request, response: var Response): bool {.closure, gcsafe, locks: 0.} =
+  result = proc(request: Request, response: Response): Future[bool] {.async, closure, gcsafe.} =

     var processedUsers = initTable[string, string]()
     for u, p in users:
@@ -904,20 +908,69 @@ proc basicAuth*(users: Table[string, string], realm="private", text="Access deni

-when isMainModule:

+proc handshake*(ws: WebSocket, headers: HttpHeaders) {.async.} =
+  ws.version = parseInt(headers["Sec-WebSocket-Version"][0])
+  ws.key = headers["Sec-WebSocket-Key"][0].strip()
+  if headers.hasKey("Sec-WebSocket-Protocol"):
+    ws.protocol = headers["Sec-WebSocket-Protocol"][0].strip()
+
+  let 
+    sh = secureHash(ws.key & "258EAFA5-E914-47DA-95CA-C5AB0DC85B11")
+    acceptKey = base64.encode(decodeBase16($sh))
+
+  var response = "HTTP/1.1 101 Web Socket Protocol Handshake\c\L"
+  response.add("Sec-WebSocket-Accept: " & acceptKey & "\c\L")
+  response.add("Connection: Upgrade\c\L")
+  response.add("Upgrade: webSocket\c\L")
+
+  if ws.protocol != "":
+    response.add("Sec-WebSocket-Protocol: " & ws.protocol & "\c\L")
+  response.add "\c\L"
+
+  await ws.tcpSocket.send(response)
+  ws.readyState = Open
+
+proc newServyWebSocket*(req: Request): Future[WebSocket] {.async.} =
+  ## Creates a new socket from an httpbeast request.
+  try:
+      let headers = req.headers
+  
+      if not headers.hasKey("Sec-WebSocket-Version"):
+        discard req.asyncSock.send(formatResponse(Http404, HttpVer11, "Not Found", headers))
+        raise newException(WebSocketError, "Not a valid websocket handshake.")
+  
+      var ws = WebSocket()
+      ws.masked = false
+  
+      # Here is the magic:
+      # req.forget() # Remove from HttpBeast event loop.
+      let fd = req.asyncSock.getFd
+       #   asyncdispatch.register(req.asyncSock.AsyncFD)  # Add to async event loop.
+      
+      ws.tcpSocket = newAsyncSocket(fd.AsyncFD)
+      await ws.handshake(headers)
+      return ws
+  
+  except ValueError, KeyError:
+      # Wrap all exceptions in a WebSocketError so its easy to catch
+      raise newException(
+      WebSocketError, 
+      "Failed to create WebSocket from request: " & getCurrentExceptionMsg()
+      )
+
+when isMainModule:

-  proc main() =
     var router = initRouter()
-    proc handleHello(req:var Request, res: var Response) =
+    proc handleHello(req: Request, res: Response) : Future[void] {.async.} =
       res.code = Http200
       res.content = "hello world from handler /hello" & $req

     router.addRoute("/hello", handleHello)

-    let assertJwtFieldExists =  proc(request: var Request, response: var Response): bool {.closure, gcsafe, locks: 0.} =
+    proc assertJwtFieldExists(req: Request, res: Response): Future[bool] {.async.} =
         # echo $request.headers
-        let jwtHeaderVals = request.headers.getOrDefault("jwt", @[""])
+        let jwtHeaderVals = req.headers.getOrDefault("jwt", @[""])
         let jwt = jwtHeaderVals[0]
         echo "================\n\njwt middleware"
         if jwt.len != 0:
@@ -927,9 +980,9 @@ when isMainModule:
         echo "===================\n\n"
         return true

-    router.addRoute("/bye", handleHello, HttpGet, @[assertJwtFieldExists])
+    # router.addRoute("/bye", handleHello, HttpGet, @[assertJwtFieldExists])

-    proc handleGreet(req:var Request, res: var Response) =
+    proc handleGreet(req: Request, res: Response) : Future[void] {.async.} =
       res.code = Http200
       res.content = "generic greet" & $req
       if "username" in req.urlParams:
@@ -950,17 +1003,17 @@ when isMainModule:
     router.addRoute("/greet/:first/:second/:lang", handleGreet, HttpGet, @[])

-    proc handlePost(req:var Request, res: var Response) =
+    proc handlePost(req: Request, res: Response) : Future[void] {.async.} =
       res.code = Http200
       res.content = $req

     router.addRoute("/post", handlePost, HttpPost, @[])

-    proc handleAbort(req:var Request, res: var Response) =
+    proc handleAbort(req: Request, res: Response) : Future[void] {.async.} =
       res.abortWith("sorry mate")

-    proc handleRedirect(req:var Request, res: var Response)=
+    proc handleRedirect(req: Request, res:  Response): Future[void] {.async.} =
       res.redirectTo("https://python.org")

@@ -971,15 +1024,24 @@ when isMainModule:
     let serveTmpDir = newStaticMiddleware("/tmp", "/tmppublic")
     let serveHomeDir = newStaticMiddleware(getHomeDir(), "/homepublic")

-    proc handleBasicAuth(req:var Request, res: var Response) =
+    proc handleBasicAuth(req: Request, res: Response) : Future[void] {.async.} =
       res.code = Http200
       res.content = "logged in!!"

     let users = {"ahmed":"password", "xmon":"xmon"}.toTable
     router.addRoute("/basicauth", handleBasicAuth, HttpGet, @[basicAuth(users)])

+
+    proc handleWS(req: Request, res: Response) : Future[void] {.async.} =
+      var ws = await newServyWebSocket(req)
+      await ws.send("Welcome to simple echo server")
+      while ws.readyState == Open:
+        let packet = await ws.receiveStrPacket()
+        await ws.send(packet)
+
+    router.addRoute("/ws", handleWS, HttpGet, @[])
+
     let opts = ServerOptions(address:"127.0.0.1", port:9000.Port, debug:true)
-    var s = initServy(opts, router, @[loggingMiddleware, trimTrailingSlash, serveTmpDir, serveHomeDir])
-    s.run()
+    var s = initServy(opts, router, @[serveTmpDir, serveHomeDir, loggingMiddleware, trimTrailingSlash])
+    s.run() 

-  main()

versions

Nim Compiler Version 1.2.0 [Linux: amd64]
Compiled at 2020-04-03
Copyright (c) 2006-2020 by Andreas Rumpf

active boot switches: -d:release
➜  ~ systemctl suspend
➜  ~ nim -v                       
Nim Compiler Version 1.2.0 [Linux: amd64]
Compiled at 2020-04-03
Copyright (c) 2006-2020 by Andreas Rumpf

active boot switches: -d:release
➜  ~ 

cc @dom96 @mratsim @xflywind @leorize

[Araq]

FWIW the error messages did improve significantly already but there is always more...