3

Why does added catch still not work at all?

function maxRequest(url = ``, times = 3) {
  // closure
  function autoRetry (url, times) {
    console.log('times = ', times);
    times--;
    return new Promise((resolve, reject) => {
      fetch(url).then(value => {
        if(value.status === 200) {
          console.log(`✅ `, value);
          resolve(value);
        } else {
          throw new Error(`❌  http code error: ${value.status }`);
        }
      }).catch((err) => {
        console.log(`❌  Error`, err);
        if (times < 1) {
          reject('  over max request times!');
        } else {
          autoRetry(url, times);
        }
      });
    });
  }
  return autoRetry(url, times);
}

maxRequest(`https://cdn.xgqfrms.xyz/json/badges.js`)
  .then(res => res.json())
  .then(json => {
      console.log('json =', json);
      return json;
  }, err => {
      console.log('error =', err);
      throw new Error(err);
  })
  .catch(err => console.log(`err =`, err))
  .finally(() => {
      console.log('whatever close loading...');
  });

enter image description here

enter image description here

VLAZ
  • 26,331
  • 9
  • 49
  • 67
xgqfrms
  • 10,077
  • 1
  • 69
  • 68

1 Answers1

2

Your use of the explicit Promise construction antipattern masks the fact that you don't return from your catch block. In a retry situation, you create a second-try Promise by calling autoRetry, but you don't do anything with it, so the catch block in your call to maxRequest doesn't protect you and you wind up with an uncaught rejected Promise.

function maxRequest(url = ``, times = 3) {
  // closure
  function autoRetry (url, times) {
    console.log('times = ', times);
    times--;
    // No more `new Promise`, just a chain.
    return fetch(url).then(value => {
      if(value.status === 200) {
        console.log(`✅ `, value);
        return value;
      } else {
        throw new Error(`❌  http code error: ${value.status }`);
      }
    }).catch((err) => {
      console.log(`❌  Error`, err);
      if (times < 1) {
        // Converted to rejection. You could still throw a
        // string, but throwing an Error is more idiomatic.
        throw new Error('  over max request times!');
      } else {
        // Make sure to return your Promise here. Don't worry
        // about returning a Promise; it's automatically wrapped
        // like Promise.resolve() does, so this chained-fetch()
        // Promise eventually gets the same result as the returned
        // recursive Promise.
        return autoRetry(url, times);
      }
    });
  }
  return autoRetry(url, times);
}

maxRequest(`https://cdn.xgqfrms.xyz/json/badges.js`)
  .then(res => res.json())
  .then(json => {
      console.log('json =', json);
      return json;
  }, err => {
      console.log('error =', err);
      throw new Error(err);
  })
  .catch(err => console.log(`err =`, err))
  .finally(() => {
      console.log('whatever close loading...');
  });

For readability's sake, you might also want to change the variable names, so times isn't used for both the outer and inner scope. You might also rephrase this as a much shorter async function that loops.

enter image description here

xgqfrms
  • 10,077
  • 1
  • 69
  • 68
Jeff Bowman
  • 90,959
  • 16
  • 217
  • 251