Merge pull request #67 from smartcontractkit/issue#56-response-listener-hangs

Issue#56 response listener hangs

Author: zeuslawyer

Diff for: .changeset/silly-berries-beam.md

@@ -0,0 +1,5 @@
+---
+'@chainlink/functions-toolkit': patch
+---
+
+Change default minimum confirmations from listenForResponseFromTransaction() to 1.

Diff for: README.md

@@ -452,20 +452,22 @@ functionsRouterAddress,
 
 To listen for a response to a single Functions request, use the `listenForResponseFromTransaction()` method.
 Optionally, you can provide:
-- timeout after which the listener will throw an error indicating that the time limit was exceeded (default 5 minutes) 
-- number of block confirmations (default 2)
-- frequency of checking if the request is already included on-chain (or if it got moved after a chain re-org) (default 2 seconds)
+
+- timeout after which the listener will throw an error indicating that the time limit was exceeded (default 5 minutes expressed in milliseconds)
+- number of block confirmations (default 1, but note that should be 2 or more to for higher confidence in finality, and to protect against reorgs)
+- frequency of checking if the request is already included on-chain (or if it got moved after a chain re-org) (default 2 seconds, but note that `checkInterval`s higher than block time could cause this listener to hang as the response will have completed before the next check.)
 
 ```
 const response: FunctionsResponse = await responseListener.listenForResponseFromTransaction(
   txHash: string,
-  timeout?: number,
+  timeoutMs?: number, // milliseconds
   confirmations?: number,
   checkInterval?: number,
 )
 ```
 
 `listenForResponseFromTransaction()` returns a response with the following structure:
+
 ```
 {
   requestId: string // Request ID of the fulfilled request represented as a bytes32 hex string
@@ -480,7 +482,8 @@ const response: FunctionsResponse = await responseListener.listenForResponseFrom
 
 Alternatively, to listen using a request ID, use the `listenForResponse()` method.
 
-**Notes:** 
+**Notes:**
+
 1. Request ID can change during a chain re-org so it's less reliable than a request transaction hash.
 2. If the methods are called after the response is already on chain, it won't be returned correctly.
 3. Listening for multiple responses simultaneously is not supported by the above methods and will lead to undefined behavior.
@@ -578,6 +581,7 @@ Any 3rd party imports used in the JavaScript source code are loaded asynchronous
 const { format } = await import("npm:date-fns");
 return Functions.encodeString(format(new Date(), "yyyy-MM-dd"));
 ```
+
 ```
 const { escape } = await import("https://deno.land/std/regexp/mod.ts");
 return Functions.encodeString(escape("$hello*world?"));
@@ -720,6 +724,6 @@ const functionsRequestBytesHexString: string = buildRequestCBOR({
 })
 ```
 
-
 ## Browser use
-This package can also be used in most modern web browsers. You can import the package in your front-end application, and call the APIs as you would in a back end NodeJs/Deno environment.  
+
+This package can also be used in most modern web browsers. You can import the package in your front-end application, and call the APIs as you would in a back end NodeJs/Deno environment.

Diff for: src/ResponseListener.ts

@@ -21,12 +21,15 @@ export class ResponseListener {
     this.functionsRouter = new Contract(functionsRouterAddress, FunctionsRouterSource.abi, provider)
   }
 
-  public async listenForResponse(requestId: string, timeout = 300000): Promise<FunctionsResponse> {
+  public async listenForResponse(
+    requestId: string,
+    timeoutMs = 300000,
+  ): Promise<FunctionsResponse> {
     let expirationTimeout: NodeJS.Timeout
     const responsePromise = new Promise<FunctionsResponse>((resolve, reject) => {
       expirationTimeout = setTimeout(() => {
         reject('Response not received within timeout period')
-      }, timeout)
+      }, timeoutMs)
 
       this.functionsRouter.on(
         'RequestProcessed',
@@ -60,11 +63,19 @@ export class ResponseListener {
     return responsePromise
   }
 
+  /**
+   *
+   * @param txHash Tx hash for the Functions Request
+   * @param timeoutMs after which the listener throws, indicating  the time limit was exceeded (default 5 minutes)
+   * @param confirmations  number of confirmations to wait for before considering the transaction successful (default 1, but recommend 2 or more)
+   * @param checkIntervalMs frequency of checking if the Tx is  included on-chain (or if it got moved after a chain re-org) (default 2 seconds. Intervals longer than block time may cause the listener to wait indefinitely.)
+   * @returns
+   */
   public async listenForResponseFromTransaction(
     txHash: string,
-    timeout = 3000000,
-    confirmations = 2,
-    checkInterval = 2000,
+    timeoutMs = 3000000,
+    confirmations = 1,
+    checkIntervalMs = 2000,
   ): Promise<FunctionsResponse> {
     return new Promise<FunctionsResponse>((resolve, reject) => {
       ;(async () => {
@@ -73,14 +84,14 @@ export class ResponseListener {
         let checkTimeout: NodeJS.Timeout
         const expirationTimeout = setTimeout(() => {
           reject('Response not received within timeout period')
-        }, timeout)
+        }, timeoutMs)
 
         const check = async () => {
-          const receipt = await this.provider.waitForTransaction(txHash, confirmations, timeout)
+          const receipt = await this.provider.waitForTransaction(txHash, confirmations, timeoutMs)
           const updatedId = receipt.logs[0].topics[1]
           if (updatedId !== requestId) {
             requestId = updatedId
-            const response = await this.listenForResponse(requestId, timeout)
+            const response = await this.listenForResponse(requestId, timeoutMs)
             if (updatedId === requestId) {
               // Resolve only if the ID hasn't changed in the meantime
               clearTimeout(expirationTimeout)
@@ -91,7 +102,7 @@ export class ResponseListener {
         }
 
         // Check periodically if the transaction has been re-orged and requestID changed
-        checkTimeout = setInterval(check, checkInterval)
+        checkTimeout = setInterval(check, checkIntervalMs)
 
         check()
       })()