From 76f141bc4939c4b79ab14009dfdc189213acf54d Mon Sep 17 00:00:00 2001 From: Roy Fu Date: Tue, 30 Jan 2024 14:59:39 -0500 Subject: [PATCH] NONE: Update our webhook route to send a status code back immediately (#207) * NONE: Update our webhook route to send a status code back immediately * update integration tests * lol finish writing comments about whats going on w jest * address comments --- src/infrastructure/event-bus.ts | 3 + src/infrastructure/index.ts | 1 + src/infrastructure/testing/event-bus-utils.ts | 7 ++ src/infrastructure/testing/index.ts | 1 + src/jobs/handle-figma-file-update-event.ts | 22 +++++ src/jobs/index.ts | 1 + src/web/routes/figma/figma-router.ts | 23 ++--- src/web/routes/figma/integration.test.ts | 94 ++++++++----------- 8 files changed, 85 insertions(+), 67 deletions(-) create mode 100644 src/infrastructure/event-bus.ts create mode 100644 src/infrastructure/testing/event-bus-utils.ts create mode 100644 src/infrastructure/testing/index.ts create mode 100644 src/jobs/handle-figma-file-update-event.ts create mode 100644 src/jobs/index.ts diff --git a/src/infrastructure/event-bus.ts b/src/infrastructure/event-bus.ts new file mode 100644 index 00000000..8eb8e2d0 --- /dev/null +++ b/src/infrastructure/event-bus.ts @@ -0,0 +1,3 @@ +import { EventEmitter } from 'events'; + +export const eventBus = new EventEmitter(); diff --git a/src/infrastructure/index.ts b/src/infrastructure/index.ts index 1ff09efd..9bff8237 100644 --- a/src/infrastructure/index.ts +++ b/src/infrastructure/index.ts @@ -1 +1,2 @@ export * from './logger'; +export * from './event-bus'; diff --git a/src/infrastructure/testing/event-bus-utils.ts b/src/infrastructure/testing/event-bus-utils.ts new file mode 100644 index 00000000..79cc88b7 --- /dev/null +++ b/src/infrastructure/testing/event-bus-utils.ts @@ -0,0 +1,7 @@ +import { eventBus } from '../event-bus'; + +export function waitForEvent(eventName: string): Promise { + return new Promise((resolve) => { + eventBus.once(eventName, resolve); + }); +} diff --git a/src/infrastructure/testing/index.ts b/src/infrastructure/testing/index.ts new file mode 100644 index 00000000..8ff86317 --- /dev/null +++ b/src/infrastructure/testing/index.ts @@ -0,0 +1 @@ +export * from './event-bus-utils'; diff --git a/src/jobs/handle-figma-file-update-event.ts b/src/jobs/handle-figma-file-update-event.ts new file mode 100644 index 00000000..42360d81 --- /dev/null +++ b/src/jobs/handle-figma-file-update-event.ts @@ -0,0 +1,22 @@ +import type { FigmaTeam } from '../domain/entities'; +import { eventBus, getLogger } from '../infrastructure'; +import { handleFigmaFileUpdateEventUseCase } from '../usecases'; +import type { FigmaFileUpdateWebhookEventRequestBody } from '../web/routes/figma'; + +export const handleFigmaFileUpdateEvent = async ( + requestBody: FigmaFileUpdateWebhookEventRequestBody, + figmaTeam: FigmaTeam, +): Promise => { + const { file_key: fileKey, webhook_id: webhookId } = requestBody; + try { + await handleFigmaFileUpdateEventUseCase.execute(figmaTeam, fileKey); + + getLogger().info({ webhookId }, 'Figma webhook callback succeeded.'); + eventBus.emit('figma.webhook.succeeded', { webhookId }); + } catch (e) { + getLogger().error(e, 'Figma webhook callback failed.', { + webhookId, + }); + eventBus.emit('figma.webhook.failed', { webhookId }); + } +}; diff --git a/src/jobs/index.ts b/src/jobs/index.ts new file mode 100644 index 00000000..30ab1bc6 --- /dev/null +++ b/src/jobs/index.ts @@ -0,0 +1 @@ +export * from './handle-figma-file-update-event'; diff --git a/src/web/routes/figma/figma-router.ts b/src/web/routes/figma/figma-router.ts index 7a2fd204..bba70e8d 100644 --- a/src/web/routes/figma/figma-router.ts +++ b/src/web/routes/figma/figma-router.ts @@ -13,10 +13,8 @@ import type { } from './types'; import { getLogger } from '../../../infrastructure'; -import { - handleFigmaAuthorizationResponseUseCase, - handleFigmaFileUpdateEventUseCase, -} from '../../../usecases'; +import { handleFigmaFileUpdateEvent } from '../../../jobs'; +import { handleFigmaAuthorizationResponseUseCase } from '../../../usecases'; import { requestSchemaValidationMiddleware } from '../../middleware'; import { figmaWebhookAuthMiddleware } from '../../middleware/figma/figma-webhook-auth-middleware'; @@ -33,18 +31,21 @@ figmaRouter.post( '/webhook', requestSchemaValidationMiddleware(FIGMA_WEBHOOK_EVENT_REQUEST_SCHEMA), figmaWebhookAuthMiddleware, - (req: FigmaWebhookEventRequest, res: FigmaWebhookEventResponse, next) => { + (req: FigmaWebhookEventRequest, res: FigmaWebhookEventResponse) => { const { figmaTeam } = res.locals; switch (req.body.event_type) { case 'FILE_UPDATE': { - const { file_key } = req.body; + // Making body its own variable so typescript is happy with the refinement + // we did on the discriminated union + const body = req.body; + void setImmediate( + () => void handleFigmaFileUpdateEvent(body, figmaTeam), + ); - handleFigmaFileUpdateEventUseCase - .execute(figmaTeam, file_key) - .then(() => res.sendStatus(HttpStatusCode.Ok)) - .catch(next); - return; + // Immediately send a 200 back to figma, before doing any of our own + // async processing + return res.sendStatus(HttpStatusCode.Ok); } default: return res.sendStatus(HttpStatusCode.Ok); diff --git a/src/web/routes/figma/integration.test.ts b/src/web/routes/figma/integration.test.ts index 419b43bb..9a03eec9 100644 --- a/src/web/routes/figma/integration.test.ts +++ b/src/web/routes/figma/integration.test.ts @@ -54,6 +54,7 @@ import { figmaOAuth2UserCredentialsRepository, figmaTeamRepository, } from '../../../infrastructure/repositories'; +import { waitForEvent } from '../../../infrastructure/testing'; import { mockFigmaGetFileEndpoint, mockFigmaGetTeamProjectsEndpoint, @@ -200,6 +201,8 @@ describe('/figma', () => { .post(FIGMA_WEBHOOK_EVENT_ENDPOINT) .send(webhookEventRequestBody) .expect(HttpStatusCode.Ok); + + await waitForEvent('figma.webhook.succeeded'); }); it('should ignore if no associated designs are found for the file key', async () => { @@ -224,6 +227,8 @@ describe('/figma', () => { .post(FIGMA_WEBHOOK_EVENT_ENDPOINT) .send(otherFilewebhookEventRequestBody) .expect(HttpStatusCode.Ok); + + await waitForEvent('figma.webhook.succeeded'); }); it('should ignore if Figma file is not found', async () => { @@ -259,6 +264,8 @@ describe('/figma', () => { .post(FIGMA_WEBHOOK_EVENT_ENDPOINT) .send(webhookEventRequestBody) .expect(HttpStatusCode.Ok); + + await waitForEvent('figma.webhook.succeeded'); }); it('should ingest designs for available Figma nodes and ignore deleted nodes', async () => { @@ -317,6 +324,8 @@ describe('/figma', () => { .post(FIGMA_WEBHOOK_EVENT_ENDPOINT) .send(webhookEventRequestBody) .expect(HttpStatusCode.Ok); + + await waitForEvent('figma.webhook.succeeded'); }); it('should return a 200 if fetching Figma team name fails with non-auth error', async () => { @@ -364,33 +373,15 @@ describe('/figma', () => { ), }); - await request(app) - .post(FIGMA_WEBHOOK_EVENT_ENDPOINT) - .send(webhookEventRequestBody) - .expect(HttpStatusCode.Ok); - }); - - it("should set the FigmaTeam status to 'ERROR' and return a 200 if fetching Figma team name fails with auth error", async () => { - mockFigmaGetTeamProjectsEndpoint({ - baseUrl: getConfig().figma.apiBaseUrl, - teamId: figmaTeam.teamId, - status: HttpStatusCode.Forbidden, - }); - await request(app) .post(FIGMA_WEBHOOK_EVENT_ENDPOINT) .send(webhookEventRequestBody) .expect(HttpStatusCode.Ok); - const updatedFigmaTeam = await figmaTeamRepository.findByWebhookId( - figmaTeam.webhookId, - ); - expect(updatedFigmaTeam?.authStatus).toStrictEqual( - FigmaTeamAuthStatus.ERROR, - ); + await waitForEvent('figma.webhook.succeeded'); }); - it('should return error if fetching Figma designs fails with unexpected error', async () => { + it('should send an error event if fetching Figma designs fails with unexpected error', async () => { const associatedFigmaDesigns = await associatedFigmaDesignRepository.findManyByFileKeyAndConnectInstallationId( fileKey, @@ -422,7 +413,31 @@ describe('/figma', () => { await request(app) .post(FIGMA_WEBHOOK_EVENT_ENDPOINT) .send(webhookEventRequestBody) - .expect(HttpStatusCode.InternalServerError); + .expect(HttpStatusCode.Ok); + + await waitForEvent('figma.webhook.failed'); + }); + + it("should set the FigmaTeam status to 'ERROR' and return a 200 if fetching Figma team name fails with auth error", async () => { + mockFigmaGetTeamProjectsEndpoint({ + baseUrl: getConfig().figma.apiBaseUrl, + teamId: figmaTeam.teamId, + status: HttpStatusCode.Forbidden, + }); + + await request(app) + .post(FIGMA_WEBHOOK_EVENT_ENDPOINT) + .send(webhookEventRequestBody) + .expect(HttpStatusCode.Ok); + + await waitForEvent('figma.webhook.succeeded'); + + const updatedFigmaTeam = await figmaTeamRepository.findByWebhookId( + figmaTeam.webhookId, + ); + expect(updatedFigmaTeam?.authStatus).toStrictEqual( + FigmaTeamAuthStatus.ERROR, + ); }); it("should set the FigmaTeam status to 'ERROR' and return a 200 if fetching Figma designs fails with auth error", async () => { @@ -459,6 +474,8 @@ describe('/figma', () => { .send(webhookEventRequestBody) .expect(HttpStatusCode.Ok); + await waitForEvent('figma.webhook.succeeded'); + const updatedFigmaTeam = await figmaTeamRepository.findByWebhookId( figmaTeam.webhookId, ); @@ -481,41 +498,6 @@ describe('/figma', () => { .send(webhookEventRequestBody) .expect(HttpStatusCode.BadRequest); }); - - it('should return a 500 if fetching designs from Figma fails', async () => { - const associatedFigmaDesigns = - await associatedFigmaDesignRepository.findManyByFileKeyAndConnectInstallationId( - fileKey, - connectInstallation.id, - ); - const nodeIds = associatedFigmaDesigns - .map(({ designId }) => designId.nodeId!) - .filter(isString); - - mockFigmaGetTeamProjectsEndpoint({ - baseUrl: getConfig().figma.apiBaseUrl, - teamId: figmaTeam.teamId, - response: generateGetTeamProjectsResponse({ - name: figmaTeam.teamName, - }), - }); - mockFigmaGetFileEndpoint({ - baseUrl: getConfig().figma.apiBaseUrl, - accessToken: adminFigmaOAuth2UserCredentials.accessToken, - fileKey: fileKey, - query: { - ids: nodeIds.join(','), - depth: '0', - node_last_modified: 'true', - }, - status: HttpStatusCode.InternalServerError, - }); - - await request(app) - .post(FIGMA_WEBHOOK_EVENT_ENDPOINT) - .send(webhookEventRequestBody) - .expect(HttpStatusCode.InternalServerError); - }); }); describe('PING event', () => {