fix: pr comments

ffi/codegen/meta.nim

@@ -67,19 +67,22 @@ proc abiCodegenImplemented*(fmt: ABIFormat): bool =
   ## seam a future PR flips once the `c` dispatch path is wired.
   fmt == ABIFormat.Cbor
 
-proc specKey*(spec: string): string =
-  ## Lowercased key of a `key = value` annotation spec (the text before `=`),
-  ## used to route a spec to its parser. `"timeout = 30000"` → `"timeout"`.
-  spec.split('=')[0].strip().toLowerAscii()
+proc overrideKey*(override: string): string =
+  ## Lowercased key of a `key = value` pragma override (the text before `=`),
+  ## used to route it to its parser. `"timeout = 30000"` → `"timeout"`.
+  override.split('=')[0].strip().toLowerAscii()
 
-proc parseTimeoutSpec*(spec: string): tuple[ok: bool, ms: int, err: string] =
+proc parseTimeoutSpec*(override: string): tuple[ok: bool, ms: int, err: string] =
   ## Parse a `"timeout = <milliseconds>"` override (whitespace/case tolerant).
   ## The value must be a positive integer number of milliseconds. On bad
   ## grammar or value, returns `ok = false` with a human-readable `err`.
-  let parts = spec.split('=')
-  if parts.len != 2 or specKey(spec) != "timeout":
-    return
-      (false, 0, "invalid timeout override '" & spec & "'; expected `timeout = <ms>`")
+  let parts = override.split('=')
+  if parts.len != 2 or overrideKey(override) != "timeout":
+    return (
+      false,
+      0,
+      "invalid timeout override: '" & override & "'; expected `timeout = <ms>`",
+    )
   let raw = parts[1].strip()
   let ms =
     try:
@@ -88,7 +91,7 @@ proc parseTimeoutSpec*(spec: string): tuple[ok: bool, ms: int, err: string] =
       return (
         false,
         0,
-        "invalid timeout value '" & raw &
+        "invalid timeout value: '" & raw &
           "'; expected a positive integer of milliseconds",
       )
   if ms <= 0:
@@ -106,28 +109,28 @@ proc parseABIFormatName*(name: string): tuple[ok: bool, fmt: ABIFormat] =
   else:
     (false, ABIFormat.Cbor)
 
-proc parseAbiSpec*(spec: string): tuple[ok: bool, fmt: ABIFormat, err: string] =
+proc parseAbiSpec*(override: string): tuple[ok: bool, fmt: ABIFormat, err: string] =
   ## Parse an `"abi = <format>"` override (whitespace/case tolerant). On bad
   ## grammar or format, returns `ok = false` with a human-readable `err`.
-  let parts = spec.split('=')
+  let parts = override.split('=')
   if parts.len != 2:
     return (
       false,
       ABIFormat.Cbor,
-      "invalid ABI override '" & spec & "'; expected `abi = c` or `abi = cbor`",
+      "invalid ABI override: '" & override & "'; expected `abi = c` or `abi = cbor`",
     )
   if parts[0].strip().toLowerAscii() != "abi":
     return (
       false,
       ABIFormat.Cbor,
-      "invalid ABI override '" & spec & "'; expected `abi = c` or `abi = cbor`",
+      "invalid ABI override: '" & override & "'; expected `abi = c` or `abi = cbor`",
     )
   let (ok, fmt) = parseABIFormatName(parts[1])
   if not ok:
     return (
       false,
       ABIFormat.Cbor,
-      "unknown ABI format '" & parts[1].strip() & "'; valid values are `c` and `cbor`",
+      "unknown ABI format: '" & parts[1].strip() & "'; valid values are `c` and `cbor`",
     )
   (true, fmt, "")
 

ffi/ffi_context.nim

@@ -43,11 +43,8 @@ type FFIContext*[T] = object
     # Per-proc timeout overrides (ms). Points at the compile-time-filled global,
     # like registeredRequests, so the FFI thread reads it GC-safely via ctx.
   defaultRequestTimeout*: Duration
-    # Deadline applied to each handler unless a `{.ffi: "timeout = <ms>".}`
-    # override raises it. On trip the caller is unblocked with a timeout err and
-    # the handler is left running (see processRequest). Set `InfiniteDuration`
-    # to opt out. Written on the owning thread before the first request; read on
-    # the FFI thread.
+    # Per-handler deadline unless a `{.ffi: "timeout = <ms>".}` override raises
+    # it; `InfiniteDuration` opts out. See processRequest for the trip behavior.
 
 var onFFIThread* {.threadvar.}: bool
   # Re-entrant dispatch guard for `sendRequestToFFIThread`.
@@ -59,8 +56,7 @@ const
   FFIHeartbeatStartDelay* = 10.seconds # grace window for library startup
   FFIHeartbeatStaleThreshold* = 1.seconds
   DefaultRequestTimeout* = 5.seconds
-    # A guess (issue #93): finite so a wedged handler can't hang a caller
-    # forever, generous enough to clear normal handlers. Overridable per proc.
+    # Finite fallback (issue #93) so a wedged handler can't hang a caller forever.
 
 include ./event_thread
 include ./ffi_thread

ffi/ffi_thread.nim

@@ -60,7 +60,8 @@ proc reportTimeoutIfTripped(
   ## deliberately do NOT cancel the handler: a hard-cancel mid-call into the
   ## underlying library (Waku/libp2p) can leave it partially applied, so we
   ## unblock the caller with a timeout err now and let the handler run to
-  ## completion. `respondOnce` keeps the two paths from answering twice.
+  ## completion. `fireCallback`'s once-only guard keeps the two paths from
+  ## answering twice.
   if deadline == InfiniteDuration:
     return
   # Handlers that already completed (e.g. a sync body) skip the timer entirely,
@@ -77,10 +78,11 @@ proc reportTimeoutIfTripped(
     return
   warn "ffi request timed out; caller unblocked, handler left running",
     reqId = reqId, timeoutMs = deadline.milliseconds
-  request.respondOnce(
+  fireCallback(
     Result[seq[byte], string].err(
       "ffi request timed out after " & $deadline.milliseconds & "ms"
-    )
+    ),
+    request,
   )
 
 proc processRequest[T](

ffi/ffi_thread_request.nim

@@ -129,9 +129,16 @@ proc deleteRequest*(request: ptr FFIThreadRequest) =
     c_free(cast[pointer](request[].reqId))
   c_free(request)
 
-proc fireCallback(res: Result[seq[byte], string], request: ptr FFIThreadRequest) =
-  ## Delivers one response to the foreign callback. Success payload is CBOR
-  ## bytes; error payload is the raw UTF-8 error string.
+proc fireCallback*(res: Result[seq[byte], string], request: ptr FFIThreadRequest) =
+  ## Delivers the response to the foreign callback, at most once per request:
+  ## the timeout path and the handler-completion path both call it, but the
+  ## foreign side must be answered exactly once. Both run on the FFI thread, so
+  ## the plain `responded` flag needs no synchronization. Success payload is the
+  ## encoded response bytes; error payload is the raw UTF-8 error string. Does
+  ## NOT free the request; that stays with `handleRes`.
+  if request[].responded:
+    return
+  request[].responded = true
   if res.isErr():
     foreignThreadGc:
       let msg = if res.error.len > 0: res.error else: EmptyErrorMarker
@@ -156,23 +163,12 @@ proc fireCallback(res: Result[seq[byte], string], request: ptr FFIThreadRequest)
         RET_OK, cast[ptr cchar](addr sentinel), 1.csize_t, request[].userData
       )
 
-proc respondOnce*(request: ptr FFIThreadRequest, res: Result[seq[byte], string]) =
-  ## Fires the callback the first time it's called for `request` and no-ops
-  ## after — the timeout path and the handler-completion path both call it, but
-  ## the foreign side must be answered exactly once. Does NOT free the request:
-  ## freeing stays with `handleRes` so the handler always owns the buffer until
-  ## it finishes.
-  if request[].responded:
-    return
-  request[].responded = true
-  fireCallback(res, request)
-
 proc handleRes*(res: Result[seq[byte], string], request: ptr FFIThreadRequest) =
   ## Terminal step of every request: delivers the response (unless a timeout
   ## already did) and frees the request exactly once.
   defer:
     deleteRequest(request)
-  respondOnce(request, res)
+  fireCallback(res, request)
 
 proc nilProcess*(reqId: cstring): Future[Result[seq[byte], string]] {.async.} =
   return err("This request type is not implemented: " & $reqId)

ffi/internal/ffi_macro.nim

@@ -22,12 +22,13 @@ proc resolveABIFormat(abiSpecs: seq[NimNode]): ABIFormat {.compileTime.} =
   ## Resolve one annotation's ABI from its optional `"abi = ..."` string specs
   ## (last wins), inheriting the library default when absent.
   var fmt = currentDefaultABIFormat
-  for spec in abiSpecs:
-    if spec.kind notin {nnkStrLit, nnkRStrLit, nnkTripleStrLit}:
+  for override in abiSpecs:
+    if override.kind notin {nnkStrLit, nnkRStrLit, nnkTripleStrLit}:
       error(
-        "FFI ABI override must be a string literal like \"abi = c\", got: " & spec.repr
+        "FFI ABI override must be a string literal like \"abi = c\", got: " &
+          override.repr
       )
-    let parsed = parseAbiSpec($spec)
+    let parsed = parseAbiSpec($override)
     if not parsed.ok:
       error(parsed.err)
     fmt = parsed.fmt
@@ -41,26 +42,27 @@ proc resolveFFISpecs(
   ## `timeoutMs == 0` means "no per-proc override" (use the context default).
   var abi = currentDefaultABIFormat
   var timeoutMs = 0
-  for spec in specs:
-    if spec.kind notin {nnkStrLit, nnkRStrLit, nnkTripleStrLit}:
+  for override in specs:
+    if override.kind notin {nnkStrLit, nnkRStrLit, nnkTripleStrLit}:
       error(
         "FFI override must be a string literal like \"abi = c\" or " &
-          "\"timeout = 30000\", got: " & spec.repr
+          "\"timeout = 30000\", got: " & override.repr
       )
-    case specKey($spec)
+    case overrideKey($override)
     of "abi":
-      let parsed = parseAbiSpec($spec)
+      let parsed = parseAbiSpec($override)
       if not parsed.ok:
         error(parsed.err)
       abi = parsed.fmt
     of "timeout":
-      let parsed = parseTimeoutSpec($spec)
+      let parsed = parseTimeoutSpec($override)
       if not parsed.ok:
         error(parsed.err)
       timeoutMs = parsed.ms
     else:
       error(
-        "unknown FFI override '" & $spec & "'; expected `abi = ...` or `timeout = ...`"
+        "unknown FFI override '" & $override &
+          "'; expected `abi = ...` or `timeout = ...`"
       )
   (abi, timeoutMs)
 
@@ -802,7 +804,7 @@ macro ffi*(args: varargs[untyped]): untyped =
   # a TypeDef; `cbor` rides the generic overloads. Both abis are valid here.
   if prc.kind == nnkTypeDef:
     if timeoutMs > 0:
-      error("`.ffi.` on a type takes no `timeout` override (it applies to procs)")
+      error("`.ffi.` on a type takes no `timeout` override (it only applies to procs)")
     gateFFITypeABIFormat(abiFormat, "`.ffi.` type")
     var cleanTypeDef = prc.copyNimTree()
     if cleanTypeDef[0].kind == nnkPragmaExpr:

tests/unit/test_abi_format.nim

@@ -99,10 +99,10 @@ suite "ABI format parsing":
     check "bson" in parseAbiSpec("abi = bson").err
 
 suite "handler-timeout spec parsing (issue #93)":
-  test "specKey extracts the lowercased, trimmed key":
-    check specKey("timeout = 30000") == "timeout"
-    check specKey("  ABI = c ") == "abi"
-    check specKey("bare") == "bare"
+  test "overrideKey extracts the lowercased, trimmed key":
+    check overrideKey("timeout = 30000") == "timeout"
+    check overrideKey("  ABI = c ") == "abi"
+    check overrideKey("bare") == "bare"
 
   test "parseTimeoutSpec accepts `timeout = <ms>`, flexible spacing":
     check parseTimeoutSpec("timeout = 30000") == (true, 30000, "")