2

I am trying to fetch a record from a database. Due to race conditions it is possible and even likely that the record isn't there when I first try to fetch it. How do I wrap this in a retry logic without going mad? I seem to be too stupid for it

  const booking = await strapi.query("api::booking.booking").findOne({
    where: {
      id: id,
    },
  });

This code should retry n times with a delay of t milliseconds. Thanks and much love.

What I've tried:

async function tryFetchBooking(
  id,
  max_retries = 3,
  current_try = 0,
  promise
) {
  promise = promise || new Promise();

  // try doing the important thing
  const booking = await strapi.query("api::booking.booking").findOne({
    where: {
      id: id,
    },
  });

  if (!booking) {
    if (current_try < max_retries) {
      console.log("No booking. Retrying");
      setTimeout(function () {
        tryFetchBooking(id, max_retries, current_try + 1, promise);
      }, 500);
    } else {
      console.log("No booking. Giving up.");
      promise.reject(new Error("no booking found in time"));
    }
    promise.catch(() => {
      throw new Error(`Failed retrying 3 times`);
    });
  } else {
    console.log("Found booking with retry");
    promise.resolve(booking);
  }
}

const booking = await tryFetchBooking(id);

The thrown error:

This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason:
TypeError: Promise resolver undefined is not a function
Ole Spaarmann
  • 15,845
  • 27
  • 98
  • 160
  • Something like that? https://stackoverflow.com/a/44577075/5767872 – Sergey Sosunov Jul 21 '22 at 21:58
  • @SergeySosunov could you give me an example how to use it in my case? I'm struggling with the phrase "This is how you call it, assuming that func sometimes succeeds and sometimes fails, always returning a string that we can log:" So how does my function of fetching a booking would fail? It either returns a record or nothing. – Ole Spaarmann Jul 21 '22 at 22:11

3 Answers3

5

That promise.reject()/promise.resolve() approach is not going to work, you cannot resolve a promise from the outside. And you shouldn't need to - just return/throw from your async function! The only place where you need to construct a new Promise is in a little helper function

function delay(t) {
  return new Promise(resolve => {
    setTimeout(resolve, t);
  });
}

Then you can write your function in a recursive manner:

async function tryFetchBooking(
  id,
  max_retries = 3,
  current_try = 0,
) {
  let booking = await strapi.query("api::booking.booking").findOne({
    where: {
      id: id,
    },
  });

  if (!booking) {
    if (current_try < max_retries) {
      console.log("No booking. Retrying");
      await delay(500);
//    ^^^^^^^^^^^^^^^^
      booking = await tryFetchBooking(id, max_retries, current_try + 1);
//              ^^^^^^^^^^^^^^^^^^^^^
      console.log("Found booking with retry");
    } else {
      console.log("No booking. Giving up.");
      throw new Error("no booking found in time");
      // or if you prefer the other error message:
      throw new Error(`Failed retrying 3 times`);
    }
  }
  return booking;
}

or even in an iterative manner:

async function tryFetchBooking(id, maxRetries = 3) {
  let currentTry = 0;
  while (true) {
    const booking = await strapi.query("api::booking.booking").findOne({
      where: {
        id: id,
      },
    });

    if (booking) {
      return booking;
    }
    if (currentTry < maxRetries) {
      await delay(500);
      currentTry++;
    } else {
      console.log("No booking. Giving up.");
      throw new Error("no booking found in time");
    }
  }
}
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • That worked. Thanks! Only issue with your first solution was an thrown error because of re-assignment to a constant variable (booking). But that was an easy fix. – Ole Spaarmann Jul 21 '22 at 22:50
0
import { strapiMock } from "./mock";

const wait = (ms) => new Promise((r) => setTimeout(r, ms));

// Credits to @Bergi
const retryOperation = (operation, delay, retries) =>
  operation().catch((reason) =>
    retries > 0
      ? wait(delay).then(() => retryOperation(operation, delay, retries - 1))
      : Promise.reject(reason)
  );

const throwIfNoResult = (result) => {
  if (!result) throw new Error("No result");
  return result;
};

const fetchBooking = (id) => {
  /*
  return strapi.query("api::booking.booking").findOne({
    where: {
      id: id
    }
  });
  */
  return strapiMock(id);
};

async function tryFetchBooking(id, delay = 1000, retries = 4) {
  const operation = () => fetchBooking(id).then(throwIfNoResult);
  const wrapped = retryOperation(operation, delay, retries);
  return await wrapped;
}

tryFetchBooking(1).then(console.log).catch(console.error);

Mock used:

let cnt = 0;

export const strapiMock = (id) => {
  return new Promise((resolve, reject) => {
    if (cnt++ === 3) {
      cnt = 0;
      // resolve(null);
      resolve(id);
    } else {
      reject("no data");
    }
  });
};

Edit silly-stonebraker-tclsw8

Sergey Sosunov
  • 4,124
  • 2
  • 11
  • 15
  • Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! – Bergi Jul 21 '22 at 22:34
  • @Bergi please, if you can and have time, can you explain what undesirable consequences can my code lead to? I tried to choose the most convenient approach in terms of reuse, your answer is obviously better, but if I have to do 10-20 methods in the same way - I'll get a little upset – Sergey Sosunov Jul 21 '22 at 22:46
  • 1
    It's simply much more complicated, and much harder to get the error handling right (although you did achieve that afaics). So simplify to `const retryOperation = (operation, delay, retries) => operation().catch(reason => retries > 0 ? wait(delay).then(() => retryOperation(operation, delay, retries-1)) : Promise.reject(reason));` – Bergi Jul 21 '22 at 22:54
  • @Bergi Thank you, much better now – Sergey Sosunov Jul 21 '22 at 23:04
0

to avoid using await inside a loop, you can use recursive function to call tryFetchBooking

async function tryFetchBooking(id, currentRetry = 0) {
  const maxRetries = 3;
  const booking = await strapi.query("api::booking.booking").findOne({
    where: {
      id: id,
    },
  });
  if (booking) {
    return booking;
  }
  if (currentRetry < maxRetries) {
    await delay(500);
    currentTry++;
    return tryFetchBooking(id, currentRety);
  } else {
    console.log("No booking. Giving up.");
    throw new Error("no booking found in time");
  }
}
Giang
  • 2,384
  • 2
  • 25
  • 26