Skip to content

Commit

Permalink
2515 handle long running requests (#2559)
Browse files Browse the repository at this point in the history
* logging load test

* fix: timeout test

* chore(iam): load test

* fix: load testing

* fix(iam): catch error from array

* chore(platforms, iam): cleanup and test timeout logic

* fix(iam): default to 2 tasks in production
  • Loading branch information
digitalmnt committed Jun 6, 2024
1 parent b206f82 commit 1c83f48
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 5 deletions.
7 changes: 6 additions & 1 deletion iam/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -364,8 +364,13 @@ async function verifyTypes(types: string[], payload: RequestPayload): Promise<Ve
// TODO to be changed to just verifyResult.errors when all providers are updated
const resultErrors = verifyResult.errors;
error = resultErrors?.join(", ")?.substring(0, 1000) || "Unable to verify provider";
if (error.includes(`Request timeout while verifying ${type}.`)) {
console.log(`Request timeout while verifying ${type}`);

Check warning on line 368 in iam/src/index.ts

View workflow job for this annotation

GitHub Actions / check-provider-bitmap

Unexpected console statement

Check warning on line 368 in iam/src/index.ts

View workflow job for this annotation

GitHub Actions / build

Unexpected console statement

Check warning on line 368 in iam/src/index.ts

View workflow job for this annotation

GitHub Actions / build

Unexpected console statement

Check warning on line 368 in iam/src/index.ts

View workflow job for this annotation

GitHub Actions / build-and-test

Unexpected console statement

Check warning on line 368 in iam/src/index.ts

View workflow job for this annotation

GitHub Actions / build-and-test

Unexpected console statement
// If a request times out exit loop and return results so additional requests are not made
break;
}
}
} catch {
} catch (e) {
error = "Unable to verify provider";
code = 400;
}
Expand Down
2 changes: 1 addition & 1 deletion infra/aws/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ const service = new aws.ecs.Service(
`passport-iam`,
{
cluster: cluster.arn,
desiredCount: 1,
desiredCount: 2,
enableEcsManagedTags: true,
enableExecuteCommand: false,
launchType: "FARGATE",
Expand Down
34 changes: 32 additions & 2 deletions platforms/src/utils/__tests__/providers.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,41 @@
/* eslint-disable */
import { RequestPayload, ProviderContext } from "@gitcoin/passport-types";
import { RequestPayload, ProviderContext, VerifiedPayload } from "@gitcoin/passport-types";
import { ProviderExternalVerificationError } from "../../types";
import { Providers } from "../providers";
import { Providers, withTimeout } from "../providers";
import { SimpleProvider, verifySimpleProvider } from "../simpleProvider";

jest.spyOn(console, "error").mockImplementation(() => {});


jest.useFakeTimers(); // Use Jest's timer mocks

describe("withTimeout", () => {
beforeAll(() => {
jest.spyOn(global, 'clearTimeout');
});
it("should resolve with the correct value if the promise resolves before the timeout", async () => {
const expectedValue = { valid: true };
const fastPromise = new Promise((resolve) => setTimeout(() => resolve(expectedValue), 1000)) as Promise<VerifiedPayload>;

const resultPromise = withTimeout(3000, fastPromise, "testType");
jest.advanceTimersByTime(1000); // Fast-forward until all timers are executed

await expect(resultPromise).resolves.toEqual(expectedValue);
expect(clearTimeout).toHaveBeenCalledTimes(1);
});

it("should reject with a timeout error if the promise does not resolve in time", async () => {
const slowPromise = new Promise((resolve) => setTimeout(() => resolve({ valid: true }), 5000)) as Promise<VerifiedPayload>;

const resultPromise = withTimeout(3000, slowPromise, "testType");
jest.advanceTimersByTime(3001); // Fast-forward until the timeout should occur

await expect(resultPromise).rejects.toThrow(ProviderExternalVerificationError);
await expect(resultPromise).rejects.toThrow("Request timeout while verifying testType. It took over 3000 ms to complete.");
expect(clearTimeout).toHaveBeenCalledTimes(1);
});
});

describe("Providers", function () {
beforeEach(() => {
jest.clearAllMocks();
Expand Down
16 changes: 15 additions & 1 deletion platforms/src/utils/providers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,20 @@ function reportUnhandledError(type: string, address: string, e: unknown) {
}
}

export const withTimeout = async (millis: number, promise: Promise<VerifiedPayload>, type: string): Promise<VerifiedPayload> => {
let timeoutPid : NodeJS.Timeout | null = null;
const timeout = new Promise<VerifiedPayload>((_resolve, reject) =>
timeoutPid = setTimeout(
() => reject( new ProviderExternalVerificationError(`Request timeout while verifying ${type}. It took over ${millis} ms to complete.`)),
millis));
const result = await Promise.race([
promise,
timeout
])
clearTimeout(timeoutPid);
return result
};

// Collate all Providers to abstract verify logic
export class Providers {
// collect providers against instance
Expand All @@ -46,7 +60,7 @@ export class Providers {

if (provider) {
try {
const result = await provider.verify(payload, context);
const result = await withTimeout(30000, provider.verify(payload, context), type);
if (!result.valid && !result.errors) {
reportUnhandledError(type, payload.address, new NoFailureReasonError());
}
Expand Down

0 comments on commit 1c83f48

Please sign in to comment.