fix v3 backwards compatibility for callbacks (#481)

Because the callback types were used explicitly in some consumers of
chronos, the change of type introduces a backwards incompatibility
preventing a smooth transition to v4 for code that doesn't uses
`raises`.

This PR restores backwards compatibility at the expense of introducing a
new type with a potentially ugly name - that said, there is already
precedence for using numbered names to provide new error handling
strategy in chronos.

chronos/apps/http/httpserver.nim

@@ -63,18 +63,22 @@ type
   HttpResponseState* {.pure.} = enum
     Empty, Prepared, Sending, Finished, Failed, Cancelled, Default
 
-  HttpProcessCallback* =
+  # TODO Evaluate naming of raises-annotated callbacks
+  HttpProcessCallback2* =
     proc(req: RequestFence): Future[HttpResponseRef] {.
-      async: (raises: [CancelledError, HttpResponseError]), gcsafe.}
+      async: (raises: [CancelledError, HttpResponseError]).}
+
+  HttpProcessCallback* {.deprecated.} =
+    proc(req: RequestFence): Future[HttpResponseRef] {.async.}
 
   HttpConnectionCallback* =
     proc(server: HttpServerRef,
          transp: StreamTransport): Future[HttpConnectionRef] {.
-      async: (raises: [CancelledError, HttpConnectionError]), gcsafe.}
+      async: (raises: [CancelledError, HttpConnectionError]).}
 
   HttpCloseConnectionCallback* =
     proc(connection: HttpConnectionRef): Future[void] {.
-      async: (raises: []), gcsafe.}
+      async: (raises: []).}
 
   HttpConnectionHolder* = object of RootObj
     connection*: HttpConnectionRef
@@ -103,7 +107,7 @@ type
     bufferSize*: int
     maxHeadersSize*: int
     maxRequestBodySize*: int
-    processCallback*: HttpProcessCallback
+    processCallback*: HttpProcessCallback2
     createConnCallback*: HttpConnectionCallback
 
   HttpServerRef* = ref HttpServer
@@ -182,7 +186,7 @@ proc createConnection(server: HttpServerRef,
 
 proc new*(htype: typedesc[HttpServerRef],
           address: TransportAddress,
-          processCallback: HttpProcessCallback,
+          processCallback: HttpProcessCallback2,
           serverFlags: set[HttpServerFlags] = {},
           socketFlags: set[ServerFlags] = {ReuseAddr},
           serverUri = Uri(),
@@ -236,6 +240,49 @@ proc new*(htype: typedesc[HttpServerRef],
   )
   ok(res)
 
+proc new*(htype: typedesc[HttpServerRef],
+          address: TransportAddress,
+          processCallback: HttpProcessCallback,
+          serverFlags: set[HttpServerFlags] = {},
+          socketFlags: set[ServerFlags] = {ReuseAddr},
+          serverUri = Uri(),
+          serverIdent = "",
+          maxConnections: int = -1,
+          bufferSize: int = 4096,
+          backlogSize: int = DefaultBacklogSize,
+          httpHeadersTimeout = 10.seconds,
+          maxHeadersSize: int = 8192,
+          maxRequestBodySize: int = 1_048_576,
+          dualstack = DualStackType.Auto): HttpResult[HttpServerRef] {.
+          deprecated: "raises missing from process callback".} =
+
+  proc processCallback2(req: RequestFence): Future[HttpResponseRef] {.
+      async: (raises: [CancelledError, HttpResponseError]).} =
+      try:
+        await processCallback(req)
+      except CancelledError as exc:
+        raise exc
+      except HttpResponseError as exc:
+        raise exc
+      except CatchableError as exc:
+        # Emulate 3.x behavior
+        raise (ref HttpCriticalError)(msg: exc.msg, code: Http503)
+
+  HttpServerRef.new(
+    address = address,
+    processCallback = processCallback2,
+    serverFlags = serverFlags,
+    socketFlags = socketFlags,
+    serverUri = serverUri,
+    serverIdent = serverIdent,
+    maxConnections = maxConnections,
+    bufferSize = bufferSize,
+    backlogSize = backlogSize,
+    httpHeadersTimeout = httpHeadersTimeout,
+    maxHeadersSize = maxHeadersSize,
+    maxRequestBodySize = maxRequestBodySize,
+    dualstack = dualstack)
+
 proc getServerFlags(req: HttpRequestRef): set[HttpServerFlags] =
   var defaultFlags: set[HttpServerFlags] = {}
   if isNil(req): return defaultFlags

chronos/apps/http/shttpserver.nim

@@ -82,7 +82,7 @@ proc createSecConnection(server: HttpServerRef,
 
 proc new*(htype: typedesc[SecureHttpServerRef],
           address: TransportAddress,
-          processCallback: HttpProcessCallback,
+          processCallback: HttpProcessCallback2,
           tlsPrivateKey: TLSPrivateKey,
           tlsCertificate: TLSCertificate,
           serverFlags: set[HttpServerFlags] = {},
@@ -145,3 +145,53 @@ proc new*(htype: typedesc[SecureHttpServerRef],
     secureFlags: secureFlags
   )
   ok(res)
+
+proc new*(htype: typedesc[SecureHttpServerRef],
+          address: TransportAddress,
+          processCallback: HttpProcessCallback,
+          tlsPrivateKey: TLSPrivateKey,
+          tlsCertificate: TLSCertificate,
+          serverFlags: set[HttpServerFlags] = {},
+          socketFlags: set[ServerFlags] = {ReuseAddr},
+          serverUri = Uri(),
+          serverIdent = "",
+          secureFlags: set[TLSFlags] = {},
+          maxConnections: int = -1,
+          bufferSize: int = 4096,
+          backlogSize: int = DefaultBacklogSize,
+          httpHeadersTimeout = 10.seconds,
+          maxHeadersSize: int = 8192,
+          maxRequestBodySize: int = 1_048_576,
+          dualstack = DualStackType.Auto
+         ): HttpResult[SecureHttpServerRef] {.
+         deprecated: "raises missing from process callback".} =
+  proc processCallback2(req: RequestFence): Future[HttpResponseRef] {.
+      async: (raises: [CancelledError, HttpResponseError]).} =
+      try:
+        await processCallback(req)
+      except CancelledError as exc:
+        raise exc
+      except HttpResponseError as exc:
+        raise exc
+      except CatchableError as exc:
+        # Emulate 3.x behavior
+        raise (ref HttpCriticalError)(msg: exc.msg, code: Http503)
+
+  SecureHttpServerRef.new(
+    address = address,
+    processCallback = processCallback2,
+    tlsPrivateKey = tlsPrivateKey,
+    tlsCertificate = tlsCertificate,
+    serverFlags = serverFlags,
+    socketFlags = socketFlags,
+    serverUri = serverUri,
+    serverIdent = serverIdent,
+    secureFlags = secureFlags,
+    maxConnections = maxConnections,
+    bufferSize = bufferSize,
+    backlogSize = backlogSize,
+    httpHeadersTimeout =  httpHeadersTimeout,
+    maxHeadersSize = maxHeadersSize,
+    maxRequestBodySize = maxRequestBodySize,
+    dualstack = dualstack
+  )

chronos/transports/stream.nim

@@ -115,13 +115,14 @@ else:
         discard
 
 type
-  StreamCallback* = proc(server: StreamServer,
+  # TODO evaluate naming of raises-annotated callbacks
+  StreamCallback2* = proc(server: StreamServer,
                           client: StreamTransport) {.async: (raises: []).}
     ## New remote client connection callback
     ## ``server`` - StreamServer object.
     ## ``client`` - accepted client transport.
 
-  UnsafeStreamCallback* = proc(server: StreamServer,
+  StreamCallback* = proc(server: StreamServer,
                          client: StreamTransport) {.async.}
     ## Connection callback that doesn't check for exceptions at compile time
     ## ``server`` - StreamServer object.
@@ -135,7 +136,7 @@ type
 
   StreamServer* = ref object of SocketServer
     ## StreamServer object
-    function*: StreamCallback         # callback which will be called after new
+    function*: StreamCallback2         # callback which will be called after new
                                       # client accepted
     init*: TransportInitCallback      # callback which will be called before
                                       # transport for new client
@@ -1870,7 +1871,7 @@ proc getBacklogSize(backlog: int): cint =
     cint(backlog)
 
 proc createStreamServer*(host: TransportAddress,
-                         cbproc: StreamCallback,
+                         cbproc: StreamCallback2,
                          flags: set[ServerFlags] = {},
                          sock: AsyncFD = asyncInvalidSocket,
                          backlog: int = DefaultBacklogSize,
@@ -2092,7 +2093,7 @@ proc createStreamServer*(host: TransportAddress,
   sres
 
 proc createStreamServer*(host: TransportAddress,
-                         cbproc: UnsafeStreamCallback,
+                         cbproc: StreamCallback,
                          flags: set[ServerFlags] = {},
                          sock: AsyncFD = asyncInvalidSocket,
                          backlog: int = DefaultBacklogSize,
@@ -2124,11 +2125,11 @@ proc createStreamServer*(host: TransportAddress,
                          udata: pointer = nil,
                          dualstack = DualStackType.Auto): StreamServer {.
     raises: [TransportOsError].} =
-  createStreamServer(host, StreamCallback(nil), flags, sock, backlog, bufferSize,
+  createStreamServer(host, StreamCallback2(nil), flags, sock, backlog, bufferSize,
                      child, init, cast[pointer](udata), dualstack)
 
 proc createStreamServer*[T](host: TransportAddress,
-                            cbproc: StreamCallback,
+                            cbproc: StreamCallback2,
                             flags: set[ServerFlags] = {},
                             udata: ref T,
                             sock: AsyncFD = asyncInvalidSocket,
@@ -2144,7 +2145,7 @@ proc createStreamServer*[T](host: TransportAddress,
                      child, init, cast[pointer](udata), dualstack)
 
 proc createStreamServer*[T](host: TransportAddress,
-                            cbproc: UnsafeStreamCallback,
+                            cbproc: StreamCallback,
                             flags: set[ServerFlags] = {},
                             udata: ref T,
                             sock: AsyncFD = asyncInvalidSocket,
@@ -2172,7 +2173,7 @@ proc createStreamServer*[T](host: TransportAddress,
     raises: [TransportOsError].} =
   var fflags = flags + {GCUserData}
   GC_ref(udata)
-  createStreamServer(host, StreamCallback(nil), fflags, sock, backlog, bufferSize,
+  createStreamServer(host, StreamCallback2(nil), fflags, sock, backlog, bufferSize,
                      child, init, cast[pointer](udata), dualstack)
 
 proc getUserData*[T](server: StreamServer): T {.inline.} =

tests/testhttpclient.nim

@@ -85,7 +85,7 @@ suite "HTTP client testing suite":
     res
 
   proc createServer(address: TransportAddress,
-                    process: HttpProcessCallback, secure: bool): HttpServerRef =
+                    process: HttpProcessCallback2, secure: bool): HttpServerRef =
     let
       socketFlags = {ServerFlags.TcpNoDelay, ServerFlags.ReuseAddr}
       serverFlags = {HttpServerFlags.Http11Pipeline}