small refactor to error message checking code in api.ts, add ability …

…to mock UrlFetchApp methods during tests, and test new error message checking logic

src-test/callFunctionInTest.ts

@@ -5,6 +5,8 @@
  * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
  */
 
+import ALL_FIXTURES from './mock-fixtures/all';
+
 export function callFunctionInTest(functionName: string, testName: string, ...params: unknown[]) {
   try {
     console.log(`calling ${functionName} in test "${testName}"`);
@@ -17,3 +19,18 @@ export function callFunctionInTest(functionName: string, testName: string, ...pa
     return JSON.stringify({ ...e, message: e.message, stack: e.stack });
   }
 }
+
+export function callFunctionInTestWithMockFixture(
+  functionName: string,
+  fixture: { name: string, params: unknown[] },
+  testName: string,
+  ...params: unknown[]
+) {
+  const fixtureInstance = (ALL_FIXTURES[fixture.name])(...params);
+  fixtureInstance.setUp();
+  try {
+    return callFunctionInTest(functionName, testName, ...params);
+  } finally {
+    fixtureInstance.tearDown();
+  }
+}

src-test/mock-fixtures/all.ts

@@ -0,0 +1,14 @@
+/**
+ * Matomo - free/libre analytics platform
+ *
+ * @link https://matomo.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+import urlFetchAppMock from './urlfetchapp-mock';
+
+const ALL_FIXTURES = {
+  urlfetchapp: urlFetchAppMock,
+};
+
+export default ALL_FIXTURES;

src-test/mock-fixtures/urlfetchapp-mock.ts

@@ -0,0 +1,32 @@
+/**
+ * Matomo - free/libre analytics platform
+ *
+ * @link https://matomo.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+export default function urlFetchAppMock(errorToThrow: string, throwAsString: boolean = false) {
+  const previousFetchAll = UrlFetchApp.fetchAll;
+  let isThrown = false;
+
+  return {
+    setUp() {
+      UrlFetchApp.fetchAll = function (...args) {
+        if (!isThrown) {
+          isThrown = true;
+
+          if (throwAsString) {
+            throw errorToThrow;
+          } else {
+            throw new Error(errorToThrow);
+          }
+        } else {
+          return previousFetchAll.call(this, ...args);
+        }
+      };
+    },
+    tearDown() {
+      UrlFetchApp.fetchAll = previousFetchAll;
+    },
+  };
+}

src/api.ts

@@ -110,6 +110,18 @@ export function extractBasicAuthFromUrl(url: string): { authHeaders: Record<stri
   return { authHeaders, urlWithoutAuth: url };
 }
 
+function isUrlFetchErrorQuotaLimitReachedError(errorMessage: unknown) {
+  return typeof errorMessage === 'string'
+    && errorMessage.toLowerCase().includes('service invoked too many times for one day: urlfetch')
+}
+
+function isUrlFetchErrorProbablyTemporary(errorMessage: unknown) {
+  return typeof errorMessage === 'string'
+    && !errorMessage.toLowerCase().includes('address unavailable')
+    && !errorMessage.toLowerCase().includes('dns error')
+    && !errorMessage.toLowerCase().includes('property fetchall on object urlfetchapp');
+}
+
 /**
  * Sends multiple API requests simultaneously to the target Matomo.
  *
@@ -205,18 +217,12 @@ export function fetchAll(requests: MatomoRequestParams[], options: ApiFetchOptio
       const errorMessage = e.message || e;
 
       // throw user friendly error messages if possible
-      if (errorMessage && errorMessage.toLowerCase().includes('service invoked too many times for one day: urlfetch')) {
+      if (isUrlFetchErrorQuotaLimitReachedError(errorMessage)) {
         throwUserError('The "urlfetch" daily quota for your account has been reached, further requests for today may not work. See https://developers.google.com/apps-script/guides/services/quotas for more information.');
       }
 
       // only rethrow for unknown errors, otherwise retry
-      if (!errorMessage
-        || (
-          !errorMessage.toLowerCase().includes('address unavailable')
-          && !errorMessage.toLowerCase().includes('dns error')
-          && !errorMessage.toLowerCase().includes('property fetchall on object urlfetchapp')
-        )
-      ) {
+      if (isUrlFetchErrorProbablyTemporary(errorMessage)) {
         throw e;
       }
     }

tests/appscript/api.spec.ts

@@ -277,6 +277,177 @@ describe('api', () => {
       // check that the request was only made once
       expect(requestCount).toEqual(1);
     });
+
+    it('should abort when UrlFetchApp throws an unknown error', async () => {
+      if (!process.env.USE_LOCALTUNNEL) {
+        console.log('*** SKIPPING TEST ***');
+        return;
+      }
+
+      server = makeApiFailureMockServer(3000);
+
+      await waitForMockServer();
+
+      // use the mock server's path that forces a non-random error
+      await Clasp.run('setCredentials', {
+        userToken: {
+          username: `${ tunnel.url }/`,
+          token: 'ignored',
+        },
+      });
+
+      const errorMessage = 'unknown error';
+
+      await expect(async () => {
+        // check it does not throw when an Error is used
+        await Clasp.runWithFixture(
+          { name: 'urlfetchapp', params: [ errorMessage ] },
+          'fetchAll',
+          [
+            {
+              method: 'SitesManager.getSitesIdWithAtLeastViewAccess',
+            },
+          ],
+          {
+            throwOnFailedRequest: true,
+          },
+        );
+      }).rejects.toHaveProperty('message', errorMessage);
+
+      // check it does not throw when a string is used
+      await expect(async () => {
+        await Clasp.runWithFixture(
+          { name: 'urlfetchapp', params: [ errorMessage, true ] },
+          'fetchAll',
+          [
+            {
+              method: 'SitesManager.getSitesIdWithAtLeastViewAccess',
+            },
+          ],
+          {
+            throwOnFailedRequest: true,
+          },
+        );
+      }).rejects.toEqual(errorMessage);
+    });
+
+    const TEMPORARY_ERRORS = [
+      {
+        name: 'address unavailable',
+        errorMessage: 'Address unavailable: test.whatever.com/index.php?',
+      },
+      {
+        name: 'dns error',
+        errorMessage: 'DNS error: https://matomo.whatever.com/index.php?',
+      },
+      {
+        name: 'urlfetchapp property',
+        errorMessage: 'Unexpected error while getting the method or property fetchAll on object UrlFetchApp.',
+      },
+    ];
+
+    TEMPORARY_ERRORS.forEach(({ name, errorMessage }) => {
+      it(`should retry when UrlFetchApp throws a "${name}" temporary error`, async () => {
+        if (!process.env.USE_LOCALTUNNEL) {
+          console.log('*** SKIPPING TEST ***');
+          return;
+        }
+
+        server = makeApiFailureMockServer(3000);
+
+        await waitForMockServer();
+
+        // use the mock server's path that forces a non-random error
+        await Clasp.run('setCredentials', {
+          userToken: {
+            username: `${ tunnel.url }/`,
+            token: 'ignored',
+          },
+        });
+
+        // check it does not throw when an Error is used
+        await Clasp.runWithFixture(
+          { name: 'urlfetchapp', params: [ errorMessage ] },
+          'fetchAll',
+          [
+            {
+              method: 'SitesManager.getSitesIdWithAtLeastViewAccess',
+            },
+          ],
+          {
+            throwOnFailedRequest: true,
+          },
+        );
+
+        // check it does not throw when a string is used
+        await Clasp.runWithFixture(
+          { name: 'urlfetchapp', params: [ errorMessage, true ] },
+          'fetchAll',
+          [
+            {
+              method: 'SitesManager.getSitesIdWithAtLeastViewAccess',
+            },
+          ],
+          {
+            throwOnFailedRequest: true,
+          },
+        );
+      });
+    });
+
+    it('should abort with a readable error message when a quota limit reached error is thrown', async () => {
+      if (!process.env.USE_LOCALTUNNEL) {
+        console.log('*** SKIPPING TEST ***');
+        return;
+      }
+
+      if (!process.env.USE_LOCALTUNNEL) {
+        console.log('*** SKIPPING TEST ***');
+        return;
+      }
+
+      server = makeApiFailureMockServer(3000);
+
+      await waitForMockServer();
+
+      // use the mock server's path that forces a non-random error
+      await Clasp.run('setCredentials', {
+        userToken: {
+          username: `${ tunnel.url }/`,
+          token: 'ignored',
+        },
+      });
+
+      await expect(async () => {
+        await Clasp.runWithFixture(
+          { name: 'urlfetchapp', params: [ 'Service invoked too many times for one day: urlfetch.' ] },
+          'fetchAll',
+          [
+            {
+              method: 'SitesManager.getSitesIdWithAtLeastViewAccess',
+            },
+          ],
+          {
+            throwOnFailedRequest: true,
+          },
+        );
+      }).rejects.toHaveProperty('message', 'Exception'); // actual data studio error message does not appear to be accessible
+
+      await expect(async () => {
+        await Clasp.runWithFixture(
+          { name: 'urlfetchapp', params: [ 'Service invoked too many times for one day: urlfetch.', true ] },
+          'fetchAll',
+          [
+            {
+              method: 'SitesManager.getSitesIdWithAtLeastViewAccess',
+            },
+          ],
+          {
+            throwOnFailedRequest: true,
+          },
+        );
+      }).rejects.toHaveProperty('message', 'Exception'); // actual data studio error message does not appear to be accessible
+    });
   });
 
   describe('isApiErrorNonRandom()', function () {

tests/utilities/clasp.ts

@@ -156,6 +156,20 @@ class Clasp {
       }
     });
   }
+
+  async runWithFixture(
+    fixture: { name: string; params: unknown[] },
+    functionName: string,
+    ...args: any[]
+  ) {
+    const testName = expect.getState().currentTestName;
+    return this.runExecutable([
+      'run',
+      '-p',
+      JSON.stringify([functionName, fixture, testName, ...args]),
+      'callFunctionInTestWithMockFixture',
+    ]);
+  }
 }
 
 export default new Clasp();