typescriptasync-await

Async capture and finally block


I came across some code that is working in production, but I have questions about how exactly this works correctly. The code looks like this:

private async someFunction {
  const client = await pool.connect()
  try {
    await client.query('BEGIN')
    //Some stuff
    await client.query('COMMIT')
    this.fireAndForgetAPICall(
      async (resultId) => {
        const updateResultQuery = `
        UPDATE projects SET result_id = $1 WHERE id = $2;
        `
        await client.query(updateResultQuery, [resultId, id])
      },
    )

    return resultingData
  } catch (error) {
    client.query('ROLLBACK')
    console.error('Database error:', error)
    throw error
  } finally {
    client.release()
  }
}

 private async fireAndForgetAPICall(
   completion?: (resultId: string) => Promise<void>,
 ) {

 try {
   let response = await axios.post(
     'https://someapi.com/api',
     {},
     {
       headers: {},
     },
   )
   if (completion) {
     await completion(response.data)
   }
 } catch (error) {
   console.error('Error calling API:', error)
 }
}

As you can see, the API calls an async function that is not awaited, as it's success is not pivotal to the main flow, and the rest of the process shouldn't block on it. However, in that block it captures a client, and seems to be able to perform an operation on it, even though the finally block should release that client.

Is this code written in a sound way? Does it just work on coincidence of execution timings? Does the finally block actually block on the fireAndForget function? Will the function actually return before this call finishes?

I'm a little curious about the interaction between the finally block and captured variables, I would have thought this code would fail, but it seems to work. Any insight would be helpful.


Solution

  • I've run and tested this code by "mocking" the pool and the client and I've come to the conclusion that no, it should not work:

    class Client {
      public canRelease = true;
    
      public async query(...args: any[]) {
        if (!this.canRelease) throw new Error("Already released");
      }
    
      public async release() {
        if (!this.canRelease) throw new Error("Already released");
        this.canRelease = false;
      }
    }
    
    const pool = {
      connect: async () => new Client(),
    };
    
    class TestClass {
      public async someFunction() {
        const client = await pool.connect();
        try {
          await client.query("BEGIN");
          //Some stuff
          await client.query("COMMIT");
          this.fireAndForgetAPICall(async (resultId) => {
            const updateResultQuery = `
            UPDATE projects SET result_id = $1 WHERE id = $2;
            `;
            await client.query(updateResultQuery, [resultId]);
          });
    
          //   return resultingData;
        } catch (error) {
          client.query("ROLLBACK");
          console.error("Database error:", error);
          throw error;
        } finally {
          client.release();
        }
      }
    
      private async fireAndForgetAPICall(
        completion ? : (resultId: string) => Promise < void >
      ) {
        try {
          let response = await new Promise < any > ((r) =>
            setTimeout(() => r({
              data: "hello there"
            }), 100)
          );
          if (completion) {
            await completion(response.data);
          }
        } catch (error) {
          console.error("Error calling API:", error);
        }
      }
    }
    (async () => {
      const a = new TestClass();
      await a.someFunction();
      console.log("DONE!");
    })();
    

    What we can see there is that, if client.query is called AFTER being released, it throws as expected.

    My best quess to what's happening here is that, your DB library simply does not throw if client.query is called after being released, that's why the code does not break. This however is very convoluted and not at easy to read, so I would, if u can, re-write it to be more readable. For example, in my own example you can see the logs Error calling API: when this is in fact, NOT an issue with the API call, but an issue executing the onCompletion callback.

    Here are the logs from my experiement:

    $ node test.ts
    DONE!
    Error calling API: Error: Already released