Skip to content

Commit

Permalink
fixes for a couple exceptions caught in the logs (#77)
Browse files Browse the repository at this point in the history
* do not propagate exceptions from UrlFetchAp.fetchAll for certain classes of known errors

* retry api request if no response is returned after fetching

* increase timeout after creating mock server again

* try waiting until requests to the tunnel work

* throw user friendly error message for "service invoked too many times" error from UrlFetchApp

* debug build

* run api.spec.ts first

* remove debugging code

* missing import and small refactor

* 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

* debug test

* introduce indirection to be able to mock UrlFetchApp methods

* remove debugging code and a redundant if statement
  • Loading branch information
diosmosis authored Oct 2, 2024
1 parent 01b7608 commit 1c9692f
Show file tree
Hide file tree
Showing 10 changed files with 345 additions and 10 deletions.
6 changes: 5 additions & 1 deletion jest.config.appscript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,16 @@ export default {
rootDir: 'tests/appscript',
preset: 'ts-jest/presets/js-with-ts-esm',
testEnvironment: 'node',
testTimeout: 180000,
testTimeout: 300000,
globalSetup: '../globalSetup.js',
globalTeardown: '../globalTeardown.js',
maxWorkers: 1, // since we run our tests within apps script, we can't run them in parallel
setupFiles: ['dotenv/config'],
transformIgnorePatterns: [
"node_modules/(?!mwp-localtunnel-client/.*)",
],
testMatch: [
'**/api.spec.ts',
'**/*.spec.ts',
],
};
25 changes: 23 additions & 2 deletions src-test/callFunctionInTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,36 @@
* @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}"`);

// there is no global object (like window) in apps script, so this is how we get a global function by name
const fn = eval(functionName);
const fn = (new Function(`return ${functionName};`))();
const result = fn(...params);
return JSON.stringify(result);
} catch (e) {
return JSON.stringify({ ...e, message: e.message, stack: e.stack });
return JSON.stringify({
...(typeof e === 'object' ? e : {}),
message: e.message || e,
stack: e.stack || 'no stack', // required so clasp.ts will recognize this object as an error
});
}
}

export function callFunctionInTestWithMockFixture(
functionName: string,
fixture: { name: string, params: unknown[] },
testName: string,
...params: unknown[]
) {
const fixtureInstance = (ALL_FIXTURES[fixture.name])(...fixture.params);
fixtureInstance.setUp();
try {
return callFunctionInTest(functionName, testName, ...params);
} finally {
fixtureInstance.tearDown();
}
}
14 changes: 14 additions & 0 deletions src-test/mock-fixtures/all.ts
Original file line number Diff line number Diff line change
@@ -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;
42 changes: 42 additions & 0 deletions src-test/mock-fixtures/urlfetchapp-mock.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/**
* 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) {
let isThrown = false;

const mockUrlFetchApp = new Proxy(UrlFetchApp, {
get(target, prop) {
if (prop === 'fetchAll') {
return function (...args) {
if (!isThrown) {
isThrown = true;

if (throwAsString) {
throw errorToThrow;
} else {
throw new Error(errorToThrow);
}
} else {
return target[prop].call(this, ...args);
}
};
}
return target[prop];
}
});

return {
setUp() {
const getServices = (new Function('return getServices;'))();
getServices().UrlFetchApp = mockUrlFetchApp;
},
tearDown() {
const getServices = (new Function('return getServices;'))();
getServices().UrlFetchApp = UrlFetchApp;
},
};
}
38 changes: 34 additions & 4 deletions src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@

import env from './env';
import { getScriptElapsedTime } from './connector';
import { throwUnexpectedError } from './error';
import { throwUnexpectedError, throwUserError } from './error';
import URLFetchRequest = GoogleAppsScript.URL_Fetch.URLFetchRequest;
import { debugLog, log, logError } from './log';
import { getServices } from './services';

const SCRIPT_RUNTIME_LIMIT = parseInt(env.SCRIPT_RUNTIME_LIMIT) || 0;
const API_REQUEST_RETRY_LIMIT_IN_SECS = parseInt(env.API_REQUEST_RETRY_LIMIT_IN_SECS) || 0;
Expand Down Expand Up @@ -110,6 +111,20 @@ 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.
*
Expand Down Expand Up @@ -198,7 +213,22 @@ export function fetchAll(requests: MatomoRequestParams[], options: ApiFetchOptio
wholeUrl: u, // used to link urlsToFetch with allUrlsMappedToIndex
}));

const responses = UrlFetchApp.fetchAll(urlsToFetch);
let responses = [];
try {
responses = getServices().UrlFetchApp.fetchAll(urlsToFetch);
} catch (e) {
const errorMessage = e.message || e;

// throw user friendly error messages if possible
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 (!isUrlFetchErrorProbablyTemporary(errorMessage)) {
throw e;
}
}

responses.forEach((r, i) => {
const urlFetched = (urlsToFetch[i] as any).wholeUrl;
Expand Down Expand Up @@ -242,8 +272,8 @@ export function fetchAll(requests: MatomoRequestParams[], options: ApiFetchOptio

// if there are still requests to try (because they failed), wait before trying again
const remainingRequestCount = Object.keys(allUrlsMappedToIndex).length;
const requestsFailed = !!remainingRequestCount;
if (requestsFailed) {
const haveRequestsFailed = remainingRequestCount > 0;
if (haveRequestsFailed) {
log(`${countOfFailedRequests} request(s) failed, retrying after ${currentWaitBeforeRetryTime / 1000} seconds.`);

Utilities.sleep(currentWaitBeforeRetryTime);
Expand Down
4 changes: 4 additions & 0 deletions src/data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,10 @@ function getReportData(request: GoogleAppsScript.Data_Studio.Request<ConnectorPa
runtimeLimitAbortMessage: pastScriptRuntimeLimitErrorMessage,
});

if (!partialResponseRaw) {
continue; // request failed in some unknown way
}

if ((partialResponseRaw as any).value === false) {
break; // nothing returned by request
}
Expand Down
1 change: 1 addition & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ export { extractBasicAuthFromUrl, fetchAll, isApiErrorNonRandom } from './api';
export * from './auth';
export * from './config';
export * from './data';
export * from './services';
14 changes: 14 additions & 0 deletions src/services.ts
Original file line number Diff line number Diff line change
@@ -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
*/

const SERVICES = {
UrlFetchApp,
};

export function getServices() {
return SERVICES;
}
Loading

0 comments on commit 1c9692f

Please sign in to comment.