From ad4cf264aaef018f20bafda44bb5e7c604e2333e Mon Sep 17 00:00:00 2001 From: Antariksh Mahajan Date: Tue, 8 Jun 2021 10:55:23 +0800 Subject: [PATCH] feat: enable retries for webhooks (#2093) * build(webhook-retries): create SQS queue in dev mode (1) (#1938) * feat(webhook-retries): add isRetryEnabled to model (2) (#1939) * feat(webhook-retries): produce and consume retries (3) (#1940) * test(webhook-retries): add unit tests for retrieveWebhookInfoById (5) (#1942) * test(webhook-retries): add unit tests for webhook service (6) (#1943) * test(webhook-retries): add unit tests for message, producer, consumer (7) (#1988) * test: add tests for WebhookQueueMessage * test: add tests for webhook consumer * test: add tests for WebhookProducer * test: add tests for important utils * feat: increase jitter with increasing base interval * fix: calculate next attempt from time of initial attempt * feat(webhook-retries): enable toggling retries on frontend (4) (#1941) * test: make Date.now consistent in producer tests --- Dockerfile.development | 9 +- docker-compose.yml | 9 +- .../init-localstack.sh | 6 - init-localstack.sh | 29 ++ package-lock.json | 31 ++ package.json | 5 +- src/app/config/feature-manager/types.ts | 1 + .../webhook-verified-content.config.ts | 6 + src/app/constants/timezone.ts | 1 + .../__tests__/form.server.model.spec.ts | 1 + .../__tests__/submission.server.model.spec.ts | 227 +++++++++++++- src/app/models/form.server.model.ts | 4 + src/app/models/submission.server.model.ts | 27 +- .../encrypt-submission.controller.ts | 10 +- .../modules/submission/submission.errors.ts | 2 +- .../__tests__/webhook.consumer.spec.ts | 285 ++++++++++++++++++ .../webhook/__tests__/webhook.message.spec.ts | 193 ++++++++++++ .../__tests__/webhook.producer.spec.ts | 145 +++++++++ .../webhook/__tests__/webhook.service.spec.ts | 227 +++++++++----- .../webhook/__tests__/webhook.utils.spec.ts | 71 +++++ src/app/modules/webhook/webhook.constants.ts | 52 ++++ src/app/modules/webhook/webhook.consumer.ts | 258 ++++++++++++++++ src/app/modules/webhook/webhook.errors.ts | 58 ++++ src/app/modules/webhook/webhook.factory.ts | 22 +- src/app/modules/webhook/webhook.message.ts | 181 +++++++++++ src/app/modules/webhook/webhook.producer.ts | 79 +++++ src/app/modules/webhook/webhook.service.ts | 63 +++- src/app/modules/webhook/webhook.types.ts | 53 +++- src/app/modules/webhook/webhook.utils.ts | 66 ++++ .../admin-forms.settings.routes.spec.ts | 8 +- .../forms/admin-forms.settings.routes.ts | 5 +- src/app/utils/random-uniform.ts | 12 + .../modules/forms/admin/css/settings-form.css | 4 + .../settings-form.client.view.html | 62 +++- .../settings-form.client.directive.js | 13 + src/public/translations/en-SG/main.json | 3 +- src/types/api/form.ts | 4 +- src/types/form.ts | 1 + src/types/submission.ts | 23 ++ 39 files changed, 2096 insertions(+), 160 deletions(-) delete mode 100644 docker-entrypoint-initaws.d/init-localstack.sh create mode 100644 init-localstack.sh create mode 100644 src/app/constants/timezone.ts create mode 100644 src/app/modules/webhook/__tests__/webhook.consumer.spec.ts create mode 100644 src/app/modules/webhook/__tests__/webhook.message.spec.ts create mode 100644 src/app/modules/webhook/__tests__/webhook.producer.spec.ts create mode 100644 src/app/modules/webhook/__tests__/webhook.utils.spec.ts create mode 100644 src/app/modules/webhook/webhook.constants.ts create mode 100644 src/app/modules/webhook/webhook.consumer.ts create mode 100644 src/app/modules/webhook/webhook.message.ts create mode 100644 src/app/modules/webhook/webhook.producer.ts create mode 100644 src/app/utils/random-uniform.ts diff --git a/Dockerfile.development b/Dockerfile.development index 3dbdf6326d..452e751bc1 100644 --- a/Dockerfile.development +++ b/Dockerfile.development @@ -26,9 +26,12 @@ RUN apk update && apk upgrade && \ ttf-freefont \ tini \ # Localstack - these are necessary in order to initialise local S3 buckets + # jq is a package for easily parsing Localstack health endpoint's JSON output + jq \ py-pip && \ npm install --quiet node-gyp -g && \ - pip install awscli-local + # [ver1] ensures that the underlying AWS CLI version is also installed + pip install awscli-local[ver1] # Chinese fonts RUN echo @edge http://nl.alpinelinux.org/alpine/edge/testing >> /etc/apk/repositories && apk add wqy-zenhei@edge @@ -41,5 +44,5 @@ EXPOSE 5000 # tini is the init process that will adopt orphaned zombie processes # e.g. chromium when launched to create a new PDF ENTRYPOINT [ "tini", "--" ] -# Create local S3 buckets before building the app -CMD npm run docker-dev +# Create local AWS resources before building the app +CMD sh init-localstack.sh && npm run docker-dev \ No newline at end of file diff --git a/docker-compose.yml b/docker-compose.yml index 0ec2065816..63c317559d 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -38,6 +38,7 @@ services: - MYINFO_CERT_PATH=./node_modules/@opengovsg/mockpass/static/certs/spcp.crt - MYINFO_CLIENT_ID=mockClientId - MYINFO_CLIENT_SECRET=mockClientSecret + - WEBHOOK_SQS_URL=http://localhost:4566/000000000000/local-webhooks-sqs-main - GA_TRACKING_ID - SENTRY_CONFIG_URL - TWILIO_ACCOUNT_SID @@ -105,17 +106,11 @@ services: depends_on: - formsg environment: - - SERVICES=s3 + - SERVICES=s3,sqs - DATA_DIR=/tmp/localstack/data - - ATTACHMENT_S3_BUCKET=local-attachment-bucket - - IMAGE_S3_BUCKET=local-image-bucket - - LOGO_S3_BUCKET=local-logo-bucket volumes: - './.localstack:/tmp/localstack' - '/var/run/docker.sock:/var/run/docker.sock' - # This is where we add scripts to initialise AWS resources. - # Docs: https://github.com/localstack/localstack#initializing-a-fresh-instance - - './docker-entrypoint-initaws.d:/docker-entrypoint-initaws.d' network_mode: 'service:formsg' # reuse formsg service's network stack so that it can resolve localhost:4566 to localstack:4566 maildev: diff --git a/docker-entrypoint-initaws.d/init-localstack.sh b/docker-entrypoint-initaws.d/init-localstack.sh deleted file mode 100644 index db04846f86..0000000000 --- a/docker-entrypoint-initaws.d/init-localstack.sh +++ /dev/null @@ -1,6 +0,0 @@ -#!/bin/bash -set -x -awslocal s3 mb s3://$IMAGE_S3_BUCKET -awslocal s3 mb s3://$LOGO_S3_BUCKET -awslocal s3 mb s3://$ATTACHMENT_S3_BUCKET -set +x \ No newline at end of file diff --git a/init-localstack.sh b/init-localstack.sh new file mode 100644 index 0000000000..bfb92305bc --- /dev/null +++ b/init-localstack.sh @@ -0,0 +1,29 @@ +#!/bin/bash +# Wait for all Localstack services to be ready +while [[ "$(curl -s -f http://localhost:4566/health | jq '[.services[] == "running"] | all')" != "true" ]]; do + sleep 5 +done + +# Create SQS queue for webhooks +# First create dead-letter queue and get its ARN so it can be specified as the DLQ +# for the main queue. Note that the DLQ name is not an environment variable +# in the application, as this is configured from the AWS console in production. +DLQ_NAME=local-webhooks-sqs-deadLetter +DLQ_URL=$(awslocal sqs create-queue --queue-name $DLQ_NAME | jq --raw-output '.QueueUrl') +DLQ_ARN=$(awslocal sqs get-queue-attributes --queue-url $DLQ_URL --attribute-names QueueArn | jq --raw-output '.Attributes.QueueArn') + +# Show output for all main resources created +set -x + +# For main queue, extract queue name, which is the part of the queue URL after the final "/" +awslocal sqs create-queue --queue-name ${WEBHOOK_SQS_URL##*/} --attributes '{ + "ReceiveMessageWaitTimeSeconds": "20", + "RedrivePolicy": "{\"deadLetterTargetArn\":\"'"$DLQ_ARN"'\",\"maxReceiveCount\":1}" +}' + +# Create S3 buckets +awslocal s3 mb s3://$IMAGE_S3_BUCKET +awslocal s3 mb s3://$LOGO_S3_BUCKET +awslocal s3 mb s3://$ATTACHMENT_S3_BUCKET + +set +x diff --git a/package-lock.json b/package-lock.json index 1457f79e24..e3b191dee6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -22502,6 +22502,32 @@ "integrity": "sha1-BOaSb2YolTVPPdAVIDYzuFcpfiw=", "dev": true }, + "sqs-consumer": { + "version": "5.5.0", + "resolved": "https://registry.npmjs.org/sqs-consumer/-/sqs-consumer-5.5.0.tgz", + "integrity": "sha512-vzKzOZlZtZarOWbg/nbEoMyNu64XnQ4QB3e74nMBNaIuM/RhelUGNGrvrB83IW6a7/DxKDulM46h2TeQP3/1nA==", + "requires": { + "debug": "^4.1.1" + }, + "dependencies": { + "debug": { + "version": "4.3.1", + "resolved": "https://registry.npmjs.org/debug/-/debug-4.3.1.tgz", + "integrity": "sha512-doEwdvm4PCeK4K3RQN2ZC2BYUBaxwLARCqZmMjtF8a51J2Rb0xpVloFRnCODwqjpwnAoao4pelN8l3RJdv3gRQ==", + "requires": { + "ms": "2.1.2" + } + } + } + }, + "sqs-producer": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/sqs-producer/-/sqs-producer-2.1.0.tgz", + "integrity": "sha512-UOlBaVIyCPJ/thAUSFjbB5MTgu3HG9FzFhjN5aiu/Y/QEeqoT4Twc+o7Yappwiz6easqJHz7+kqBq7Oy1GwQ8w==", + "requires": { + "aws-sdk": "^2.673.0" + } + }, "sshpk": { "version": "1.16.1", "resolved": "https://registry.npmjs.org/sshpk/-/sshpk-1.16.1.tgz", @@ -27097,6 +27123,11 @@ "integrity": "sha512-Ux4ygGWsu2c7isFWe8Yu1YluJmqVhxqK2cLXNQA5AcC3QfbGNpM7fu0Y8b/z16pXLnFxZYvWhd3fhBY9DLmC6Q==", "dev": true }, + "zod": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/zod/-/zod-3.0.0.tgz", + "integrity": "sha512-4DBG6siN02ooPB1yvEEqoe32maHzKEdGgtQ2HEz6FnFtgTjwZtzJ3ScuiDgtssWfDyLnQ3MvtSj6ff5ANL4STw==" + }, "zwitch": { "version": "1.0.5", "resolved": "https://registry.npmjs.org/zwitch/-/zwitch-1.0.5.tgz", diff --git a/package.json b/package.json index c2f4b1075e..cd79dcc801 100644 --- a/package.json +++ b/package.json @@ -142,6 +142,8 @@ "slick-carousel": "1.8.1", "sortablejs": "~1.13.0", "spark-md5": "^3.0.1", + "sqs-consumer": "^5.5.0", + "sqs-producer": "^2.1.0", "text-encoding": "^0.7.0", "toastr": "^2.1.4", "triple-beam": "^1.3.0", @@ -154,7 +156,8 @@ "web-streams-polyfill": "^3.0.3", "whatwg-fetch": "^3.6.2", "winston": "^3.3.3", - "winston-cloudwatch": "^2.5.2" + "winston-cloudwatch": "^2.5.2", + "zod": "^3.0.0" }, "devDependencies": { "@babel/core": "^7.14.3", diff --git a/src/app/config/feature-manager/types.ts b/src/app/config/feature-manager/types.ts index 075095c27e..6e2b3ac337 100644 --- a/src/app/config/feature-manager/types.ts +++ b/src/app/config/feature-manager/types.ts @@ -79,6 +79,7 @@ export interface IVerifiedFields { export interface IWebhookVerifiedContent { signingSecretKey: string + webhookQueueUrl: string } export interface IIntranet { diff --git a/src/app/config/feature-manager/webhook-verified-content.config.ts b/src/app/config/feature-manager/webhook-verified-content.config.ts index 2a5fd6520e..f5511ef7f4 100644 --- a/src/app/config/feature-manager/webhook-verified-content.config.ts +++ b/src/app/config/feature-manager/webhook-verified-content.config.ts @@ -10,6 +10,12 @@ const webhookVerifiedContentFeature: RegisterableFeature ({ + promises: { + resolve: jest.fn(), + }, +})) +const MockDns = mocked(dns, true) + const Submission = getSubmissionModel(mongoose) const EncryptedSubmission = getEncryptSubmissionModel(mongoose) const EmailSubmission = getEmailSubmissionModel(mongoose) // TODO: Add more tests for the rest of the submission schema. describe('Submission Model', () => { - beforeAll(async () => await dbHandler.connect()) + beforeAll(async () => { + await dbHandler.connect() + MockDns.resolve.mockResolvedValue(['1.1.1.1']) + }) afterEach(async () => await dbHandler.clearDatabase()) afterAll(async () => await dbHandler.closeDatabase()) const MOCK_ENCRYPTED_CONTENT = 'abcdefg encryptedContent' + const MOCK_VERIFIED_CONTENT = 'hijklmnop verifiedContent' + const MOCK_WEBHOOK_URL = 'https://test.web.site' describe('Statics', () => { + describe('retrieveWebhookInfoById', () => { + it('should return the populated submission when the submission and webhook URL exist', async () => { + const { form } = await dbHandler.insertEncryptForm({ + formOptions: { + webhook: { + url: MOCK_WEBHOOK_URL, + isRetryEnabled: true, + }, + }, + }) + const submission = await EncryptedSubmission.create({ + form: form._id, + encryptedContent: MOCK_ENCRYPTED_CONTENT, + version: 0, + }) + + const result = await EncryptedSubmission.retrieveWebhookInfoById( + String(submission._id), + ) + + expect(result).toEqual({ + webhookUrl: MOCK_WEBHOOK_URL, + isRetryEnabled: true, + webhookView: { + data: { + formId: String(form._id), + submissionId: String(submission._id), + encryptedContent: MOCK_ENCRYPTED_CONTENT, + verifiedContent: undefined, + version: 0, + created: submission.created, + }, + }, + }) + }) + + it('should return null when the submission ID does not exist', async () => { + // Create submission + const { form } = await dbHandler.insertEncryptForm({ + formOptions: { + webhook: { + url: MOCK_WEBHOOK_URL, + isRetryEnabled: true, + }, + }, + }) + await EncryptedSubmission.create({ + form: form._id, + encryptedContent: MOCK_ENCRYPTED_CONTENT, + version: 0, + }) + + // Attempt to find submission with a different ID + const result = await EncryptedSubmission.retrieveWebhookInfoById( + String(new ObjectId().toHexString()), + ) + + expect(result).toBeNull() + }) + + it('should return empty string for the webhook URL when the form does not have a webhook URL', async () => { + const { form } = await dbHandler.insertEncryptForm() + const submission = await EncryptedSubmission.create({ + form: form._id, + encryptedContent: MOCK_ENCRYPTED_CONTENT, + version: 0, + }) + + const result = await EncryptedSubmission.retrieveWebhookInfoById( + String(submission._id), + ) + + expect(result).toEqual({ + webhookUrl: '', + isRetryEnabled: false, + webhookView: { + data: { + formId: String(form._id), + submissionId: String(submission._id), + encryptedContent: MOCK_ENCRYPTED_CONTENT, + verifiedContent: undefined, + version: 0, + created: submission.created, + }, + }, + }) + }) + + it('should return false for isRetryEnabled when the form does not have retries enabled', async () => { + const { form } = await dbHandler.insertEncryptForm({ + formOptions: { + webhook: { + url: MOCK_WEBHOOK_URL, + isRetryEnabled: false, + }, + }, + }) + const submission = await EncryptedSubmission.create({ + form: form._id, + encryptedContent: MOCK_ENCRYPTED_CONTENT, + version: 0, + }) + + const result = await EncryptedSubmission.retrieveWebhookInfoById( + String(submission._id), + ) + + expect(result).toEqual({ + webhookUrl: MOCK_WEBHOOK_URL, + isRetryEnabled: false, + webhookView: { + data: { + formId: String(form._id), + submissionId: String(submission._id), + encryptedContent: MOCK_ENCRYPTED_CONTENT, + verifiedContent: undefined, + version: 0, + created: submission.created, + }, + }, + }) + }) + }) + describe('findFormsWithSubsAbove', () => { it('should return ids and counts of forms with more than given minimum submissions', async () => { // Arrange @@ -105,9 +243,9 @@ describe('Submission Model', () => { describe('Methods', () => { describe('getWebhookView', () => { - it('should return non-null view with encryptedSubmission type (without verified content)', async () => { + it('should return non-null view with encryptedSubmission type when submission has no verified content', async () => { // Arrange - const formId = new ObjectID() + const formId = new ObjectId() const submission = await EncryptedSubmission.create({ submissionType: SubmissionType.Encrypt, @@ -135,9 +273,82 @@ describe('Submission Model', () => { }) }) + it('should return non-null view with encryptedSubmission type when submission has verified content', async () => { + // Arrange + const formId = new ObjectId() + + const submission = await EncryptedSubmission.create({ + submissionType: SubmissionType.Encrypt, + form: formId, + encryptedContent: MOCK_ENCRYPTED_CONTENT, + verifiedContent: MOCK_VERIFIED_CONTENT, + version: 1, + authType: AuthType.NIL, + myInfoFields: [], + webhookResponses: [], + }) + + // Act + const actualWebhookView = submission.getWebhookView() + + // Assert + expect(actualWebhookView).toEqual({ + data: { + formId: expect.any(String), + submissionId: expect.any(String), + created: expect.any(Date), + encryptedContent: MOCK_ENCRYPTED_CONTENT, + verifiedContent: MOCK_VERIFIED_CONTENT, + version: 1, + }, + }) + }) + + it('should return non-null view with encryptedSubmission type when submission is populated with webhook info', async () => { + // Arrange + const { form } = await dbHandler.insertEncryptForm({ + formOptions: { + webhook: { + url: MOCK_WEBHOOK_URL, + isRetryEnabled: false, + }, + }, + }) + + const submission = await EncryptedSubmission.create({ + submissionType: SubmissionType.Encrypt, + form: form._id, + encryptedContent: MOCK_ENCRYPTED_CONTENT, + verifiedContent: MOCK_VERIFIED_CONTENT, + version: 1, + authType: AuthType.NIL, + myInfoFields: [], + webhookResponses: [], + }) + + const populatedSubmission = await EncryptedSubmission.findById( + submission._id, + ).populate('form', 'webhook') + + // Act + const actualWebhookView = populatedSubmission!.getWebhookView() + + // Assert + expect(actualWebhookView).toEqual({ + data: { + formId: expect.any(String), + submissionId: expect.any(String), + created: expect.any(Date), + encryptedContent: MOCK_ENCRYPTED_CONTENT, + verifiedContent: MOCK_VERIFIED_CONTENT, + version: 1, + }, + }) + }) + it('should return null view with non-encryptSubmission type', async () => { // Arrange - const formId = new ObjectID() + const formId = new ObjectId() const submission = await EmailSubmission.create({ submissionType: SubmissionType.Email, form: formId, @@ -162,7 +373,7 @@ describe('Submission Model', () => { describe('addWebhookResponse', () => { it('should return updated submission with webhook response when submission ID is valid', async () => { // Arrange - const formId = new ObjectID() + const formId = new ObjectId() const submission = await EncryptedSubmission.create({ submissionType: SubmissionType.Encrypt, form: formId, @@ -205,7 +416,7 @@ describe('Submission Model', () => { it('should return null when submission id is invalid', async () => { // Arrange - const formId = new ObjectID() + const formId = new ObjectId() const submission = await EncryptedSubmission.create({ submissionType: SubmissionType.Encrypt, form: formId, @@ -231,7 +442,7 @@ describe('Submission Model', () => { }, } as IWebhookResponse - const invalidSubmissionId = new ObjectID().toHexString() + const invalidSubmissionId = new ObjectId().toHexString() // Act const actualSubmission = await EncryptedSubmission.addWebhookResponse( diff --git a/src/app/models/form.server.model.ts b/src/app/models/form.server.model.ts index 1f9aff820a..516aa8b4f9 100644 --- a/src/app/models/form.server.model.ts +++ b/src/app/models/form.server.model.ts @@ -356,6 +356,10 @@ const compileFormModel = (db: Mongoose): IFormModel => { 'Webhook must be a valid URL over HTTPS and point to a public IP.', }, }, + isRetryEnabled: { + type: Boolean, + default: false, + }, }, msgSrvcName: { diff --git a/src/app/models/submission.server.model.ts b/src/app/models/submission.server.model.ts index 9b3c92e9d8..2e37daae76 100644 --- a/src/app/models/submission.server.model.ts +++ b/src/app/models/submission.server.model.ts @@ -9,6 +9,7 @@ import { IEmailSubmissionSchema, IEncryptedSubmissionSchema, IEncryptSubmissionModel, + IPopulatedWebhookSubmission, ISubmissionModel, ISubmissionSchema, IWebhookResponse, @@ -18,6 +19,7 @@ import { SubmissionData, SubmissionMetadata, SubmissionType, + SubmissionWebhookInfo, WebhookData, WebhookView, } from '../../types' @@ -179,9 +181,14 @@ const EncryptSubmissionSchema = new Schema< * Returns an object which represents the encrypted submission * which will be posted to the webhook URL. */ -EncryptSubmissionSchema.methods.getWebhookView = function (): WebhookView { +EncryptSubmissionSchema.methods.getWebhookView = function ( + this: IEncryptedSubmissionSchema | IPopulatedWebhookSubmission, +): WebhookView { + const formId = this.populated('form') + ? String(this.form._id) + : String(this.form) const webhookData: WebhookData = { - formId: String(this.form), + formId, submissionId: String(this._id), encryptedContent: this.encryptedContent, verifiedContent: this.verifiedContent, @@ -205,6 +212,22 @@ EncryptSubmissionSchema.statics.addWebhookResponse = function ( ).exec() } +EncryptSubmissionSchema.statics.retrieveWebhookInfoById = function ( + this: IEncryptSubmissionModel, + submissionId: string, +): Promise { + return this.findById(submissionId) + .populate('form', 'webhook') + .then((populatedSubmission: IPopulatedWebhookSubmission | null) => { + if (!populatedSubmission) return null + return { + webhookUrl: populatedSubmission.form.webhook?.url ?? '', + isRetryEnabled: !!populatedSubmission.form.webhook?.isRetryEnabled, + webhookView: populatedSubmission.getWebhookView(), + } + }) +} + EncryptSubmissionSchema.statics.findSingleMetadata = function ( formId: string, submissionId: string, diff --git a/src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts b/src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts index 06fbac9f9f..1691f417c7 100644 --- a/src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts +++ b/src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts @@ -360,12 +360,14 @@ const submitEncryptModeForm: ControllerHandler< }) // Fire webhooks if available - // Note that we push data to webhook endpoints on a best effort basis - // As such, we should not await on these post requests + // To avoid being coupled to latency of receiving system, + // do not await on webhook const webhookUrl = form.webhook?.url if (webhookUrl) { - void WebhookFactory.sendWebhook(submission, webhookUrl).andThen( - (response) => WebhookFactory.saveWebhookRecord(submission._id, response), + void WebhookFactory.sendInitialWebhook( + submission, + webhookUrl, + !!form.webhook?.isRetryEnabled, ) } diff --git a/src/app/modules/submission/submission.errors.ts b/src/app/modules/submission/submission.errors.ts index 5bf1a82e4b..105c42e99c 100644 --- a/src/app/modules/submission/submission.errors.ts +++ b/src/app/modules/submission/submission.errors.ts @@ -12,7 +12,7 @@ export class ConflictError extends ApplicationError { } export class SubmissionNotFoundError extends ApplicationError { - constructor(message: string) { + constructor(message = 'Submission not found for given ID') { super(message) } } diff --git a/src/app/modules/webhook/__tests__/webhook.consumer.spec.ts b/src/app/modules/webhook/__tests__/webhook.consumer.spec.ts new file mode 100644 index 0000000000..9a75934623 --- /dev/null +++ b/src/app/modules/webhook/__tests__/webhook.consumer.spec.ts @@ -0,0 +1,285 @@ +import aws from 'aws-sdk' +import { ObjectId } from 'bson' +import { addHours } from 'date-fns' +import mongoose from 'mongoose' +import { errAsync, okAsync } from 'neverthrow' +import { mocked } from 'ts-jest/utils' + +import { getEncryptSubmissionModel } from 'src/app/models/submission.server.model' +import { IWebhookResponse, SubmissionWebhookInfo } from 'src/types' + +import dbHandler from 'tests/unit/backend/helpers/jest-db' + +import { createWebhookQueueHandler } from '../webhook.consumer' +import { WebhookPushToQueueError } from '../webhook.errors' +import { WebhookProducer } from '../webhook.producer' +import * as WebhookService from '../webhook.service' +import { WebhookQueueMessageObject } from '../webhook.types' + +jest.mock('../webhook.service') +const MockWebhookService = mocked(WebhookService, true) + +const EncryptSubmissionModel = getEncryptSubmissionModel(mongoose) + +const MOCK_WEBHOOK_SUCCESS_RESPONSE: IWebhookResponse = { + signature: 'mockSignature', + webhookUrl: 'mockWebhookUrl', + response: { + data: 'mockData', + headers: 'mockHeaders', + status: 200, + }, +} +const MOCK_WEBHOOK_FAILURE_RESPONSE: IWebhookResponse = { + signature: 'mockSignature', + webhookUrl: 'mockWebhookUrl', + response: { + data: 'mockData', + headers: 'mockHeaders', + status: 400, + }, +} + +const SUCCESS_PRODUCER = { + sendMessage: jest.fn().mockReturnValue(okAsync(true)), +} as unknown as WebhookProducer + +const FAILURE_PRODUCER = { + sendMessage: jest + .fn() + .mockReturnValue(errAsync(new WebhookPushToQueueError())), +} as unknown as WebhookProducer + +const VALID_MESSAGE_BODY: WebhookQueueMessageObject = { + submissionId: new ObjectId().toHexString(), + previousAttempts: [Date.now()], + nextAttempt: Date.now(), + _v: 0, +} + +const VALID_SQS_MESSAGE: aws.SQS.Message = { + Body: JSON.stringify(VALID_MESSAGE_BODY), +} + +const MOCK_WEBHOOK_INFO = { + isRetryEnabled: true, + webhookUrl: 'some url', + webhookView: { + data: { + submissionId: VALID_MESSAGE_BODY.submissionId, + }, + }, +} as SubmissionWebhookInfo + +describe('webhook.consumer', () => { + beforeAll(async () => await dbHandler.connect()) + beforeEach(() => { + jest.clearAllMocks() + jest.restoreAllMocks() + }) + afterAll(async () => await dbHandler.closeDatabase()) + + describe('createWebhookQueueHandler', () => { + it('should reject when message body is undefined', async () => { + const result = createWebhookQueueHandler(SUCCESS_PRODUCER)({}) + + await expect(result).toReject() + }) + + it('should reject when message body cannot be parsed', async () => { + const result = createWebhookQueueHandler(SUCCESS_PRODUCER)({ + Body: 'yoooooooooooo', + }) + + await expect(result).toReject() + }) + + it('should requeue webhook when it is not due', async () => { + const message = { + Body: JSON.stringify({ + ...VALID_MESSAGE_BODY, + // next attempt in the future + nextAttempt: addHours(Date.now(), 1).getTime(), + }), + } + + await expect( + createWebhookQueueHandler(SUCCESS_PRODUCER)(message), + ).toResolve() + expect(MockWebhookService.sendWebhook).not.toHaveBeenCalled() + expect(MockWebhookService.saveWebhookRecord).not.toHaveBeenCalled() + expect(SUCCESS_PRODUCER.sendMessage).toHaveBeenCalled() + }) + + it('should reject when it fails to requeue webhook which is not due', async () => { + const message = { + Body: JSON.stringify({ + ...VALID_MESSAGE_BODY, + // next attempt in the future + nextAttempt: addHours(Date.now(), 1).getTime(), + }), + } + + await expect( + createWebhookQueueHandler(FAILURE_PRODUCER)(message), + ).toReject() + expect(MockWebhookService.sendWebhook).not.toHaveBeenCalled() + expect(MockWebhookService.saveWebhookRecord).not.toHaveBeenCalled() + expect(FAILURE_PRODUCER.sendMessage).toHaveBeenCalled() + }) + + it('should reject when submission ID cannot be found', async () => { + jest + .spyOn(EncryptSubmissionModel, 'retrieveWebhookInfoById') + .mockResolvedValueOnce(null) + + await expect( + createWebhookQueueHandler(SUCCESS_PRODUCER)(VALID_SQS_MESSAGE), + ).toReject() + expect(MockWebhookService.sendWebhook).not.toHaveBeenCalled() + expect(MockWebhookService.saveWebhookRecord).not.toHaveBeenCalled() + expect(SUCCESS_PRODUCER.sendMessage).not.toHaveBeenCalled() + }) + + it('should reject when database error occurs', async () => { + jest + .spyOn(EncryptSubmissionModel, 'retrieveWebhookInfoById') + .mockRejectedValueOnce(new Error('')) + + await expect( + createWebhookQueueHandler(SUCCESS_PRODUCER)(VALID_SQS_MESSAGE), + ).toReject() + expect(MockWebhookService.sendWebhook).not.toHaveBeenCalled() + expect(MockWebhookService.saveWebhookRecord).not.toHaveBeenCalled() + expect(SUCCESS_PRODUCER.sendMessage).not.toHaveBeenCalled() + }) + + it('should resolve when form has no webhook URL', async () => { + jest + .spyOn(EncryptSubmissionModel, 'retrieveWebhookInfoById') + .mockResolvedValueOnce({ + ...MOCK_WEBHOOK_INFO, + webhookUrl: '', + }) + + await expect( + createWebhookQueueHandler(SUCCESS_PRODUCER)(VALID_SQS_MESSAGE), + ).toResolve() + expect(MockWebhookService.sendWebhook).not.toHaveBeenCalled() + expect(MockWebhookService.saveWebhookRecord).not.toHaveBeenCalled() + expect(SUCCESS_PRODUCER.sendMessage).not.toHaveBeenCalled() + }) + + it('should resolve when form does not have retries enabled', async () => { + jest + .spyOn(EncryptSubmissionModel, 'retrieveWebhookInfoById') + .mockResolvedValueOnce({ + ...MOCK_WEBHOOK_INFO, + isRetryEnabled: false, + }) + + await expect( + createWebhookQueueHandler(SUCCESS_PRODUCER)(VALID_SQS_MESSAGE), + ).toResolve() + expect(MockWebhookService.sendWebhook).not.toHaveBeenCalled() + expect(MockWebhookService.saveWebhookRecord).not.toHaveBeenCalled() + expect(SUCCESS_PRODUCER.sendMessage).not.toHaveBeenCalled() + }) + + it('should resolve without requeuing when webhook succeeds', async () => { + jest + .spyOn(EncryptSubmissionModel, 'retrieveWebhookInfoById') + .mockResolvedValueOnce(MOCK_WEBHOOK_INFO) + MockWebhookService.sendWebhook.mockReturnValueOnce( + okAsync(MOCK_WEBHOOK_SUCCESS_RESPONSE), + ) + + await expect( + createWebhookQueueHandler(SUCCESS_PRODUCER)(VALID_SQS_MESSAGE), + ).toResolve() + expect(MockWebhookService.sendWebhook).toHaveBeenCalledWith( + MOCK_WEBHOOK_INFO.webhookView, + MOCK_WEBHOOK_INFO.webhookUrl, + ) + expect(MockWebhookService.saveWebhookRecord).toHaveBeenCalledWith( + VALID_MESSAGE_BODY.submissionId, + MOCK_WEBHOOK_SUCCESS_RESPONSE, + ) + expect(SUCCESS_PRODUCER.sendMessage).not.toHaveBeenCalled() + }) + + it('should requeue webhook when retry fails and there are retries remaining', async () => { + jest + .spyOn(EncryptSubmissionModel, 'retrieveWebhookInfoById') + .mockResolvedValueOnce(MOCK_WEBHOOK_INFO) + MockWebhookService.sendWebhook.mockReturnValueOnce( + // note failure response instead of success + okAsync(MOCK_WEBHOOK_FAILURE_RESPONSE), + ) + + await expect( + createWebhookQueueHandler(SUCCESS_PRODUCER)(VALID_SQS_MESSAGE), + ).toResolve() + expect(MockWebhookService.sendWebhook).toHaveBeenCalledWith( + MOCK_WEBHOOK_INFO.webhookView, + MOCK_WEBHOOK_INFO.webhookUrl, + ) + expect(MockWebhookService.saveWebhookRecord).toHaveBeenCalledWith( + VALID_MESSAGE_BODY.submissionId, + MOCK_WEBHOOK_FAILURE_RESPONSE, + ) + expect(SUCCESS_PRODUCER.sendMessage).toHaveBeenCalled() + }) + + it('should resolve without requeuing when retry fails and there are no retries remaining', async () => { + jest + .spyOn(EncryptSubmissionModel, 'retrieveWebhookInfoById') + .mockResolvedValueOnce(MOCK_WEBHOOK_INFO) + MockWebhookService.sendWebhook.mockReturnValueOnce( + okAsync(MOCK_WEBHOOK_SUCCESS_RESPONSE), + ) + const message = { + Body: JSON.stringify({ + ...VALID_MESSAGE_BODY, + // length greater than max possible number of retries + previousAttempts: Array(10).fill(0), + }), + } + + await expect( + createWebhookQueueHandler(SUCCESS_PRODUCER)(message), + ).toResolve() + expect(MockWebhookService.sendWebhook).toHaveBeenCalledWith( + MOCK_WEBHOOK_INFO.webhookView, + MOCK_WEBHOOK_INFO.webhookUrl, + ) + expect(MockWebhookService.saveWebhookRecord).toHaveBeenCalledWith( + VALID_MESSAGE_BODY.submissionId, + MOCK_WEBHOOK_SUCCESS_RESPONSE, + ) + expect(SUCCESS_PRODUCER.sendMessage).not.toHaveBeenCalled() + }) + + it('should reject when retry fails and subsequently fails to be requeued', async () => { + jest + .spyOn(EncryptSubmissionModel, 'retrieveWebhookInfoById') + .mockResolvedValueOnce(MOCK_WEBHOOK_INFO) + MockWebhookService.sendWebhook.mockReturnValueOnce( + okAsync(MOCK_WEBHOOK_FAILURE_RESPONSE), + ) + + await expect( + createWebhookQueueHandler(FAILURE_PRODUCER)(VALID_SQS_MESSAGE), + ).toReject() + expect(MockWebhookService.sendWebhook).toHaveBeenCalledWith( + MOCK_WEBHOOK_INFO.webhookView, + MOCK_WEBHOOK_INFO.webhookUrl, + ) + expect(MockWebhookService.saveWebhookRecord).toHaveBeenCalledWith( + VALID_MESSAGE_BODY.submissionId, + MOCK_WEBHOOK_FAILURE_RESPONSE, + ) + expect(FAILURE_PRODUCER.sendMessage).toHaveBeenCalled() + }) + }) +}) diff --git a/src/app/modules/webhook/__tests__/webhook.message.spec.ts b/src/app/modules/webhook/__tests__/webhook.message.spec.ts new file mode 100644 index 0000000000..579a4ec0f6 --- /dev/null +++ b/src/app/modules/webhook/__tests__/webhook.message.spec.ts @@ -0,0 +1,193 @@ +import { ObjectId } from 'bson' + +import { + DUE_TIME_TOLERANCE_SECONDS, + QUEUE_MESSAGE_VERSION, + RETRY_INTERVALS, +} from '../webhook.constants' +import { + WebhookNoMoreRetriesError, + WebhookQueueMessageParsingError, +} from '../webhook.errors' +import { WebhookQueueMessage } from '../webhook.message' +import { WebhookQueueMessageObject } from '../webhook.types' +import { prettifyEpoch } from '../webhook.utils' + +describe('WebhookQueueMessage', () => { + const VALID_MESSAGE: WebhookQueueMessageObject = { + submissionId: new ObjectId().toHexString(), + previousAttempts: [Date.now()], + nextAttempt: Date.now(), + _v: 0, + } + + beforeEach(() => { + jest.clearAllMocks() + }) + + describe('deserialise', () => { + it('should return WebhookQueueMessageParsingError when string is invalid JSON', () => { + const result = WebhookQueueMessage.deserialise('tis') + + expect(result._unsafeUnwrapErr()).toBeInstanceOf( + WebhookQueueMessageParsingError, + ) + }) + + it('should return WebhookQueueMessageParsingError when JSON has invalid shape', () => { + const result = WebhookQueueMessage.deserialise( + JSON.stringify({ but: 'a' }), + ) + + expect(result._unsafeUnwrapErr()).toBeInstanceOf( + WebhookQueueMessageParsingError, + ) + }) + + it('should return WebhookQueueMessageParsingError when submissionId is not an ObjectId', () => { + const result = WebhookQueueMessage.deserialise( + JSON.stringify({ + ...VALID_MESSAGE, + submissionId: 'flesh wound', + }), + ) + + expect(result._unsafeUnwrapErr()).toBeInstanceOf( + WebhookQueueMessageParsingError, + ) + }) + + it('should return instance of WebhookQueueMessage when input is valid', () => { + const result = WebhookQueueMessage.deserialise( + JSON.stringify(VALID_MESSAGE), + ) + + expect(result._unsafeUnwrap().message).toEqual(VALID_MESSAGE) + }) + }) + + describe('fromSubmissionId', () => { + const MOCK_NOW = Date.now() + + beforeAll(() => { + jest.spyOn(Date, 'now').mockReturnValue(MOCK_NOW) + }) + + afterAll(() => jest.restoreAllMocks()) + + it('should correctly create a WebhookQueueMessage without any retry history', () => { + const submissionId = new ObjectId().toHexString() + const result = WebhookQueueMessage.fromSubmissionId(submissionId) + + expect(result._unsafeUnwrap().message).toEqual({ + submissionId, + previousAttempts: [MOCK_NOW], + nextAttempt: expect.any(Number), + _v: QUEUE_MESSAGE_VERSION, + }) + }) + }) + + describe('serialise', () => { + it('should return stringified message', () => { + const msg = new WebhookQueueMessage(VALID_MESSAGE) + + expect(msg.serialise()).toEqual(JSON.stringify(VALID_MESSAGE)) + }) + }) + + describe('isDue', () => { + const MOCK_NOW = Date.now() + + beforeAll(() => { + jest.spyOn(Date, 'now').mockReturnValue(MOCK_NOW) + }) + + afterAll(() => jest.restoreAllMocks()) + + it('should return true if nextAttempt is in the past', () => { + const msg = new WebhookQueueMessage({ + ...VALID_MESSAGE, + nextAttempt: MOCK_NOW - 1, + }) + + expect(msg.isDue()).toBe(true) + }) + + it('should return true if nextAttempt is in the future but within tolerance', () => { + const msg = new WebhookQueueMessage({ + ...VALID_MESSAGE, + nextAttempt: MOCK_NOW + DUE_TIME_TOLERANCE_SECONDS * 1000 - 1, + }) + + expect(msg.isDue()).toBe(true) + }) + + it('should return false if nextAttempt is in the future and outside tolerance', () => { + const msg = new WebhookQueueMessage({ + ...VALID_MESSAGE, + nextAttempt: MOCK_NOW + DUE_TIME_TOLERANCE_SECONDS * 1000 + 1, + }) + + expect(msg.isDue()).toBe(true) + }) + }) + + describe('incrementAttempts', () => { + it('should return incremented attempts when retries have not been exhausted', () => { + const msg = new WebhookQueueMessage(VALID_MESSAGE) + + const result = msg.incrementAttempts()._unsafeUnwrap() + + expect(result.message.previousAttempts).toEqual([ + ...VALID_MESSAGE.previousAttempts, + VALID_MESSAGE.nextAttempt, + ]) + expect(result.message.submissionId).toBe(VALID_MESSAGE.submissionId) + // nextAttempt should have been incremented + expect(result.message.nextAttempt).toBeGreaterThan( + VALID_MESSAGE.nextAttempt, + ) + }) + + it('should return WebhookNoMoreRetriesError when retries have been exhausted', () => { + const msg = new WebhookQueueMessage({ + ...VALID_MESSAGE, + // length greater than allowed number of retries + previousAttempts: Array(RETRY_INTERVALS.length).fill(0), + }) + + const result = msg.incrementAttempts()._unsafeUnwrapErr() + + expect(result).toBeInstanceOf(WebhookNoMoreRetriesError) + }) + }) + + describe('getRetriesFailedState', () => { + it('should correctly convert message to failed state', () => { + const msg = new WebhookQueueMessage(VALID_MESSAGE) + + expect(msg.getRetriesFailedState()).toEqual({ + submissionId: VALID_MESSAGE.submissionId, + previousAttempts: [ + ...VALID_MESSAGE.previousAttempts, + VALID_MESSAGE.nextAttempt, + ].map(prettifyEpoch), + _v: VALID_MESSAGE._v, + }) + }) + }) + + describe('prettify', () => { + it('should return human-readable form of message', () => { + const msg = new WebhookQueueMessage(VALID_MESSAGE) + + expect(msg.prettify()).toEqual({ + submissionId: VALID_MESSAGE.submissionId, + previousAttempts: VALID_MESSAGE.previousAttempts.map(prettifyEpoch), + nextAttempt: prettifyEpoch(VALID_MESSAGE.nextAttempt), + _v: VALID_MESSAGE._v, + }) + }) + }) +}) diff --git a/src/app/modules/webhook/__tests__/webhook.producer.spec.ts b/src/app/modules/webhook/__tests__/webhook.producer.spec.ts new file mode 100644 index 0000000000..eeb54b225d --- /dev/null +++ b/src/app/modules/webhook/__tests__/webhook.producer.spec.ts @@ -0,0 +1,145 @@ +import { ObjectId } from 'bson' +import { addHours, addMinutes, subMinutes } from 'date-fns' +import { Producer } from 'sqs-producer' +import { mocked } from 'ts-jest/utils' + +import { MAX_DELAY_SECONDS } from '../webhook.constants' +import { WebhookPushToQueueError } from '../webhook.errors' +import { WebhookQueueMessage } from '../webhook.message' +import { WebhookProducer } from '../webhook.producer' + +jest.mock('sqs-producer') +const MockSqsProducer = mocked(Producer, true) + +describe('WebhookProducer', () => { + let webhookProducer: WebhookProducer + const mockSendMessage = jest.fn() + + const MOCK_NOW = Date.now() + + const MESSAGE_BODY = { + submissionId: new ObjectId().toHexString(), + previousAttempts: [MOCK_NOW], + nextAttempt: MOCK_NOW, + _v: 0, + } + + beforeAll(() => { + MockSqsProducer.create.mockReturnValue({ + send: mockSendMessage, + } as unknown as Producer) + webhookProducer = new WebhookProducer('') + }) + + beforeEach(() => { + jest.resetAllMocks() + jest.spyOn(Date, 'now').mockReturnValue(MOCK_NOW) + }) + + describe('sendMessage', () => { + it('should return true when message is sent on first try', async () => { + mockSendMessage.mockResolvedValueOnce([]) + const webhookMessage = new WebhookQueueMessage(MESSAGE_BODY) + + const result = await webhookProducer.sendMessage(webhookMessage) + + expect(result._unsafeUnwrap()).toBe(true) + expect(mockSendMessage).toHaveBeenCalledTimes(1) + expect(mockSendMessage).toHaveBeenCalledWith({ + body: JSON.stringify(webhookMessage.message), + id: webhookMessage.submissionId, + delaySeconds: expect.any(Number), + }) + }) + + it('should return true when message fails on first try, but subsequently succeeds', async () => { + mockSendMessage + .mockRejectedValueOnce(new Error('')) + .mockResolvedValueOnce([]) + const webhookMessage = new WebhookQueueMessage(MESSAGE_BODY) + + const result = await webhookProducer.sendMessage(webhookMessage) + + expect(result._unsafeUnwrap()).toBe(true) + expect(mockSendMessage).toHaveBeenCalledTimes(2) + expect(mockSendMessage).toHaveBeenCalledWith({ + body: JSON.stringify(webhookMessage.message), + id: webhookMessage.submissionId, + delaySeconds: expect.any(Number), + }) + }) + + it('should return WebhookPushToQueueError when message fails all attempts to be sent', async () => { + mockSendMessage.mockRejectedValue(new Error('')) + const webhookMessage = new WebhookQueueMessage(MESSAGE_BODY) + + const result = await webhookProducer.sendMessage(webhookMessage, { + minTimeout: 0, + }) + + expect(result._unsafeUnwrapErr()).toEqual(new WebhookPushToQueueError()) + // promise-retry retries 10 times by default, so total is 1 try + 10 retries = 11 + expect(mockSendMessage).toHaveBeenCalledTimes(11) + expect(mockSendMessage).toHaveBeenCalledWith({ + body: JSON.stringify(webhookMessage.message), + id: webhookMessage.submissionId, + delaySeconds: expect.any(Number), + }) + }) + + it('should queue message with 0 delay when nextAttempt is in the past', async () => { + mockSendMessage.mockResolvedValueOnce([]) + const webhookMessage = new WebhookQueueMessage({ + ...MESSAGE_BODY, + nextAttempt: subMinutes(MOCK_NOW, 10).getTime(), + }) + + const result = await webhookProducer.sendMessage(webhookMessage) + + expect(result._unsafeUnwrap()).toBe(true) + expect(mockSendMessage).toHaveBeenCalledTimes(1) + expect(mockSendMessage).toHaveBeenCalledWith({ + body: JSON.stringify(webhookMessage.message), + id: webhookMessage.submissionId, + delaySeconds: 0, + }) + }) + + it('should queue message with a maximum of 15min delay when nextAttempt is in the future', async () => { + mockSendMessage.mockResolvedValueOnce([]) + const webhookMessage = new WebhookQueueMessage({ + ...MESSAGE_BODY, + nextAttempt: addHours(MOCK_NOW, 10).getTime(), + }) + + const result = await webhookProducer.sendMessage(webhookMessage) + + expect(result._unsafeUnwrap()).toBe(true) + expect(mockSendMessage).toHaveBeenCalledTimes(1) + expect(mockSendMessage).toHaveBeenCalledWith({ + body: JSON.stringify(webhookMessage.message), + id: webhookMessage.submissionId, + delaySeconds: MAX_DELAY_SECONDS, + }) + }) + + it('should queue message with exactly the required delay of nextAttempt is less than 15min in the future', async () => { + const minutesInFuture = 10 + mockSendMessage.mockResolvedValueOnce([]) + const webhookMessage = new WebhookQueueMessage({ + ...MESSAGE_BODY, + nextAttempt: addMinutes(MOCK_NOW, minutesInFuture).getTime(), + }) + + const result = await webhookProducer.sendMessage(webhookMessage) + + expect(result._unsafeUnwrap()).toBe(true) + expect(mockSendMessage).toHaveBeenCalledTimes(1) + expect(mockSendMessage).toHaveBeenCalledWith({ + body: JSON.stringify(webhookMessage.message), + id: webhookMessage.submissionId, + delaySeconds: minutesInFuture * 60, + }) + }) + }) +}) diff --git a/src/app/modules/webhook/__tests__/webhook.service.spec.ts b/src/app/modules/webhook/__tests__/webhook.service.spec.ts index a42645c3eb..c763f183aa 100644 --- a/src/app/modules/webhook/__tests__/webhook.service.spec.ts +++ b/src/app/modules/webhook/__tests__/webhook.service.spec.ts @@ -1,10 +1,10 @@ import axios, { AxiosError, AxiosRequestConfig, AxiosResponse } from 'axios' -import { ObjectID } from 'bson' +import { ObjectId } from 'bson' import mongoose from 'mongoose' +import { ok, okAsync } from 'neverthrow' import { mocked } from 'ts-jest/utils' import formsgSdk from 'src/app/config/formsg-sdk' -import getFormModel from 'src/app/models/form.server.model' import { getEncryptSubmissionModel } from 'src/app/models/submission.server.model' import { WebhookValidationError } from 'src/app/modules/webhook/webhook.errors' import * as WebhookValidationModule from 'src/app/modules/webhook/webhook.validation' @@ -12,14 +12,15 @@ import { transformMongoError } from 'src/app/utils/handle-mongo-error' import { IEncryptedSubmissionSchema, IWebhookResponse, - ResponseMode, WebhookView, } from 'src/types' import dbHandler from 'tests/unit/backend/helpers/jest-db' import { SubmissionNotFoundError } from '../../submission/submission.errors' -import { saveWebhookRecord, sendWebhook } from '../webhook.service' +import { WebhookQueueMessage } from '../webhook.message' +import { WebhookProducer } from '../webhook.producer' +import * as WebhookService from '../webhook.service' // define suite-wide mocks jest.mock('axios') @@ -28,10 +29,16 @@ const MockAxios = mocked(axios, true) jest.mock('src/app/modules/webhook/webhook.validation') const MockWebhookValidationModule = mocked(WebhookValidationModule, true) -// define test constants -const FormModel = getFormModel(mongoose) +jest.mock('src/app/config/formsg-sdk') +const MockFormSgSdk = mocked(formsgSdk, true) + +jest.mock('../webhook.message.ts') +const MockWebhookQueueMessage = mocked(WebhookQueueMessage, true) + const EncryptSubmissionModel = getEncryptSubmissionModel(mongoose) +// define test constants + const MOCK_WEBHOOK_URL = 'https://form.gov.sg/endpoint' const DEFAULT_ERROR_MSG = 'a generic error has occurred' const AXIOS_ERROR_MSG = 'an axios error has occurred' @@ -78,6 +85,20 @@ const MOCK_WEBHOOK_DEFAULT_FORMAT_RESPONSE: Pick = } describe('webhook.service', () => { + const MOCK_FORM_ID = new ObjectId().toHexString() + const MOCK_SUBMISSION_ID = new ObjectId().toHexString() + const MOCK_WEBHOOK_VIEW: WebhookView = { + data: { + created: new Date(), + encryptedContent: 'mockEncryptedContent', + formId: MOCK_FORM_ID, + submissionId: MOCK_SUBMISSION_ID, + verifiedContent: 'mockVerifiedContent', + version: 1, + }, + } + const MOCK_SIGNATURE = 'mockSignature' + beforeAll(async () => await dbHandler.connect()) afterEach(async () => { await dbHandler.clearDatabase() @@ -85,57 +106,26 @@ describe('webhook.service', () => { afterAll(async () => await dbHandler.closeDatabase()) // test variables - let testEncryptedSubmission: IEncryptedSubmissionSchema let testConfig: AxiosRequestConfig - let testSubmissionWebhookView: WebhookView | null - let testSignature: string beforeEach(async () => { jest.restoreAllMocks() - // prepare for form creation workflow - const MOCK_ADMIN_OBJ_ID = new ObjectID() const MOCK_EPOCH = 1487076708000 - const preloaded = await dbHandler.insertFormCollectionReqs({ - userId: MOCK_ADMIN_OBJ_ID, - }) - jest.spyOn(Date, 'now').mockImplementation(() => MOCK_EPOCH) - // instantiate new form and save - const testEncryptedForm = await FormModel.create({ - title: 'Test Form', - admin: preloaded.user._id, - responseMode: ResponseMode.Encrypt, - publicKey: 'fake-public-key', - }) - - // initialise encrypted submussion - testEncryptedSubmission = await EncryptSubmissionModel.create({ - form: testEncryptedForm._id, - authType: testEncryptedForm.authType, - myInfoFields: [], - encryptedContent: 'encrypted-content', - verifiedContent: 'verified-content', - version: 1, - webhookResponses: [], - }) - - // initialise webhook related variables - testSubmissionWebhookView = testEncryptedSubmission.getWebhookView() - - testSignature = formsgSdk.webhooks.generateSignature({ - uri: MOCK_WEBHOOK_URL, - submissionId: testEncryptedSubmission._id, - formId: testEncryptedForm._id, - epoch: MOCK_EPOCH, - }) + MockFormSgSdk.webhooks.generateSignature.mockReturnValueOnce(MOCK_SIGNATURE) + const mockWebhookHeader = `t=${MOCK_EPOCH},s=${MOCK_SUBMISSION_ID},f=${MOCK_FORM_ID},v1=${MOCK_SIGNATURE}` + MockFormSgSdk.webhooks.constructHeader.mockReturnValueOnce( + mockWebhookHeader, + ) testConfig = { headers: { - 'X-FormSG-Signature': `t=${MOCK_EPOCH},s=${testEncryptedSubmission._id},f=${testEncryptedForm._id},v1=${testSignature}`, + 'X-FormSG-Signature': mockWebhookHeader, }, maxRedirects: 0, + timeout: 10000, } }) @@ -144,7 +134,7 @@ describe('webhook.service', () => { // Arrange const mockWebhookResponse = { ...MOCK_WEBHOOK_SUCCESS_RESPONSE, - signature: testSignature, + signature: MOCK_SIGNATURE, webhookUrl: MOCK_WEBHOOK_URL, } as IWebhookResponse @@ -155,8 +145,8 @@ describe('webhook.service', () => { .mockRejectedValueOnce(mockDBError) // Act - const actual = await saveWebhookRecord( - testEncryptedSubmission._id, + const actual = await WebhookService.saveWebhookRecord( + MOCK_SUBMISSION_ID, mockWebhookResponse, ) @@ -164,7 +154,7 @@ describe('webhook.service', () => { const expectedError = transformMongoError(mockDBError) expect(addWebhookResponseSpy).toHaveBeenCalledWith( - testEncryptedSubmission._id, + MOCK_SUBMISSION_ID, mockWebhookResponse, ) expect(actual._unsafeUnwrapErr()).toEqual(expectedError) @@ -174,13 +164,13 @@ describe('webhook.service', () => { // Arrange const mockWebhookResponse = { ...MOCK_WEBHOOK_SUCCESS_RESPONSE, - signature: testSignature, + signature: MOCK_SIGNATURE, webhookUrl: MOCK_WEBHOOK_URL, } as IWebhookResponse // Act - const actual = await saveWebhookRecord( - new ObjectID(), + const actual = await WebhookService.saveWebhookRecord( + new ObjectId(), mockWebhookResponse, ) @@ -195,15 +185,15 @@ describe('webhook.service', () => { it('should return updated submission with new webhook response if the record is successfully saved', async () => { // Arrange const mockWebhookResponse = { - _id: testEncryptedSubmission._id, - created: testEncryptedSubmission.created, + _id: MOCK_SUBMISSION_ID, + created: new Date(), ...MOCK_WEBHOOK_SUCCESS_RESPONSE, - signature: testSignature, + signature: MOCK_SIGNATURE, webhookUrl: MOCK_WEBHOOK_URL, } as IWebhookResponse const expectedSubmission = new EncryptSubmissionModel({ - ...testEncryptedSubmission, + _id: MOCK_SUBMISSION_ID, }) expectedSubmission.webhookResponses = [mockWebhookResponse] @@ -212,14 +202,14 @@ describe('webhook.service', () => { .mockResolvedValue(expectedSubmission) // Act - const actual = await saveWebhookRecord( - testEncryptedSubmission._id, + const actual = await WebhookService.saveWebhookRecord( + MOCK_SUBMISSION_ID, mockWebhookResponse, ) // Assert expect(addWebhookResponseSpy).toHaveBeenCalledWith( - testEncryptedSubmission._id, + MOCK_SUBMISSION_ID, mockWebhookResponse, ) expect(actual._unsafeUnwrap()).toEqual(expectedSubmission) @@ -234,8 +224,8 @@ describe('webhook.service', () => { ) // Act - const actual = await sendWebhook( - testEncryptedSubmission, + const actual = await WebhookService.sendWebhook( + MOCK_WEBHOOK_VIEW, MOCK_WEBHOOK_URL, ) @@ -255,8 +245,8 @@ describe('webhook.service', () => { ) // Act - const actual = await sendWebhook( - testEncryptedSubmission, + const actual = await WebhookService.sendWebhook( + MOCK_WEBHOOK_VIEW, MOCK_WEBHOOK_URL, ) @@ -285,28 +275,28 @@ describe('webhook.service', () => { toJSON: () => jest.fn(), } - expect( - MockWebhookValidationModule.validateWebhookUrl, - ).toHaveBeenCalledWith(MOCK_WEBHOOK_URL) MockAxios.post.mockRejectedValue(MOCK_AXIOS_ERROR) MockAxios.isAxiosError.mockReturnValue(true) // Act - const actual = await sendWebhook( - testEncryptedSubmission, + const actual = await WebhookService.sendWebhook( + MOCK_WEBHOOK_VIEW, MOCK_WEBHOOK_URL, ) // Assert const expectedResult = { ...MOCK_WEBHOOK_FAILURE_RESPONSE, - signature: testSignature, + signature: MOCK_SIGNATURE, webhookUrl: MOCK_WEBHOOK_URL, } + expect( + MockWebhookValidationModule.validateWebhookUrl, + ).toHaveBeenCalledWith(MOCK_WEBHOOK_URL) expect(MockAxios.post).toHaveBeenCalledWith( MOCK_WEBHOOK_URL, - testSubmissionWebhookView, + MOCK_WEBHOOK_VIEW, testConfig, ) expect(actual._unsafeUnwrap()).toEqual(expectedResult) @@ -320,15 +310,15 @@ describe('webhook.service', () => { MockAxios.isAxiosError.mockReturnValue(false) // Act - const actual = await sendWebhook( - testEncryptedSubmission, + const actual = await WebhookService.sendWebhook( + MOCK_WEBHOOK_VIEW, MOCK_WEBHOOK_URL, ) // Assert const expectedResult = { ...MOCK_WEBHOOK_DEFAULT_FORMAT_RESPONSE, - signature: testSignature, + signature: MOCK_SIGNATURE, webhookUrl: MOCK_WEBHOOK_URL, } @@ -337,7 +327,7 @@ describe('webhook.service', () => { ).toHaveBeenCalledWith(MOCK_WEBHOOK_URL) expect(MockAxios.post).toHaveBeenCalledWith( MOCK_WEBHOOK_URL, - testSubmissionWebhookView, + MOCK_WEBHOOK_VIEW, testConfig, ) expect(actual._unsafeUnwrap()).toEqual(expectedResult) @@ -353,15 +343,15 @@ describe('webhook.service', () => { MockAxios.isAxiosError.mockReturnValue(false) // Act - const actual = await sendWebhook( - testEncryptedSubmission, + const actual = await WebhookService.sendWebhook( + MOCK_WEBHOOK_VIEW, MOCK_WEBHOOK_URL, ) // Assert const expectedResult = { ...MOCK_WEBHOOK_DEFAULT_FORMAT_RESPONSE, - signature: testSignature, + signature: MOCK_SIGNATURE, webhookUrl: MOCK_WEBHOOK_URL, } @@ -370,7 +360,7 @@ describe('webhook.service', () => { ).toHaveBeenCalledWith(MOCK_WEBHOOK_URL) expect(MockAxios.post).toHaveBeenCalledWith( MOCK_WEBHOOK_URL, - testSubmissionWebhookView, + MOCK_WEBHOOK_VIEW, testConfig, ) expect(actual._unsafeUnwrap()).toEqual(expectedResult) @@ -383,15 +373,15 @@ describe('webhook.service', () => { MockAxios.post.mockResolvedValue(MOCK_AXIOS_SUCCESS_RESPONSE) // Act - const actual = await sendWebhook( - testEncryptedSubmission, + const actual = await WebhookService.sendWebhook( + MOCK_WEBHOOK_VIEW, MOCK_WEBHOOK_URL, ) // Assert const expectedResult = { ...MOCK_WEBHOOK_SUCCESS_RESPONSE, - signature: testSignature, + signature: MOCK_SIGNATURE, webhookUrl: MOCK_WEBHOOK_URL, } @@ -400,10 +390,85 @@ describe('webhook.service', () => { ).toHaveBeenCalledWith(MOCK_WEBHOOK_URL) expect(MockAxios.post).toHaveBeenCalledWith( MOCK_WEBHOOK_URL, - testSubmissionWebhookView, + MOCK_WEBHOOK_VIEW, testConfig, ) expect(actual._unsafeUnwrap()).toEqual(expectedResult) }) }) + + describe('createInitialWebhookSender', () => { + // This suite only checks for correct behaviour for webhook retries, + // since there are separate tests for sending webhooks and saving + // responses to the database. + let testSubmission: IEncryptedSubmissionSchema + const MOCK_PRODUCER = { + sendMessage: jest.fn().mockReturnValue(okAsync(true)), + } as unknown as WebhookProducer + beforeEach(() => { + jest.clearAllMocks() + + testSubmission = new EncryptSubmissionModel({ + _id: MOCK_SUBMISSION_ID, + }) + jest + .spyOn(EncryptSubmissionModel, 'addWebhookResponse') + .mockResolvedValue(testSubmission) + MockWebhookValidationModule.validateWebhookUrl.mockResolvedValue() + }) + + it('should return true without retrying when webhook is successful and retries are enabled', async () => { + MockAxios.post.mockResolvedValue(MOCK_AXIOS_SUCCESS_RESPONSE) + + const result = await WebhookService.createInitialWebhookSender( + MOCK_PRODUCER, + )(testSubmission, MOCK_WEBHOOK_URL, /* isRetryEnabled= */ true) + + expect(result._unsafeUnwrap()).toBe(true) + expect(MockWebhookQueueMessage.fromSubmissionId).not.toHaveBeenCalled() + }) + + it('should return true without retrying when webhook fails but retries are not enabled globally', async () => { + MockAxios.post.mockResolvedValue(MOCK_AXIOS_SUCCESS_RESPONSE) + + const result = await WebhookService + .createInitialWebhookSender + // no producer passed to createInitialWebhookSender, so retries not enabled globally + ()(testSubmission, MOCK_WEBHOOK_URL, true) + + expect(result._unsafeUnwrap()).toBe(true) + expect(MockWebhookQueueMessage.fromSubmissionId).not.toHaveBeenCalled() + }) + + it('should return true without retrying when webhook fails and retries are not enabled for form', async () => { + MockAxios.post.mockResolvedValue(MOCK_AXIOS_FAILURE_RESPONSE) + + const result = await WebhookService + .createInitialWebhookSender + // no producer passed to createInitialWebhookSender, so retries not enabled globally + ()(testSubmission, MOCK_WEBHOOK_URL, /* isRetryEnabled= */ false) + + expect(result._unsafeUnwrap()).toBe(true) + expect(MockWebhookQueueMessage.fromSubmissionId).not.toHaveBeenCalled() + }) + + it('should return true and retry when webhook fails and retries are enabled', async () => { + const mockQueueMessage = + 'mockQueueMessage' as unknown as WebhookQueueMessage + MockWebhookQueueMessage.fromSubmissionId.mockReturnValueOnce( + ok(mockQueueMessage), + ) + MockAxios.post.mockResolvedValue(MOCK_AXIOS_FAILURE_RESPONSE) + + const result = await WebhookService.createInitialWebhookSender( + MOCK_PRODUCER, + )(testSubmission, MOCK_WEBHOOK_URL, /* isRetryEnabled= */ true) + + expect(result._unsafeUnwrap()).toBe(true) + expect(MockWebhookQueueMessage.fromSubmissionId).toHaveBeenCalledWith( + String(testSubmission._id), + ) + expect(MOCK_PRODUCER.sendMessage).toHaveBeenCalledWith(mockQueueMessage) + }) + }) }) diff --git a/src/app/modules/webhook/__tests__/webhook.utils.spec.ts b/src/app/modules/webhook/__tests__/webhook.utils.spec.ts new file mode 100644 index 0000000000..7dc36f52f1 --- /dev/null +++ b/src/app/modules/webhook/__tests__/webhook.utils.spec.ts @@ -0,0 +1,71 @@ +import { addHours, addMinutes } from 'date-fns' +import { last } from 'lodash' +import { mocked } from 'ts-jest/utils' + +import { randomUniformInt } from 'src/app/utils/random-uniform' + +import { MAX_DELAY_SECONDS, RETRY_INTERVALS } from '../webhook.constants' +import { WebhookNoMoreRetriesError } from '../webhook.errors' +import { calculateDelaySeconds, getNextAttempt } from '../webhook.utils' + +jest.mock('src/app/utils/random-uniform') +const MockRandomUniformInt = mocked(randomUniformInt, true) + +describe('webhook.utils', () => { + const MOCK_NOW = Date.now() + const MOCK_RANDOM_INT = 37 + + beforeAll(() => { + jest.spyOn(Date, 'now').mockReturnValue(MOCK_NOW) + MockRandomUniformInt.mockReturnValue(MOCK_RANDOM_INT) + }) + describe('getNextAttempt', () => { + it('should return WebhookNoMoreRetriesError when retry limit is exceeded', () => { + // array of previous attempts is equal to RETRY_INTERVALS + 1, meaning + // all retries are used up (in addition to 1 initial webhook attempt) + const result = getNextAttempt(Array(RETRY_INTERVALS.length + 1).fill(0)) + + expect(result._unsafeUnwrapErr()).toEqual(new WebhookNoMoreRetriesError()) + }) + + it('should return time of next attempt correctly when there are retries remaining', () => { + // total number of allowed attempts is RETRY_INTERVALS.length + 1, with the +1 + // accounting for the initial attempt + const result = getNextAttempt( + Array(RETRY_INTERVALS.length).fill(MOCK_NOW), + ) + + const finalRetryInterval = last(RETRY_INTERVALS)! + expect(MockRandomUniformInt).toHaveBeenCalledWith( + finalRetryInterval.base - finalRetryInterval.jitter, + finalRetryInterval.base + finalRetryInterval.jitter, + ) + // previousAttempts array was filled with MOCK_NOW, so next attempt is calculated + // from MOCK_NOW + expect(result._unsafeUnwrap()).toBe(MOCK_NOW + MOCK_RANDOM_INT * 1000) + }) + }) + + describe('calculateDelaySeconds', () => { + it('should return 0 when nextAttempt is in the past', () => { + const result = calculateDelaySeconds(MOCK_NOW - 1000) + + expect(result).toBe(0) + }) + + it('should return a maximum of 15min regardless of how far nextAttempt is in the future', () => { + const result = calculateDelaySeconds(addHours(MOCK_NOW, 12).getTime()) + + expect(result).toBe(MAX_DELAY_SECONDS) + }) + + it('should return exactly the time to nextAttempt if it is less than 15min in the future', () => { + const minutesInFuture = 10 + const result = calculateDelaySeconds( + addMinutes(MOCK_NOW, minutesInFuture).getTime(), + ) + + expect(result).toBe(minutesInFuture * 60) + }) + }) +}) diff --git a/src/app/modules/webhook/webhook.constants.ts b/src/app/modules/webhook/webhook.constants.ts new file mode 100644 index 0000000000..1001941bc9 --- /dev/null +++ b/src/app/modules/webhook/webhook.constants.ts @@ -0,0 +1,52 @@ +import config from '../../config/config' + +import { RetryInterval } from './webhook.types' + +/** + * Current version of queue message format. + */ +export const QUEUE_MESSAGE_VERSION = 0 + +// Conversion to seconds +const hours = (h: number) => h * 60 * 60 +const minutes = (m: number) => m * 60 + +/** + * Encodes retry policy. + * Element 0 is time to wait + jitter before + * retrying the first time, element 1 is time to wait + * to wait + jitter before 2nd time, etc. + * All units are in seconds. + * + * @example [{ base: 10, jitter: 5}, { base: 20, jitter: 5 }] means + * the first retry is attempted between 10 - 5 = 5 seconds and + * 10 + 5 = 15 seconds after the submission. If the first retry fails, + * then the second retry is attempted between 15 and 25 seconds after + * the submission. + */ +export const RETRY_INTERVALS: RetryInterval[] = config.isDev + ? [ + { base: 10, jitter: 5 }, + { base: 20, jitter: 5 }, + { base: 30, jitter: 5 }, + ] + : [ + { base: minutes(5), jitter: minutes(1) }, + { base: hours(1), jitter: minutes(15) }, + { base: hours(2), jitter: minutes(30) }, + { base: hours(4), jitter: hours(1) }, + { base: hours(8), jitter: hours(2) }, + { base: hours(20), jitter: hours(4) }, + ] + +/** + * Max possible delay for a message, as specified by AWS. + */ +export const MAX_DELAY_SECONDS = minutes(15) + +/** + * Tolerance allowed for determining if a message is due to be sent. + * If a message's next attempt is scheduled either in the past or this + * number of seconds in the future, it will be sent. + */ +export const DUE_TIME_TOLERANCE_SECONDS = minutes(1) diff --git a/src/app/modules/webhook/webhook.consumer.ts b/src/app/modules/webhook/webhook.consumer.ts new file mode 100644 index 0000000000..1af9de0126 --- /dev/null +++ b/src/app/modules/webhook/webhook.consumer.ts @@ -0,0 +1,258 @@ +import aws from 'aws-sdk' +import https from 'https' +import mongoose from 'mongoose' +import { errAsync, okAsync, ResultAsync } from 'neverthrow' +import { Consumer } from 'sqs-consumer' + +import { SubmissionWebhookInfo } from '../../../types' +import config from '../../config/config' +import { createLoggerWithLabel } from '../../config/logger' +import { getEncryptSubmissionModel } from '../../models/submission.server.model' +import { transformMongoError } from '../../utils/handle-mongo-error' +import { PossibleDatabaseError } from '../core/core.errors' +import { SubmissionNotFoundError } from '../submission/submission.errors' + +import { + WebhookNoMoreRetriesError, + WebhookPushToQueueError, + WebhookRetriesNotEnabledError, + WebhookValidationError, +} from './webhook.errors' +import { WebhookQueueMessage } from './webhook.message' +import { WebhookProducer } from './webhook.producer' +import * as WebhookService from './webhook.service' +import { isSuccessfulResponse } from './webhook.utils' + +const logger = createLoggerWithLabel(module) +const EncryptSubmission = getEncryptSubmissionModel(mongoose) + +/** + * Starts polling a queue for webhook messages. + * @param queueUrl URL of queue from which to consume messages + * @param producer Producer which can be used to enqueue messages + */ +export const startWebhookConsumer = ( + queueUrl: string, + producer: WebhookProducer, +): void => { + const app = Consumer.create({ + queueUrl, + region: config.aws.region, + handleMessage: createWebhookQueueHandler(producer), + // By default, the default Node.js HTTP/HTTPS SQS agent + // creates a new TCP connection for every new request. + // In production, pass an SQS instance to avoid the cost + // of establishing new connections. + sqs: config.isDev + ? undefined + : new aws.SQS({ + region: config.aws.region, + httpOptions: { + agent: new https.Agent({ + keepAlive: true, + }), + }, + }), + }) + + app.on('error', (error, message) => { + logger.error({ + message: + 'Webhook consumer encountered error while interacting with queue', + meta: { + action: 'startWebhookConsumer', + message, + }, + error, + }) + }) + + app.start() + + logger.info({ + message: 'Webhook consumer started', + meta: { + action: 'startWebhookConsumer', + }, + }) +} + +/** + * Creates a handler to consume messages from webhook queue. + * This handler does the following: + * 1) Parses the message + * 2) If the webhook is not due, requeues the message + * 3) If the webhook is due, attempts the webhook + * 4) Records the webhook attempt in the database + * 5) If the webhook failed again, requeues the message + * + * Exported for testing. + * @param producer Producer which can write messages to queue + * @returns Handler for consumption of queue messages + */ +export const createWebhookQueueHandler = + (producer: WebhookProducer) => + async (sqsMessage: aws.SQS.Message): Promise => { + const { Body, MessageId } = sqsMessage + const logMeta = { + action: 'createWebhookQueueHandler', + MessageId, + } + logger.info({ + message: 'Consumed message from webhook queue', + meta: logMeta, + }) + if (!Body) { + logger.error({ + message: 'Webhook queue message contained undefined body', + meta: logMeta, + }) + // Malformed message will be retried until redrive policy is exceeded, + // upon which it will be moved to dead-letter queue + return Promise.reject() + } + + // Parse message + const webhookMessageResult = WebhookQueueMessage.deserialise(Body) + if (webhookMessageResult.isErr()) { + logger.error({ + message: 'Webhook queue message could not be parsed', + meta: logMeta, + error: webhookMessageResult.error, + }) + return Promise.reject() + } + const webhookMessage = webhookMessageResult.value + + // If not due, requeue + if (!webhookMessage.isDue()) { + logger.info({ + message: 'Webhook not due yet, requeueing', + meta: logMeta, + }) + const requeueResult = await producer.sendMessage(webhookMessage) + if (requeueResult.isErr()) { + logger.error({ + message: 'Webhook queue message could not be requeued', + meta: { + ...logMeta, + webhookMessage: webhookMessage.prettify(), + }, + error: requeueResult.error, + }) + // Reject so message is moved to DLQ + return Promise.reject() + } + // Delete existing message from queue + return Promise.resolve() + } + + // If due, send webhook + // First, retrieve webhook view and URL from database + const retryResult = await retrieveWebhookInfo( + webhookMessage.submissionId, + ).andThen< + true, + | WebhookRetriesNotEnabledError + | WebhookValidationError + | WebhookNoMoreRetriesError + | WebhookPushToQueueError + >((webhookInfo) => { + const { webhookUrl, isRetryEnabled } = webhookInfo + // Webhook URL was deleted or retries disabled + if (!webhookUrl || !isRetryEnabled) + return errAsync( + new WebhookRetriesNotEnabledError(webhookUrl, isRetryEnabled), + ) + + // Attempt webhook + return WebhookService.sendWebhook( + webhookInfo.webhookView, + webhookUrl, + ).andThen((webhookResponse) => { + // Save webhook response to database, but carry on even if it fails + void WebhookService.saveWebhookRecord( + webhookMessage.submissionId, + webhookResponse, + ) + + // Webhook was successful, no further action required + if (isSuccessfulResponse(webhookResponse)) return okAsync(true) + + // Requeue webhook for subsequent retry + return webhookMessage + .incrementAttempts() + .asyncAndThen((newMessage) => producer.sendMessage(newMessage)) + }) + }) + + if (retryResult.isOk()) return Promise.resolve() + // Error cases + // Special handling for max retries exceeded - log a separate message + // and resolve Promise so that message is removed from queue + if (retryResult.error instanceof WebhookNoMoreRetriesError) { + logger.warn({ + message: 'Maximum retries exceeded for webhook', + meta: { + action: 'createWebhookQueueHandler', + webhookMessage: webhookMessage.getRetriesFailedState(), + }, + }) + return Promise.resolve() + } + // Special handling for retries not enabled - this should not be moved + // to DLQ as admin has disabled webhooks and/or webhook retries on purpose + if (retryResult.error instanceof WebhookRetriesNotEnabledError) { + logger.warn({ + message: 'Webhook retries no longer enabled on form', + meta: { + action: 'createWebhookQueueHandler', + webhookMessage: webhookMessage.prettify(), + }, + }) + return Promise.resolve() + } + // Remaining cases are unexpected errors, move to DLQ + logger.error({ + message: 'Error while attempting to retry webhook', + meta: { + action: 'createWebhookQueueHandler', + webhookMessage: webhookMessage.prettify(), + }, + error: retryResult.error, + }) + // Reject so retry can be moved to dead-letter queue + // if redrive policy is exceeded + return Promise.reject() + } + +/** + * Retrieves all relevant information to send webhook for a given submission. + * @param submissionId + * @returns ok(webhook information) if database retrieval succeeds + * @returns err if submission ID does not exist or database retrieval errors + */ +const retrieveWebhookInfo = ( + submissionId: string, +): ResultAsync< + SubmissionWebhookInfo, + SubmissionNotFoundError | PossibleDatabaseError +> => { + return ResultAsync.fromPromise( + EncryptSubmission.retrieveWebhookInfoById(submissionId), + (error) => { + logger.error({ + message: 'Error while retrieving webhook info for submission', + meta: { + action: 'retrieveWebhookInfo', + submissionId, + }, + error, + }) + return transformMongoError(error) + }, + ).andThen((submissionInfo) => { + if (!submissionInfo) return errAsync(new SubmissionNotFoundError()) + return okAsync(submissionInfo) + }) +} diff --git a/src/app/modules/webhook/webhook.errors.ts b/src/app/modules/webhook/webhook.errors.ts index 210f4ff2ce..426d32f170 100644 --- a/src/app/modules/webhook/webhook.errors.ts +++ b/src/app/modules/webhook/webhook.errors.ts @@ -39,3 +39,61 @@ export class WebhookFailedWithAxiosError extends ApplicationError { this.meta = { originalError: error } } } + +/** + * Webhook queue message incorrectly formatted and hence could not be parsed + */ +export class WebhookQueueMessageParsingError extends ApplicationError { + meta: { + originalError: unknown + } + + constructor( + error: unknown, + message = 'Unable to parse body of webhook queue message', + ) { + super(message) + this.meta = { originalError: error } + } +} + +/** + * Maximum retries exceeded for webhook. + */ +export class WebhookNoMoreRetriesError extends ApplicationError { + constructor(message = 'Maximum retries exceeded for webhook') { + super(message) + } +} + +/** + * Failed to push message to SQS. + */ +export class WebhookPushToQueueError extends ApplicationError { + constructor(message = 'Failed to push webhook to message queue') { + super(message) + } +} + +/** + * Cannot send webhook retry because form has no webhook URL or does not have + * retries enabled. + */ +export class WebhookRetriesNotEnabledError extends ApplicationError { + meta: { + webhookUrl: string + isRetryEnabled: boolean + } + + constructor( + webhookUrl: string, + isRetryEnabled: boolean, + message = 'Unable to send webhook as form has no webhook URL or does not have retries enabled', + ) { + super(message) + this.meta = { + webhookUrl, + isRetryEnabled, + } + } +} diff --git a/src/app/modules/webhook/webhook.factory.ts b/src/app/modules/webhook/webhook.factory.ts index 9e2db715d3..f65b3aed22 100644 --- a/src/app/modules/webhook/webhook.factory.ts +++ b/src/app/modules/webhook/webhook.factory.ts @@ -6,22 +6,34 @@ import FeatureManager, { } from '../../config/feature-manager' import { MissingFeatureError } from '../core/core.errors' +import { startWebhookConsumer } from './webhook.consumer' +import { WebhookProducer } from './webhook.producer' import * as WebhookService from './webhook.service' interface IWebhookFactory { - sendWebhook: typeof WebhookService.sendWebhook - saveWebhookRecord: typeof WebhookService.saveWebhookRecord + sendInitialWebhook: ReturnType< + typeof WebhookService.createInitialWebhookSender + > } export const createWebhookFactory = ({ isEnabled, props, }: RegisteredFeature): IWebhookFactory => { - if (isEnabled && props) return WebhookService + if (isEnabled && props) { + const { webhookQueueUrl } = props + let producer: WebhookProducer | undefined + if (webhookQueueUrl) { + producer = new WebhookProducer(webhookQueueUrl) + startWebhookConsumer(webhookQueueUrl, producer) + } + return { + sendInitialWebhook: WebhookService.createInitialWebhookSender(producer), + } + } const error = new MissingFeatureError(FeatureNames.SpcpMyInfo) return { - sendWebhook: () => errAsync(error), - saveWebhookRecord: () => errAsync(error), + sendInitialWebhook: () => errAsync(error), } } diff --git a/src/app/modules/webhook/webhook.message.ts b/src/app/modules/webhook/webhook.message.ts new file mode 100644 index 0000000000..c26089749c --- /dev/null +++ b/src/app/modules/webhook/webhook.message.ts @@ -0,0 +1,181 @@ +import { differenceInSeconds } from 'date-fns' +import { Result } from 'neverthrow' + +import { createLoggerWithLabel } from '../../config/logger' + +import { + DUE_TIME_TOLERANCE_SECONDS, + QUEUE_MESSAGE_VERSION, +} from './webhook.constants' +import { + WebhookNoMoreRetriesError, + WebhookQueueMessageParsingError, +} from './webhook.errors' +import { + WebhookFailedQueueMessage, + webhookMessageSchema, + WebhookQueueMessageObject, + WebhookQueueMessagePrettified, +} from './webhook.types' +import { getNextAttempt, prettifyEpoch } from './webhook.utils' + +const logger = createLoggerWithLabel(module) + +/** + * Encapsulates a queue message for webhook retries. + */ +export class WebhookQueueMessage { + message: WebhookQueueMessageObject + + constructor(message: WebhookQueueMessageObject) { + this.message = message + } + + /** + * Converts a webhook queue message body into an encapsulated + * class instance. + * @param body Raw body of webhook queue message + * @returns ok(encapsulated message) if message can be parsed successfully + * @returns err if message fails to be parsed + */ + static deserialise( + body: string, + ): Result { + return Result.fromThrowable( + () => JSON.parse(body) as unknown, + (error) => { + logger.error({ + message: 'Unable to parse webhook queue message body', + meta: { + action: 'deserialise', + body, + }, + error, + }) + return new WebhookQueueMessageParsingError(error) + }, + )() + .andThen((parsed) => + Result.fromThrowable( + () => webhookMessageSchema.parse(parsed), + (error) => { + logger.error({ + message: 'Webhook queue message body has wrong shape', + meta: { + action: 'deserialise', + body, + }, + error, + }) + return new WebhookQueueMessageParsingError(error) + }, + )(), + ) + .map((validated) => new WebhookQueueMessage(validated)) + } + + /** + * Initialises a webhook queue message which has not been + * retried as yet. This function succeeds as long as + * the retry policy allows for at least one retry. + * + * Assumes that initial webhook has just been attempted, + * hence uses the current date as the time of the first + * webhook attempt. + * @param submissionId + * @returns ok(encapsulated message) if retry policy exists + * @returns err if the retry policy does not allow any retries + */ + static fromSubmissionId( + submissionId: string, + ): Result { + const initialAttempt = Date.now() + return getNextAttempt(/* previousAttempts =*/ [initialAttempt]).map( + (nextAttempt) => + new WebhookQueueMessage({ + submissionId, + previousAttempts: [initialAttempt], + nextAttempt, + _v: QUEUE_MESSAGE_VERSION, + }), + ) + } + + /** + * Serialises for enqueueing. + * @returns Serialised message + */ + serialise(): string { + return JSON.stringify(this.message) + } + + /** + * Determines whether the message is currently due to be sent. + * @returns true if webhook is currently due to be sent, false otherwise + */ + isDue(): boolean { + // Allow tolerance for clock drift + return ( + // Argument order is important. If nextAttempt is in the past, + // differenceInSeconds will return a negative number. + differenceInSeconds(this.message.nextAttempt, Date.now()) <= + DUE_TIME_TOLERANCE_SECONDS + ) + } + + /** + * Updates the message as having just been retried, and adds a new time for the + * next attempt. + * This function should only be called on a message for which the webhook has just + * been attempted and failed. + * @returns ok(WebhookQueueMessage) if message can still be retried + * @returns err(WebhookNoMoreRetriesError) if max retries have been exceeded + */ + incrementAttempts(): Result { + const updatedPreviousAttempts = [ + ...this.message.previousAttempts, + this.message.nextAttempt, + ] + return getNextAttempt(updatedPreviousAttempts).map( + (nextAttempt) => + new WebhookQueueMessage({ + submissionId: this.message.submissionId, + previousAttempts: updatedPreviousAttempts, + nextAttempt, + _v: QUEUE_MESSAGE_VERSION, + }), + ) + } + + /** + * Converts a message to reflect that all retries have failed. + * @returns Message converted into a failure shape + */ + getRetriesFailedState(): WebhookFailedQueueMessage { + return { + submissionId: this.submissionId, + previousAttempts: [ + ...this.message.previousAttempts, + this.nextAttempt, + ].map(prettifyEpoch), + _v: this.message._v, + } + } + + prettify(): WebhookQueueMessagePrettified { + return { + submissionId: this.submissionId, + previousAttempts: this.message.previousAttempts.map(prettifyEpoch), + nextAttempt: prettifyEpoch(this.nextAttempt), + _v: this.message._v, + } + } + + get submissionId(): string { + return this.message.submissionId + } + + get nextAttempt(): number { + return this.message.nextAttempt + } +} diff --git a/src/app/modules/webhook/webhook.producer.ts b/src/app/modules/webhook/webhook.producer.ts new file mode 100644 index 0000000000..f90aec62c7 --- /dev/null +++ b/src/app/modules/webhook/webhook.producer.ts @@ -0,0 +1,79 @@ +import { ResultAsync } from 'neverthrow' +import promiseRetry from 'promise-retry' +import { OperationOptions } from 'retry' +import { Producer } from 'sqs-producer' + +import config from '../../config/config' +import { createLoggerWithLabel } from '../../config/logger' + +import { WebhookPushToQueueError } from './webhook.errors' +import { WebhookQueueMessage } from './webhook.message' +import { calculateDelaySeconds } from './webhook.utils' + +const logger = createLoggerWithLabel(module) + +/** + * Encapsulates a producer which can write webhook retry messages + * to a message queue. + */ +export class WebhookProducer { + producer: Producer + + constructor(queueUrl: string) { + this.producer = Producer.create({ + queueUrl, + region: config.aws.region, + }) + } + + /** + * Enqueues a message. + * @param queueMessage Message to send + * @param retryOptions optional customisation of retry parameters + * @returns ok(true) if sending message suceeds + * @returns err if sending message fails + */ + sendMessage( + queueMessage: WebhookQueueMessage, + retryOptions?: OperationOptions, + ): ResultAsync { + const sendMessageRetry = promiseRetry(async (retry, attemptNum) => { + try { + await this.producer.send({ + body: queueMessage.serialise(), + id: queueMessage.submissionId, // only needs to be unique within request + delaySeconds: calculateDelaySeconds(queueMessage.nextAttempt), + }) + logger.info({ + message: `Pushed webhook to queue`, + meta: { + action: 'sendMessage', + webhookMessage: queueMessage.prettify(), + attemptNum, + }, + }) + return true + } catch (error) { + logger.error({ + message: `Failed to push webhook to queue`, + meta: { + action: 'sendMessage', + attemptNum, + }, + error, + }) + return retry(error) + } + }, retryOptions) + return ResultAsync.fromPromise(sendMessageRetry, (error) => { + logger.error({ + message: 'All attempts to push webhook to queue failed', + meta: { + action: 'sendMessage', + }, + error, + }) + return new WebhookPushToQueueError() + }) + } +} diff --git a/src/app/modules/webhook/webhook.service.ts b/src/app/modules/webhook/webhook.service.ts index ce4e6bf55c..be3f7feb19 100644 --- a/src/app/modules/webhook/webhook.service.ts +++ b/src/app/modules/webhook/webhook.service.ts @@ -7,6 +7,7 @@ import { IEncryptedSubmissionSchema, ISubmissionSchema, IWebhookResponse, + WebhookView, } from '../../../types' import formsgSdk from '../../config/formsg-sdk' import { createLoggerWithLabel } from '../../config/logger' @@ -18,9 +19,12 @@ import { SubmissionNotFoundError } from '../submission/submission.errors' import { WebhookFailedWithAxiosError, WebhookFailedWithUnknownError, + WebhookPushToQueueError, WebhookValidationError, } from './webhook.errors' -import { formatWebhookResponse } from './webhook.utils' +import { WebhookQueueMessage } from './webhook.message' +import { WebhookProducer } from './webhook.producer' +import { formatWebhookResponse, isSuccessfulResponse } from './webhook.utils' import { validateWebhookUrl } from './webhook.validation' const logger = createLoggerWithLabel(module) @@ -69,17 +73,11 @@ export const saveWebhookRecord = ( } export const sendWebhook = ( - submission: IEncryptedSubmissionSchema, + webhookView: WebhookView, webhookUrl: string, -): ResultAsync< - IWebhookResponse, - | WebhookValidationError - | WebhookFailedWithAxiosError - | WebhookFailedWithUnknownError -> => { +): ResultAsync => { const now = Date.now() - const submissionWebhookView = submission.getWebhookView() - const { submissionId, formId } = submissionWebhookView.data + const { submissionId, formId } = webhookView.data const signature = formsgSdk.webhooks.generateSignature({ uri: webhookUrl, @@ -109,7 +107,7 @@ export const sendWebhook = ( }) .andThen(() => ResultAsync.fromPromise( - axios.post(webhookUrl, submissionWebhookView, { + axios.post(webhookUrl, webhookView, { headers: { 'X-FormSG-Signature': formsgSdk.webhooks.constructHeader({ epoch: now, @@ -119,6 +117,9 @@ export const sendWebhook = ( }), }, maxRedirects: 0, + // Timeout after 10 seconds to allow for cold starts in receiver, + // e.g. Lambdas + timeout: 10 * 1000, }), (error) => { logger.error({ @@ -175,3 +176,43 @@ export const sendWebhook = ( }) }) } + +/** + * Creates a function which sends a webhook and saves the necessary records. + * This function sends the INITIAL webhook, which occurs immediately after + * a submission. If the initial webhook fails and retries are enabled, the + * webhook is queued for retries. + * @returns function which sends webhook and saves a record of it + */ +export const createInitialWebhookSender = + (producer?: WebhookProducer) => + ( + submission: IEncryptedSubmissionSchema, + webhookUrl: string, + isRetryEnabled: boolean, + ): ResultAsync< + true, + | WebhookValidationError + | PossibleDatabaseError + | SubmissionNotFoundError + | WebhookPushToQueueError + > => { + // Attempt to send webhook + return sendWebhook(submission.getWebhookView(), webhookUrl).andThen( + (webhookResponse) => + // Save record of sending to database + saveWebhookRecord(submission._id, webhookResponse).andThen(() => { + // If webhook successful or retries not enabled, no further action + if ( + isSuccessfulResponse(webhookResponse) || + !producer || + !isRetryEnabled + ) + return okAsync(true) + // Webhook failed and retries enabled, so create initial message and enqueue + return WebhookQueueMessage.fromSubmissionId( + String(submission._id), + ).asyncAndThen((queueMessage) => producer.sendMessage(queueMessage)) + }), + ) + } diff --git a/src/app/modules/webhook/webhook.types.ts b/src/app/modules/webhook/webhook.types.ts index 0a77e16e4d..f6256b02e2 100644 --- a/src/app/modules/webhook/webhook.types.ts +++ b/src/app/modules/webhook/webhook.types.ts @@ -1,9 +1,6 @@ -import { - IEncryptedSubmissionSchema, - IFormSchema, - ISubmissionSchema, - WebhookView, -} from '../../../types' +import * as z from 'zod' + +import { IFormSchema, ISubmissionSchema, WebhookView } from '../../../types' export interface WebhookParams { webhookUrl: string @@ -14,7 +11,45 @@ export interface WebhookParams { signature: string } -export interface WebhookRequestLocals { - form: IFormSchema - submission: IEncryptedSubmissionSchema +/** + * Schema for webhook queue message, which allows an object to be validated. + */ +export const webhookMessageSchema = z.object({ + submissionId: z.string().regex(/^[a-f\d]{24}$/i), + previousAttempts: z.array(z.number()), + nextAttempt: z.number(), + _v: z.number(), +}) + +/** + * Shape of webhook queue message object. + */ +export type WebhookQueueMessageObject = z.infer + +/** + * Webhook queue message object formatted for readable logs. + */ +export type WebhookQueueMessagePrettified = Omit< + WebhookQueueMessageObject, + 'previousAttempts' | 'nextAttempt' +> & { + previousAttempts: string[] + nextAttempt: string +} + +/** + * Failed webhook queue message formatted for readable logs. + * Same as a regular queue message except no next attempt. + */ +export type WebhookFailedQueueMessage = Omit< + WebhookQueueMessagePrettified, + 'nextAttempt' +> + +/** + * Specification of when a webhook should be retried. + */ +export type RetryInterval = { + base: number + jitter: number } diff --git a/src/app/modules/webhook/webhook.utils.ts b/src/app/modules/webhook/webhook.utils.ts index e3331465b4..8fb69408ab 100644 --- a/src/app/modules/webhook/webhook.utils.ts +++ b/src/app/modules/webhook/webhook.utils.ts @@ -1,7 +1,15 @@ import { AxiosResponse } from 'axios' +import { inRange } from 'lodash' +import moment from 'moment-timezone' +import { err, ok, Result } from 'neverthrow' import { stringifySafe } from '../../../shared/util/stringify-safe' import { IWebhookResponse } from '../../../types' +import { TIMEZONE } from '../../constants/timezone' +import { randomUniformInt } from '../../utils/random-uniform' + +import { MAX_DELAY_SECONDS, RETRY_INTERVALS } from './webhook.constants' +import { WebhookNoMoreRetriesError } from './webhook.errors' /** * Formats a response object for update in the Submissions collection @@ -14,3 +22,61 @@ export const formatWebhookResponse = ( headers: stringifySafe(response?.headers) ?? '', data: stringifySafe(response?.data) ?? '', }) + +/** + * Computes epoch of next webhook attempt based on previous attempts. + * @param previousAttempts Array of epochs of previous attempts + * @returns ok(epoch of next attempt) if there are valid retries remaining + * @returns err(WebhookNoMoreRetriesError) if there are no more retries remaining + */ +export const getNextAttempt = ( + previousAttempts: number[], +): Result => { + // Total allowed number of attempts is RETRY_INTERVALS + 1. + // The +1 accounts for the initial webhook attempt immediately + // after form submission. + if (previousAttempts.length >= RETRY_INTERVALS.length + 1) { + return err(new WebhookNoMoreRetriesError()) + } + // The -1 accounts for the initial webhook attempt, e.g. if + // the length of previousAttempts is 1, then we should get + // the interval for the first retry at RETRY_INTERVALS[0] + const interval = RETRY_INTERVALS[previousAttempts.length - 1] + const nextAttemptWaitTimeSeconds = randomUniformInt( + interval.base - interval.jitter, + interval.base + interval.jitter, + ) + // Calculate next attempt based on time from initial attempt, or + // current date if initial attempt does not exist + const initialAttempt = previousAttempts[0] ?? Date.now() + return ok(initialAttempt + nextAttemptWaitTimeSeconds * 1000) +} + +/** + * Encodes success condition of webhook. Webhooks are considered + * successful if the status code >= 200 and < 300. + * @param webhookResponse Response from receiving server + * @returns true if webhook was successful + */ +export const isSuccessfulResponse = ( + webhookResponse: IWebhookResponse, +): boolean => inRange(webhookResponse.response.status, 200, 300) + +/** + * Calculates the number of seconds to delay a message sent to + * the webhook queue. This is the minimum of (time to next attempt, + * max possible delay timeout). + * @param nextAttempt Epoch of next attempt + */ +export const calculateDelaySeconds = (nextAttempt: number): number => { + const secondsToNextAttempt = Math.max(0, (nextAttempt - Date.now()) / 1000) + return Math.min(secondsToNextAttempt, MAX_DELAY_SECONDS) +} + +/** + * Converts an epoch to a readable format. + * @param epoch + * @returns the epoch represented as a readable string + */ +export const prettifyEpoch = (epoch: number): string => + moment(epoch).tz(TIMEZONE).format('D MMM YYYY, h:mm:ssa z') diff --git a/src/app/routes/api/v3/admin/forms/__tests__/admin-forms.settings.routes.spec.ts b/src/app/routes/api/v3/admin/forms/__tests__/admin-forms.settings.routes.spec.ts index 45ae661a3e..b5b7f3546a 100644 --- a/src/app/routes/api/v3/admin/forms/__tests__/admin-forms.settings.routes.spec.ts +++ b/src/app/routes/api/v3/admin/forms/__tests__/admin-forms.settings.routes.spec.ts @@ -1,4 +1,5 @@ import { ObjectId } from 'bson-ext' +import { merge } from 'lodash' import mongoose from 'mongoose' import { errAsync } from 'neverthrow' import supertest, { Session } from 'supertest-session' @@ -85,11 +86,8 @@ describe('admin-form.settings.routes', () => { // Assert const expectedResponse = JSON.parse( - JSON.stringify({ - ...formToUpdate.getSettings(), - // Should get updated with new settings - ...settingsToUpdate, - }), + // Should get updated with new settings + JSON.stringify(merge(formToUpdate.getSettings(), settingsToUpdate)), ) expect(response.status).toEqual(200) expect(response.body).toEqual(expectedResponse) diff --git a/src/app/routes/api/v3/admin/forms/admin-forms.settings.routes.ts b/src/app/routes/api/v3/admin/forms/admin-forms.settings.routes.ts index 5768e55e89..799e784260 100644 --- a/src/app/routes/api/v3/admin/forms/admin-forms.settings.routes.ts +++ b/src/app/routes/api/v3/admin/forms/admin-forms.settings.routes.ts @@ -24,8 +24,9 @@ const updateSettingsValidator = celebrate({ submissionLimit: Joi.number().allow(null), title: Joi.string(), webhook: Joi.object({ - url: Joi.string().uri().required().allow(''), - }), + url: Joi.string().uri().allow(''), + isRetryEnabled: Joi.boolean(), + }).min(1), }).min(1), }) diff --git a/src/app/utils/random-uniform.ts b/src/app/utils/random-uniform.ts new file mode 100644 index 0000000000..d0e5dffbdc --- /dev/null +++ b/src/app/utils/random-uniform.ts @@ -0,0 +1,12 @@ +/** + * Generates a random integer between min and max (both inclusive). + * If min/max are not integers, the ceiling and floor are taken respectively. + * @param min + * @param max + * @returns Integer generated uniformly in interval + */ +export const randomUniformInt = (min: number, max: number): number => { + const roundedMin = Math.ceil(min) + const roundedMax = Math.floor(max) + return Math.floor(Math.random() * (roundedMax - roundedMin + 1)) + roundedMin +} diff --git a/src/public/modules/forms/admin/css/settings-form.css b/src/public/modules/forms/admin/css/settings-form.css index cdaa27b420..960d1b4429 100644 --- a/src/public/modules/forms/admin/css/settings-form.css +++ b/src/public/modules/forms/admin/css/settings-form.css @@ -171,6 +171,10 @@ padding-bottom: 45px; } +#settings-form .feature-container.webhook-feature-container { + padding-bottom: 30px; +} + #settings-form #enable-auth { padding-top: 45px; padding-bottom: 30px; diff --git a/src/public/modules/forms/admin/directiveViews/settings-form.client.view.html b/src/public/modules/forms/admin/directiveViews/settings-form.client.view.html index ee4eca1c62..5b6f67c24e 100644 --- a/src/public/modules/forms/admin/directiveViews/settings-form.client.view.html +++ b/src/public/modules/forms/admin/directiveViews/settings-form.client.view.html @@ -379,15 +379,6 @@ (optional) -
- - Webhook cannot be changed unless your form is deactivated. -
Webhook is not available for forms with attachment fields.
-
+
+
+ + Webhook URL cannot be changed unless your form is + deactivated. +
+
+
+
+ Enable retries +
+
+ +
+
+
+ + + Your system must meet certain requirements before retries can be + safely enabled. + Learn more + +
+
diff --git a/src/public/modules/forms/admin/directives/settings-form.client.directive.js b/src/public/modules/forms/admin/directives/settings-form.client.directive.js index dde36d6e08..88f4e260a3 100644 --- a/src/public/modules/forms/admin/directives/settings-form.client.directive.js +++ b/src/public/modules/forms/admin/directives/settings-form.client.directive.js @@ -14,6 +14,7 @@ const SETTINGS_PATH = [ 'inactiveMessage', 'submissionLimit', 'webhook.url', + 'webhook.isRetryEnabled', ] const createTempSettings = (myform) => { @@ -365,6 +366,18 @@ function settingsFormDirective( } } } + + $scope.isWebhookRetryToggleDisabled = () => { + // disable if there is no valid saved webhook URL + return !get($scope.myform, 'webhook.url') + } + + $scope.saveWebhookUrl = () => { + if (!get($scope, 'tempForm.webhook.url')) { + set($scope, 'tempForm.webhook.isRetryEnabled', false) + } + return $scope.saveForm() + } }, ], } diff --git a/src/public/translations/en-SG/main.json b/src/public/translations/en-SG/main.json index a8a8af9fc2..746d70d70f 100644 --- a/src/public/translations/en-SG/main.json +++ b/src/public/translations/en-SG/main.json @@ -21,6 +21,7 @@ "WHITELISTED_ATTACHMENT_TYPES": "https://go.gov.sg/formsg-cwl", "SINGPASS_ELIGIBILITY_FAQ": "https://www.ifaq.gov.sg/SINGPASS/apps/Fcd_faqmain.aspx#FAQ_2101385", "ESERVICE_ID_FAQ": "https://go.gov.sg/formsg-spcp", - "TERMS_THIRD_PARTY_LIST": "https://s3-ap-southeast-1.amazonaws.com/misc.form.gov.sg/OSS-Legal.pdf" + "TERMS_THIRD_PARTY_LIST": "https://s3-ap-southeast-1.amazonaws.com/misc.form.gov.sg/OSS-Legal.pdf", + "WEBHOOK_RETRIES": "https://go.gov.sg/form-webhook-retries" } } diff --git a/src/types/api/form.ts b/src/types/api/form.ts index 256e99d122..f904e4b844 100644 --- a/src/types/api/form.ts +++ b/src/types/api/form.ts @@ -1,5 +1,5 @@ import { LeanDocument } from 'mongoose' -import { ConditionalPick, Primitive } from 'type-fest' +import { ConditionalPick, PartialDeep, Primitive } from 'type-fest' import { FormField, @@ -22,7 +22,7 @@ import { SpcpSession } from '../spcp' import { EditFormFieldParams } from './field' -export type SettingsUpdateDto = Partial +export type SettingsUpdateDto = PartialDeep export type FieldUpdateDto = FormFieldWithId diff --git a/src/types/form.ts b/src/types/form.ts index 2c2ffa0829..013b067bbd 100644 --- a/src/types/form.ts +++ b/src/types/form.ts @@ -99,6 +99,7 @@ export type Permission = { export type Webhook = { url: string + isRetryEnabled: boolean } /** diff --git a/src/types/submission.ts b/src/types/submission.ts index d1940974d0..b1aaabee5c 100644 --- a/src/types/submission.ts +++ b/src/types/submission.ts @@ -57,6 +57,20 @@ export interface WebhookView { data: WebhookData } +export type SubmissionWebhookInfo = { + webhookUrl: string + isRetryEnabled: boolean + webhookView: WebhookView +} + +export interface IPopulatedWebhookSubmission + extends IEncryptedSubmissionSchema { + form: { + _id: IFormSchema['_id'] + webhook: IFormSchema['webhook'] + } +} + export interface ISubmissionSchema extends ISubmission, Document {} export type FindFormsWithSubsAboveResult = { @@ -192,6 +206,15 @@ export type IEncryptSubmissionModel = Model & submissionId: string, webhookResponse: IWebhookResponse, ): Promise + + /** + * Retrieves webhook-related info for a given submission. + * @param submissionId + * @returns Object containing webhook destination and data + */ + retrieveWebhookInfoById( + submissionId: string, + ): Promise } export interface IWebhookResponseSchema extends IWebhookResponse, Document {}