4

SO friends!

I need to make a call to an api; if it fails, I need to make a call to same api with different param; if it fails AGAIN, I need to make a call to same api with a third, different param; if it fails finally after that, is an actual error and can bugger out.

Only way I can figure to do this is with nested try/catch statements, ala:

const identityCheck = async (slug) => {
  let res;
  try {
    res = await Bundle.sdk.find(slug);
  } catch (err) {
    console.log('Fragment didn\'t work ========', slug, err);

    try {
      res = await Bundle.sdk.find(`package/${slug}`);
    } catch (e) {
      console.log('Fragment didn\'t work package ========', e);

      try {
        res = await Bundle.sdk.find(`${slug}-list`);
      } catch (error) {
        console.log('None of the fragments worked================.', error);
      }
    }
  }

  return logResponse(res);
};

identityCheck('fashion');

But seems like there MUST be another simpler way to do this. I tried boiling down into a retry function, but that just ends up being even more code and way less clear:

const identityCheck = (slug) => {
  const toTry = [
    slug,
    `package/${slug}`,
    `${slug}-list`
  ];

  return new Promise((resolve, reject) => {
    let res;
    let tryValIndex = 0;

    const attempt = async () => {
      try {
        res = await Bundle.sdk.find(toTry[tryValIndex]);
        return resolve(logResponse(res));
      } catch (err) {
        console.log(`toTry ${toTry[tryValIndex]} did not work ========`, slug, err);

        if (tryValIndex >= toTry.length) {
          return reject(new Error('Everything is broken forever.'));
        }

        tryValIndex++;
        attempt();
      }
    };

    attempt();
  });
};

Guidance and opinions appreciated!

megkadams
  • 717
  • 1
  • 10
  • 19

2 Answers2

3

Avoid the Promise constructor antipattern, and use a parameter instead of the outer-scope variable for the recursion count:

function identityCheck(slug) {
  const toTry = [
    slug,
    `package/${slug}`,
    `${slug}-list`
  ];
  async function attempt(tryIndex) {
    try {
      return await Bundle.sdk.find(toTry[tryIndex]);
    } catch (err) {
      console.log(`toTry ${toTry[tryIndex]} did not work ========`, slug, err);
      if (tryIndex >= toTry.length) {
        throw new Error('Everything is broken forever.'));
      } else {
        return attempt(tryIndex+1);
      }
    }
  }
  return attempt(0);
}
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • Oh, noted! Does that mean you think this version is better than the nested try/catch or just general helpful intel? – megkadams Oct 18 '17 at 02:08
  • Yes, it can be better: It's much easier to extend with more `toTry` values (just add them to the array), and it might be useful to further abstract out if other places in your code need similar retry logic. If neither of these reasons matter, it comes down to preference of simple vs abstract. – Bergi Oct 18 '17 at 02:16
  • Lovely, thank you for your snappy feedback! Might wait for some additional banter and then will accept answer. :) – megkadams Oct 18 '17 at 02:19
  • Should `console.error()` be used within `catch()` instead of `console.log()`? – guest271314 Oct 18 '17 at 02:56
  • @guest271314 No, why would it? – Bergi Oct 18 '17 at 03:19
  • @Bergi The use of `console.error()` instead of `console.log()` more than likely would have resulted in this Question [Why does .catch() not catch reject() within Promise constructor within loop at an async function unless Error is passed?](https://stackoverflow.com/questions/45430716/why-does-catch-not-catch-reject-within-promise-constructor-within-loop-at-a) not being asked in the first instance; have used `console.error()` within `catch()` or `.catch()` since – guest271314 Oct 18 '17 at 03:26
  • @guest271314 An appropriate description message together with the logged value would have solved that problem as well - and the OP does have those already. Assuming the program doesn't have results to output (as most node servers), [it doesn't matter whether to use stdout or stderr](https://stackoverflow.com/q/4919093/1048572) ([opinions](https://www.jstorimer.com/blogs/workingwithcode/7766119-when-to-use-stderr-instead-of-stdout) [differ](https://unix.stackexchange.com/q/331611/190272)). In any case, not all outputs in `catch` blocks are necessarily errors. – Bergi Oct 18 '17 at 03:52
0

Following on Bergi's answer, but trying to preserve the original structure to avoid "even more code":

const idCheck = async (slug, alsoTry = [`package/${slug}`, `${slug}-list`]) => {
  let res;
  try {
    res = await Bundle.sdk.find(slug);
  } catch (err) {
    if (!alsoTry.length) throw err;
    return idCheck(alsoTry.shift(), alsoTry);
  }
  return logResponse(res);
};

idCheck('fashion');

This takes advantage of default arguments being quite powerful.

Same complexity, but aesthetically closer to the nested try-blocks, a simpler pattern perhaps.

jib
  • 40,579
  • 17
  • 100
  • 158