From 130617025597cd4e10a3be3d6f171ff9892ca3cd Mon Sep 17 00:00:00 2001
From: Jacek Sieka <jacek@status.im>
Date: Fri, 17 Nov 2023 13:45:17 +0100
Subject: [PATCH] dedicated exceptions for `Future.read` failures (#474)

Dedicated exceptions for `read` failures reduce the risk of mixing up
"user" exceptions with those of Future itself. The risk still exists, if
the user allows a chronos exception to bubble up explicitly.

Because `await` structurally guarantees that the Future is not `pending`
at the time of `read`, it does not raise this new exception.

* introduce `FuturePendingError` and `FutureCompletedError` when
`read`:ing a future of uncertain state
* fix `waitFor` / `read` to return `lent` values
* simplify code generation for `void`-returning async procs
* document `Raising` type helper
---
 chronos/futures.nim               |  22 +--
 chronos/internal/asyncfutures.nim | 221 ++++++++++++++++++++----------
 chronos/internal/asyncmacro.nim   | 177 ++++++++++++++----------
 docs/src/async_procs.md           |  18 ++-
 docs/src/concepts.md              |  20 ++-
 docs/src/error_handling.md        |  15 ++
 docs/src/porting.md               |   9 +-
 7 files changed, 315 insertions(+), 167 deletions(-)

diff --git a/chronos/futures.nim b/chronos/futures.nim
index 0af635f..6fb9592 100644
--- a/chronos/futures.nim
+++ b/chronos/futures.nim
@@ -73,10 +73,15 @@ type
     cause*: FutureBase
 
   FutureError* = object of CatchableError
+    future*: FutureBase
 
   CancelledError* = object of FutureError
     ## Exception raised when accessing the value of a cancelled future
 
+func raiseFutureDefect(msg: static string, fut: FutureBase) {.
+    noinline, noreturn.} =
+  raise (ref FutureDefect)(msg: msg, cause: fut)
+
 when chronosFutureId:
   var currentID* {.threadvar.}: uint
   template id*(fut: FutureBase): uint = fut.internalId
@@ -202,13 +207,11 @@ func value*[T: not void](future: Future[T]): lent T =
   ## Return the value in a completed future - raises Defect when
   ## `fut.completed()` is `false`.
   ##
-  ## See `read` for a version that raises an catchable error when future
+  ## See `read` for a version that raises a catchable error when future
   ## has not completed.
   when chronosStrictFutureAccess:
     if not future.completed():
-      raise (ref FutureDefect)(
-        msg: "Future not completed while accessing value",
-        cause: future)
+      raiseFutureDefect("Future not completed while accessing value", future)
 
   future.internalValue
 
@@ -216,13 +219,11 @@ func value*(future: Future[void]) =
   ## Return the value in a completed future - raises Defect when
   ## `fut.completed()` is `false`.
   ##
-  ## See `read` for a version that raises an catchable error when future
+  ## See `read` for a version that raises a catchable error when future
   ## has not completed.
   when chronosStrictFutureAccess:
     if not future.completed():
-      raise (ref FutureDefect)(
-        msg: "Future not completed while accessing value",
-        cause: future)
+      raiseFutureDefect("Future not completed while accessing value", future)
 
 func error*(future: FutureBase): ref CatchableError =
   ## Return the error of `future`, or `nil` if future did not fail.
@@ -231,9 +232,8 @@ func error*(future: FutureBase): ref CatchableError =
   ## future has not failed.
   when chronosStrictFutureAccess:
     if not future.failed() and not future.cancelled():
-      raise (ref FutureDefect)(
-        msg: "Future not failed/cancelled while accessing error",
-        cause: future)
+      raiseFutureDefect(
+        "Future not failed/cancelled while accessing error", future)
 
   future.internalError
 
diff --git a/chronos/internal/asyncfutures.nim b/chronos/internal/asyncfutures.nim
index a36ff4a..f60b2d9 100644
--- a/chronos/internal/asyncfutures.nim
+++ b/chronos/internal/asyncfutures.nim
@@ -8,6 +8,9 @@
 #    Apache License, version 2.0, (LICENSE-APACHEv2)
 #                MIT license (LICENSE-MIT)
 
+## Features and utilities for `Future` that integrate it with the dispatcher
+## and the rest of the async machinery
+
 {.push raises: [].}
 
 import std/[sequtils, macros]
@@ -45,15 +48,28 @@ func `[]`*(loc: array[LocationKind, ptr SrcLoc], v: int): ptr SrcLoc {.
 
 type
   FutureStr*[T] = ref object of Future[T]
-    ## Future to hold GC strings
+    ## Deprecated
     gcholder*: string
 
   FutureSeq*[A, B] = ref object of Future[A]
-    ## Future to hold GC seqs
+    ## Deprecated
     gcholder*: seq[B]
 
+  FuturePendingError* = object of FutureError
+    ## Error raised when trying to `read` a Future that is still pending
+  FutureCompletedError* = object of FutureError
+    ## Error raised when trying access the error of a completed Future
+
   SomeFuture = Future|InternalRaisesFuture
 
+func raiseFuturePendingError(fut: FutureBase) {.
+    noinline, noreturn, raises: FuturePendingError.} =
+  raise (ref FuturePendingError)(msg: "Future is still pending", future: fut)
+func raiseFutureCompletedError(fut: FutureBase) {.
+    noinline, noreturn, raises: FutureCompletedError.} =
+  raise (ref FutureCompletedError)(
+    msg: "Future is completed, cannot read error", future: fut)
+
 # Backwards compatibility for old FutureState name
 template Finished* {.deprecated: "Use Completed instead".} = Completed
 template Finished*(T: type FutureState): FutureState {.
@@ -479,6 +495,10 @@ macro internalCheckComplete*(fut: InternalRaisesFuture, raises: typed) =
   #      generics are lost - so instead, we pass the raises list explicitly
 
   let types = getRaisesTypes(raises)
+  types.copyLineInfo(raises)
+  for t in types:
+    t.copyLineInfo(raises)
+
   if isNoRaises(types):
     return quote do:
       if not(isNil(`fut`.internalError)):
@@ -497,8 +517,8 @@ macro internalCheckComplete*(fut: InternalRaisesFuture, raises: typed) =
       quote do: discard
     ),
     nnkElseExpr.newTree(
-      nnkRaiseStmt.newNimNode(lineInfoFrom=fut).add(
-        quote do: (`fut`.internalError)
+      nnkRaiseStmt.newTree(
+        nnkDotExpr.newTree(fut, ident "internalError")
       )
     )
   )
@@ -520,39 +540,51 @@ macro internalCheckComplete*(fut: InternalRaisesFuture, raises: typed) =
     ifRaise
   )
 
-proc read*[T: not void](future: Future[T] ): lent T {.raises: [CatchableError].} =
-  ## Retrieves the value of ``future``. Future must be finished otherwise
-  ## this function will fail with a ``ValueError`` exception.
-  ##
-  ## If the result of the future is an error then that error will be raised.
-  if not future.finished():
-    # TODO: Make a custom exception type for this?
-    raise newException(ValueError, "Future still in progress.")
+proc readFinished[T: not void](fut: Future[T]): lent T {.
+    raises: [CatchableError].} =
+  # Read a future that is known to be finished, avoiding the extra exception
+  # effect.
+  internalCheckComplete(fut)
+  fut.internalValue
 
-  internalCheckComplete(future)
-  future.internalValue
-
-proc read*(future: Future[void] ) {.raises: [CatchableError].} =
-  ## Retrieves the value of ``future``. Future must be finished otherwise
-  ## this function will fail with a ``ValueError`` exception.
+proc read*[T: not void](fut: Future[T] ): lent T {.raises: [CatchableError].} =
+  ## Retrieves the value of `fut`.
   ##
-  ## If the result of the future is an error then that error will be raised.
-  if future.finished():
-    internalCheckComplete(future)
-  else:
-    # TODO: Make a custom exception type for this?
-    raise newException(ValueError, "Future still in progress.")
-
-proc readError*(future: FutureBase): ref CatchableError {.raises: [ValueError].} =
-  ## Retrieves the exception stored in ``future``.
+  ## If the future failed or was cancelled, the corresponding exception will be
+  ## raised.
   ##
-  ## An ``ValueError`` exception will be thrown if no exception exists
-  ## in the specified Future.
-  if not(isNil(future.error)):
-    return future.error
-  else:
-    # TODO: Make a custom exception type for this?
-    raise newException(ValueError, "No error in future.")
+  ## If the future is still pending, `FuturePendingError` will be raised.
+  if not fut.finished():
+    raiseFuturePendingError(fut)
+
+  fut.readFinished()
+
+proc read*(fut: Future[void]) {.raises: [CatchableError].} =
+  ## Checks that `fut` completed.
+  ##
+  ## If the future failed or was cancelled, the corresponding exception will be
+  ## raised.
+  ##
+  ## If the future is still pending, `FuturePendingError` will be raised.
+  if not fut.finished():
+    raiseFuturePendingError(fut)
+
+  internalCheckComplete(fut)
+
+proc readError*(fut: FutureBase): ref CatchableError {.raises: [FutureError].} =
+  ## Retrieves the exception of the failed or cancelled `fut`.
+  ##
+  ## If the future was completed with a value, `FutureCompletedError` will be
+  ## raised.
+  ##
+  ## If the future is still pending, `FuturePendingError` will be raised.
+  if not fut.finished():
+    raiseFuturePendingError(fut)
+
+  if isNil(fut.error):
+    raiseFutureCompletedError(fut)
+
+  fut.error
 
 template taskFutureLocation(future: FutureBase): string =
   let loc = future.location[LocationKind.Create]
@@ -568,18 +600,46 @@ template taskErrorMessage(future: FutureBase): string =
 template taskCancelMessage(future: FutureBase): string =
   "Asynchronous task " & taskFutureLocation(future) & " was cancelled!"
 
-proc waitFor*[T](fut: Future[T]): T {.raises: [CatchableError].} =
-  ## **Blocks** the current thread until the specified future finishes and
-  ## reads it, potentially raising an exception if the future failed or was
-  ## cancelled.
-  var finished = false
-  # Ensure that callbacks currently scheduled on the future run before returning
-  proc continuation(udata: pointer) {.gcsafe.} = finished = true
+proc pollFor[F: Future | InternalRaisesFuture](fut: F): F {.raises: [].} =
+  # Blocks the current thread of execution until `fut` has finished, returning
+  # the given future.
+  #
+  # Must not be called recursively (from inside `async` procedures).
+  #
+  # See alse `awaitne`.
   if not(fut.finished()):
+    var finished = false
+    # Ensure that callbacks currently scheduled on the future run before returning
+    proc continuation(udata: pointer) {.gcsafe.} = finished = true
     fut.addCallback(continuation)
+
     while not(finished):
       poll()
-  fut.read()
+
+  fut
+
+proc waitFor*[T: not void](fut: Future[T]): lent T {.raises: [CatchableError].} =
+  ## Blocks the current thread of execution until `fut` has finished, returning
+  ## its value.
+  ##
+  ## If the future failed or was cancelled, the corresponding exception will be
+  ## raised.
+  ##
+  ## Must not be called recursively (from inside `async` procedures).
+  ##
+  ## See also `await`, `Future.read`
+  pollFor(fut).readFinished()
+
+proc waitFor*(fut: Future[void]) {.raises: [CatchableError].} =
+  ## Blocks the current thread of execution until `fut` has finished.
+  ##
+  ## If the future failed or was cancelled, the corresponding exception will be
+  ## raised.
+  ##
+  ## Must not be called recursively (from inside `async` procedures).
+  ##
+  ## See also `await`, `Future.read`
+  pollFor(fut).internalCheckComplete()
 
 proc asyncSpawn*(future: Future[void]) =
   ## Spawns a new concurrent async task.
@@ -943,7 +1003,7 @@ proc cancelAndWait*(future: FutureBase, loc: ptr SrcLoc): Future[void] {.
 
   retFuture
 
-template cancelAndWait*(future: FutureBase): Future[void] =
+template cancelAndWait*(future: FutureBase): Future[void].Raising([CancelledError]) =
   ## Cancel ``future``.
   cancelAndWait(future, getSrcLocation())
 
@@ -1500,37 +1560,56 @@ when defined(windows):
 
 {.pop.} # Automatically deduced raises from here onwards
 
-proc waitFor*[T, E](fut: InternalRaisesFuture[T, E]): T = # {.raises: [E]}
-  ## **Blocks** the current thread until the specified future finishes and
-  ## reads it, potentially raising an exception if the future failed or was
-  ## cancelled.
-  while not(fut.finished()):
-    poll()
+proc readFinished[T: not void; E](fut: InternalRaisesFuture[T, E]): lent T =
+  internalCheckComplete(fut, E)
+  fut.internalValue
 
-  fut.read()
-
-proc read*[T: not void, E](future: InternalRaisesFuture[T, E]): lent T = # {.raises: [E, ValueError].}
-  ## Retrieves the value of ``future``. Future must be finished otherwise
-  ## this function will fail with a ``ValueError`` exception.
+proc read*[T: not void, E](fut: InternalRaisesFuture[T, E]): lent T = # {.raises: [E, FuturePendingError].}
+  ## Retrieves the value of `fut`.
   ##
-  ## If the result of the future is an error then that error will be raised.
-  if not future.finished():
-    # TODO: Make a custom exception type for this?
-    raise newException(ValueError, "Future still in progress.")
-
-  internalCheckComplete(future, E)
-  future.internalValue
-
-proc read*[E](future: InternalRaisesFuture[void, E]) = # {.raises: [E, CancelledError].}
-  ## Retrieves the value of ``future``. Future must be finished otherwise
-  ## this function will fail with a ``ValueError`` exception.
+  ## If the future failed or was cancelled, the corresponding exception will be
+  ## raised.
   ##
-  ## If the result of the future is an error then that error will be raised.
-  if future.finished():
-    internalCheckComplete(future)
-  else:
-    # TODO: Make a custom exception type for this?
-    raise newException(ValueError, "Future still in progress.")
+  ## If the future is still pending, `FuturePendingError` will be raised.
+  if not fut.finished():
+    raiseFuturePendingError(fut)
+
+  fut.readFinished()
+
+proc read*[E](fut: InternalRaisesFuture[void, E]) = # {.raises: [E].}
+  ## Checks that `fut` completed.
+  ##
+  ## If the future failed or was cancelled, the corresponding exception will be
+  ## raised.
+  ##
+  ## If the future is still pending, `FuturePendingError` will be raised.
+  if not fut.finished():
+    raiseFuturePendingError(fut)
+
+  internalCheckComplete(fut, E)
+
+proc waitFor*[T: not void; E](fut: InternalRaisesFuture[T, E]): lent T = # {.raises: [E]}
+  ## Blocks the current thread of execution until `fut` has finished, returning
+  ## its value.
+  ##
+  ## If the future failed or was cancelled, the corresponding exception will be
+  ## raised.
+  ##
+  ## Must not be called recursively (from inside `async` procedures).
+  ##
+  ## See also `await`, `Future.read`
+  pollFor(fut).readFinished()
+
+proc waitFor*[E](fut: InternalRaisesFuture[void, E]) = # {.raises: [E]}
+  ## Blocks the current thread of execution until `fut` has finished.
+  ##
+  ## If the future failed or was cancelled, the corresponding exception will be
+  ## raised.
+  ##
+  ## Must not be called recursively (from inside `async` procedures).
+  ##
+  ## See also `await`, `Future.read`
+  pollFor(fut).internalCheckComplete(E)
 
 proc `or`*[T, Y, E1, E2](
     fut1: InternalRaisesFuture[T, E1],
diff --git a/chronos/internal/asyncmacro.nim b/chronos/internal/asyncmacro.nim
index 88e11e3..079e3bb 100644
--- a/chronos/internal/asyncmacro.nim
+++ b/chronos/internal/asyncmacro.nim
@@ -13,14 +13,14 @@ import
   ../[futures, config],
   ./raisesfutures
 
-proc processBody(node, setResultSym, baseType: NimNode): NimNode {.compileTime.} =
+proc processBody(node, setResultSym: NimNode): NimNode {.compileTime.} =
   case node.kind
   of nnkReturnStmt:
     # `return ...` -> `setResult(...); return`
     let
       res = newNimNode(nnkStmtList, node)
     if node[0].kind != nnkEmpty:
-      res.add newCall(setResultSym, processBody(node[0], setResultSym, baseType))
+      res.add newCall(setResultSym, processBody(node[0], setResultSym))
     res.add newNimNode(nnkReturnStmt, node).add(newEmptyNode())
 
     res
@@ -29,8 +29,14 @@ proc processBody(node, setResultSym, baseType: NimNode): NimNode {.compileTime.}
     # the Future we inject
     node
   else:
+    if node.kind == nnkYieldStmt:
+      # asyncdispatch allows `yield` but this breaks cancellation
+      warning(
+        "`yield` in async procedures not supported - use `awaitne` instead",
+        node)
+
     for i in 0 ..< node.len:
-      node[i] = processBody(node[i], setResultSym, baseType)
+      node[i] = processBody(node[i], setResultSym)
     node
 
 proc wrapInTryFinally(
@@ -179,7 +185,7 @@ proc getName(node: NimNode): string {.compileTime.} =
 macro unsupported(s: static[string]): untyped =
   error s
 
-proc params2(someProc: NimNode): NimNode =
+proc params2(someProc: NimNode): NimNode {.compileTime.} =
   # until https://github.com/nim-lang/Nim/pull/19563 is available
   if someProc.kind == nnkProcTy:
     someProc[0]
@@ -275,6 +281,10 @@ proc asyncSingleProc(prc, params: NimNode): NimNode {.compileTime.} =
       returnType[1]
 
   let
+    # When the base type is known to be void (and not generic), we can simplify
+    # code generation - however, in the case of generic async procedures it
+    # could still end up being void, meaning void detection needs to happen
+    # post-macro-expansion.
     baseTypeIsVoid = baseType.eqIdent("void")
     (raw, raises, handleException) = decodeParams(params)
     internalFutureType =
@@ -295,7 +305,7 @@ proc asyncSingleProc(prc, params: NimNode): NimNode {.compileTime.} =
 
   prc.params2[0] = internalReturnType
 
-  if prc.kind notin {nnkProcTy, nnkLambda}: # TODO: Nim bug?
+  if prc.kind notin {nnkProcTy, nnkLambda}:
     prc.addPragma(newColonExpr(ident "stackTrace", ident "off"))
 
   # The proc itself doesn't raise
@@ -326,63 +336,57 @@ proc asyncSingleProc(prc, params: NimNode): NimNode {.compileTime.} =
         prc.body
       )
 
-    when chronosDumpAsync:
-      echo repr prc
-
-    return prc
-
-  if prc.kind in {nnkProcDef, nnkLambda, nnkMethodDef, nnkDo} and
+  elif prc.kind in {nnkProcDef, nnkLambda, nnkMethodDef, nnkDo} and
       not isEmpty(prc.body):
-    # don't do anything with forward bodies (empty)
     let
-      prcName = prc.name.getName
       setResultSym = ident "setResult"
-      procBody = prc.body.processBody(setResultSym, baseType)
-      internalFutureSym = ident "chronosInternalRetFuture"
-      castFutureSym = nnkCast.newTree(internalFutureType, internalFutureSym)
+      procBody = prc.body.processBody(setResultSym)
       resultIdent = ident "result"
-
-      resultDecl = nnkWhenStmt.newTree(
-        # when `baseType` is void:
-        nnkElifExpr.newTree(
-          nnkInfix.newTree(ident "is", baseType, ident "void"),
-          quote do:
-            template result: auto {.used.} =
-              {.fatal: "You should not reference the `result` variable inside" &
-                      " a void async proc".}
-        ),
-        # else:
-        nnkElseExpr.newTree(
-          newStmtList(
-            quote do: {.push warning[resultshadowed]: off.},
-            # var result {.used.}: `baseType`
-            # In the proc body, result may or may not end up being used
-            # depending on how the body is written - with implicit returns /
-            # expressions in particular, it is likely but not guaranteed that
-            # it is not used. Ideally, we would avoid emitting it in this
-            # case to avoid the default initializaiton. {.used.} typically
-            # works better than {.push.} which has a tendency to leak out of
-            # scope.
-            # TODO figure out if there's a way to detect `result` usage in
-            #      the proc body _after_ template exapnsion, and therefore
-            #      avoid creating this variable - one option is to create an
-            #      addtional when branch witha fake `result` and check
-            #      `compiles(procBody)` - this is not without cost though
-            nnkVarSection.newTree(nnkIdentDefs.newTree(
-              nnkPragmaExpr.newTree(
-                resultIdent,
-                nnkPragma.newTree(ident "used")),
-              baseType, newEmptyNode())
-              ),
-            quote do: {.pop.},
+      fakeResult = quote do:
+        template result: auto {.used.} =
+          {.fatal: "You should not reference the `result` variable inside" &
+                  " a void async proc".}
+      resultDecl =
+        if baseTypeIsVoid: fakeResult
+        else: nnkWhenStmt.newTree(
+          # when `baseType` is void:
+          nnkElifExpr.newTree(
+            nnkInfix.newTree(ident "is", baseType, ident "void"),
+            fakeResult
+          ),
+          # else:
+          nnkElseExpr.newTree(
+            newStmtList(
+              quote do: {.push warning[resultshadowed]: off.},
+              # var result {.used.}: `baseType`
+              # In the proc body, result may or may not end up being used
+              # depending on how the body is written - with implicit returns /
+              # expressions in particular, it is likely but not guaranteed that
+              # it is not used. Ideally, we would avoid emitting it in this
+              # case to avoid the default initializaiton. {.used.} typically
+              # works better than {.push.} which has a tendency to leak out of
+              # scope.
+              # TODO figure out if there's a way to detect `result` usage in
+              #      the proc body _after_ template exapnsion, and therefore
+              #      avoid creating this variable - one option is to create an
+              #      addtional when branch witha fake `result` and check
+              #      `compiles(procBody)` - this is not without cost though
+              nnkVarSection.newTree(nnkIdentDefs.newTree(
+                nnkPragmaExpr.newTree(
+                  resultIdent,
+                  nnkPragma.newTree(ident "used")),
+                baseType, newEmptyNode())
+                ),
+              quote do: {.pop.},
+            )
           )
         )
-      )
 
-      # generates:
+      # ```nim
       # template `setResultSym`(code: untyped) {.used.} =
       #   when typeof(code) is void: code
       #   else: `resultIdent` = code
+      # ```
       #
       # this is useful to handle implicit returns, but also
       # to bind the `result` to the one we declare here
@@ -415,6 +419,8 @@ proc asyncSingleProc(prc, params: NimNode): NimNode {.compileTime.} =
             )
           )
 
+      internalFutureSym = ident "chronosInternalRetFuture"
+      castFutureSym = nnkCast.newTree(internalFutureType, internalFutureSym)
       # Wrapping in try/finally ensures that early returns are handled properly
       # and that `defer` is processed in the right scope
       completeDecl = wrapInTryFinally(
@@ -429,18 +435,13 @@ proc asyncSingleProc(prc, params: NimNode): NimNode {.compileTime.} =
 
       internalFutureParameter = nnkIdentDefs.newTree(
         internalFutureSym, newIdentNode("FutureBase"), newEmptyNode())
+      prcName = prc.name.getName
       iteratorNameSym = genSym(nskIterator, $prcName)
       closureIterator = newProc(
         iteratorNameSym,
         [newIdentNode("FutureBase"), internalFutureParameter],
         closureBody, nnkIteratorDef)
 
-      outerProcBody = newNimNode(nnkStmtList, prc.body)
-
-    # Copy comment for nimdoc
-    if prc.body.len > 0 and prc.body[0].kind == nnkCommentStmt:
-      outerProcBody.add(prc.body[0])
-
     iteratorNameSym.copyLineInfo(prc)
 
     closureIterator.pragma = newNimNode(nnkPragma, lineInfoFrom=prc.body)
@@ -455,39 +456,56 @@ proc asyncSingleProc(prc, params: NimNode): NimNode {.compileTime.} =
       nnkBracket.newTree()
     ))
 
+    # The body of the original procedure (now moved to the iterator) is replaced
+    # with:
+    #
+    # ```nim
+    # let resultFuture = newFuture[T]()
+    # resultFuture.internalClosure = `iteratorNameSym`
+    # futureContinue(resultFuture)
+    # return resultFuture
+    # ```
+    #
+    # Declared at the end to be sure that the closure doesn't reference it,
+    # avoid cyclic ref (#203)
+    #
+    # Do not change this code to `quote do` version because `instantiationInfo`
+    # will be broken for `newFuture()` call.
+
+    let
+      outerProcBody = newNimNode(nnkStmtList, prc.body)
+
+    # Copy comment for nimdoc
+    if prc.body.len > 0 and prc.body[0].kind == nnkCommentStmt:
+      outerProcBody.add(prc.body[0])
+
     outerProcBody.add(closureIterator)
 
-    # -> let resultFuture = newInternalRaisesFuture[T, E]()
-    # declared at the end to be sure that the closure
-    # doesn't reference it, avoid cyclic ref (#203)
     let
       retFutureSym = ident "resultFuture"
       newFutProc = if raises == nil:
-        newTree(nnkBracketExpr, ident "newFuture", baseType)
+        nnkBracketExpr.newTree(ident "newFuture", baseType)
       else:
-        newTree(nnkBracketExpr, ident "newInternalRaisesFuture", baseType, raises)
+        nnkBracketExpr.newTree(ident "newInternalRaisesFuture", baseType, raises)
+
     retFutureSym.copyLineInfo(prc)
-    # Do not change this code to `quote do` version because `instantiationInfo`
-    # will be broken for `newFuture()` call.
     outerProcBody.add(
       newLetStmt(
         retFutureSym,
         newCall(newFutProc, newLit(prcName))
       )
     )
-    # -> resultFuture.internalClosure = iterator
+
     outerProcBody.add(
       newAssignment(
         newDotExpr(retFutureSym, newIdentNode("internalClosure")),
         iteratorNameSym)
     )
 
-    # -> futureContinue(resultFuture))
     outerProcBody.add(
         newCall(newIdentNode("futureContinue"), retFutureSym)
     )
 
-    # -> return resultFuture
     outerProcBody.add newNimNode(nnkReturnStmt, prc.body[^1]).add(retFutureSym)
 
     prc.body = outerProcBody
@@ -498,6 +516,13 @@ proc asyncSingleProc(prc, params: NimNode): NimNode {.compileTime.} =
   prc
 
 template await*[T](f: Future[T]): T =
+  ## Ensure that the given `Future` is finished, then return its value.
+  ##
+  ## If the `Future` failed or was cancelled, the corresponding exception will
+  ## be raised instead.
+  ##
+  ## If the `Future` is pending, execution of the current `async` procedure
+  ## will be suspended until the `Future` is finished.
   when declared(chronosInternalRetFuture):
     chronosInternalRetFuture.internalChild = f
     # `futureContinue` calls the iterator generated by the `async`
@@ -512,18 +537,26 @@ template await*[T](f: Future[T]): T =
   else:
     unsupported "await is only available within {.async.}"
 
-template await*[T, E](f: InternalRaisesFuture[T, E]): T =
+template await*[T, E](fut: InternalRaisesFuture[T, E]): T =
+  ## Ensure that the given `Future` is finished, then return its value.
+  ##
+  ## If the `Future` failed or was cancelled, the corresponding exception will
+  ## be raised instead.
+  ##
+  ## If the `Future` is pending, execution of the current `async` procedure
+  ## will be suspended until the `Future` is finished.
   when declared(chronosInternalRetFuture):
-    chronosInternalRetFuture.internalChild = f
+    chronosInternalRetFuture.internalChild = fut
     # `futureContinue` calls the iterator generated by the `async`
     # transformation - `yield` gives control back to `futureContinue` which is
     # responsible for resuming execution once the yielded future is finished
     yield chronosInternalRetFuture.internalChild
     # `child` released by `futureContinue`
-    cast[type(f)](chronosInternalRetFuture.internalChild).internalCheckComplete(E)
+    cast[type(fut)](
+      chronosInternalRetFuture.internalChild).internalCheckComplete(E)
 
     when T isnot void:
-      cast[type(f)](chronosInternalRetFuture.internalChild).value()
+      cast[type(fut)](chronosInternalRetFuture.internalChild).value()
   else:
     unsupported "await is only available within {.async.}"
 
diff --git a/docs/src/async_procs.md b/docs/src/async_procs.md
index ae8eb51..648f19b 100644
--- a/docs/src/async_procs.md
+++ b/docs/src/async_procs.md
@@ -1,5 +1,13 @@
 # Async procedures
 
+Async procedures are those that interact with `chronos` to cooperatively
+suspend and resume their execution depending on the completion of other
+async procedures which themselves may be waiting for I/O to complete, timers to
+expire or tasks running on other threads to complete.
+
+Async procedures are marked with the `{.async.}` pragma and return a `Future`
+indicating the state of the operation.
+
 <!-- toc -->
 
 ## The `async` pragma
@@ -20,8 +28,8 @@ echo p().type # prints "Future[system.void]"
 Whenever `await` is encountered inside an async procedure, control is given
 back to the dispatcher for as many steps as it's necessary for the awaited
 future to complete, fail or be cancelled. `await` calls the
-equivalent of `Future.read()` on the completed future and returns the
-encapsulated value.
+equivalent of `Future.read()` on the completed future to return the
+encapsulated value when the operation finishes.
 
 ```nim
 proc p1() {.async.} =
@@ -51,10 +59,10 @@ In particular, if two `async` procedures have access to the same mutable state,
 the value before and after `await` might not be the same as the order of execution is not guaranteed!
 ```
 
-## Raw functions
+## Raw procedures
 
-Raw functions are those that interact with `chronos` via the `Future` type but
-whose body does not go through the async transformation.
+Raw async procedures are those that interact with `chronos` via the `Future`
+type but whose body does not go through the async transformation.
 
 Such functions are created by adding `raw: true` to the `async` parameters:
 
diff --git a/docs/src/concepts.md b/docs/src/concepts.md
index fcc33af..0469b8b 100644
--- a/docs/src/concepts.md
+++ b/docs/src/concepts.md
@@ -1,12 +1,13 @@
 # Concepts
 
+Async/await is a programming model that relies on cooperative multitasking to
+coordinate the concurrent execution of procedures, using event notifications
+from the operating system or other treads to resume execution.
+
 <!-- toc -->
 
 ## The dispatcher
 
-Async/await programming relies on cooperative multitasking to coordinate the
-concurrent execution of procedures, using event notifications from the operating system to resume execution.
-
 The event handler loop is called a "dispatcher" and a single instance per
 thread is created, as soon as one is needed.
 
@@ -16,6 +17,9 @@ progress, for example because it's waiting for some data to arrive, it hands
 control back to the dispatcher which ensures that the procedure is resumed when
 ready.
 
+A single thread, and thus a single dispatcher, is typically able to handle
+thousands of concurrent in-progress requests.
+
 ## The `Future` type
 
 `Future` objects encapsulate the outcome of executing an `async` procedure. The
@@ -69,13 +73,14 @@ structured this way.
 Both `waitFor` and `runForever` call `poll` which offers fine-grained
 control over the event loop steps.
 
-Nested calls to `poll`, `waitFor` and `runForever` are not allowed.
+Nested calls to `poll` - directly or indirectly via `waitFor` and `runForever`
+are not allowed.
 ```
 
 ## Cancellation
 
 Any pending `Future` can be cancelled. This can be used for timeouts, to start
-multiple operations in parallel and cancel the rest as soon as one finishes,
+multiple parallel operations and cancel the rest as soon as one finishes,
 to initiate the orderely shutdown of an application etc.
 
 ```nim
@@ -110,7 +115,10 @@ waitFor(work.cancelAndWait())
 ```
 
 The `CancelledError` will now travel up the stack like any other exception.
-It can be caught and handled (for instance, freeing some resources)
+It can be caught for instance to free some resources and is then typically
+re-raised for the whole chain operations to get cancelled.
+
+Alternatively, the cancellation request can be translated to a regular outcome of the operation - for example, a `read` operation might return an empty result.
 
 Cancelling an already-finished `Future` has no effect, as the following example
 of downloading two web pages concurrently shows:
diff --git a/docs/src/error_handling.md b/docs/src/error_handling.md
index be06a35..54c1236 100644
--- a/docs/src/error_handling.md
+++ b/docs/src/error_handling.md
@@ -85,6 +85,21 @@ the operation they implement might get cancelled resulting in neither value nor
 error!
 ```
 
+When using checked exceptions, the `Future` type is modified to include
+`raises` information - it can be constructed with the `Raising` helper:
+
+```nim
+# Create a variable of the type that will be returned by a an async function
+# raising `[CancelledError]`:
+var fut: Future[int].Raising([CancelledError])
+```
+
+```admonition note
+`Raising` creates a specialization of `InternalRaisesFuture` type - as the name
+suggests, this is an internal type whose implementation details are likely to
+change in future `chronos` versions.
+```
+
 ## The `Exception` type
 
 Exceptions deriving from `Exception` are not caught by default as these may
diff --git a/docs/src/porting.md b/docs/src/porting.md
index 519de64..1bdffe2 100644
--- a/docs/src/porting.md
+++ b/docs/src/porting.md
@@ -16,20 +16,25 @@ here are several things to consider:
 * Exception handling is now strict by default - see the [error handling](./error_handling.md)
   chapter for how to deal with `raises` effects
 * `AsyncEventBus` was removed - use `AsyncEventQueue` instead
+* `Future.value` and `Future.error` panic when accessed in the wrong state
+* `Future.read` and `Future.readError` raise `FutureError` instead of
+  `ValueError` when accessed in the wrong state
 
 ## `asyncdispatch`
 
-Projects written for `asyncdispatch` and `chronos` look similar but there are
+Code written for `asyncdispatch` and `chronos` looks similar but there are
 several differences to be aware of:
 
 * `chronos` has its own dispatch loop - you can typically not mix `chronos` and
   `asyncdispatch` in the same thread
 * `import chronos` instead of `import asyncdispatch`
 * cleanup is important - make sure to use `closeWait` to release any resources
-  you're using or file descript leaks and other
+  you're using or file descriptor and other leaks will ensue
 * cancellation support means that `CancelledError` may be raised from most
   `{.async.}` functions
 * Calling `yield` directly in tasks is not supported - instead, use `awaitne`.
+* `asyncSpawn` is used instead of `asyncCheck` - note that exceptions raised
+  in tasks that are `asyncSpawn`:ed cause panic
 
 ## Supporting multiple backends