Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SFDP-152: Retry Synchronous Notify Failures #47

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion app/jobs/check-notify-status/get-notify-status.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import notifyClient from '../../clients/notify-client.js'

const getNotifyStatus = async (id) => {
const { data } = await notifyClient.getNotificationById(id)

return {
id: data.id,
status: data.status
Expand Down
49 changes: 45 additions & 4 deletions app/messages/inbound/comms-request/handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ import { validate } from '../../../schemas/validate.js'
import {
v3 as commsSchema
} from '../../../schemas/comms-request/index.js'
import { publishInvalidRequest, publishReceived } from '../../outbound/notification-status/publish.js'
import { publishInvalidRequest, publishReceived, publishStatus } from '../../outbound/notification-status/publish.js'
import { sendNotification } from './send-notification.js'
import notifyStatus from '../../../constants/notify-statuses.js'

const handleCommsRequest = async (message, receiver) => {
const commsRequest = message.body
Expand All @@ -21,14 +22,54 @@ const handleCommsRequest = async (message, receiver) => {
}

await receiver.deadLetterMessage(message)

return
}

await publishReceived(validated)
await sendNotification(validated)

await receiver.completeMessage(message)
try {
await sendNotification(validated, receiver)
await receiver.completeMessage(message)
} catch (error) {
if (error.message === 'NOTIFY_RETRY_ERROR') {
const deliveryCount = message.deliveryCount || 1

console.log(`Current delivery attempt: ${deliveryCount} of 10`)

// our logic here and the interaction with service bus needs to be tested and checked more thoroughly
if (deliveryCount === 8) { // if i change this to > 9 the message will be moved to the dead letter queue by azure service bus by default so need to understand how to handle this better
console.log('Maximum retry attempts reached')

try {
console.log('Publishing technical failure status to data layer...')
await publishStatus(
validated,
validated.data.commsAddresses,
notifyStatus.TECHNICAL_FAILURE,
{
error: 'Maximum retry attempts exceeded',
deliveryCount,
messageId: message.messageId
}
)
console.log('Status published successfully')

console.log('Moving message to dead letter queue...')
await receiver.deadLetterMessage(message)
console.log('Message moved to dead letter queue')
} catch (statusError) {
console.error('Error during max attempts handling:', statusError)
throw statusError
}
return
}

console.log(`Abandoning message for retry. Attempt ${deliveryCount} of 10`)
await receiver.abandonMessage(message)
return
}
throw error
}
} catch (error) {
console.error('Error handling message: ', error)
await receiver.abandonMessage(message)
Expand Down
80 changes: 56 additions & 24 deletions app/messages/inbound/comms-request/send-notification.js
Original file line number Diff line number Diff line change
@@ -1,52 +1,84 @@
import crypto from 'crypto'

import notifyClient from '../../../clients/notify-client.js'
// import crypto from 'crypto'
// import notifyClient from '../../../clients/notify-client.js'
import notifyStatus from '../../../constants/notify-statuses.js'

import { logCreatedNotification, logRejectedNotification } from '../../../repos/notification-log.js'
import { logCreatedNotification, logRejectedNotification, checkDuplicateNotification } from '../../../repos/notification-log.js'
import { publishStatus } from '../../outbound/notification-status/publish.js'

const trySendViaNotify = async (message, emailAddress) => {
// temporarily removed for testing purposes
// try {
// const response = await notifyClient.sendEmail(
// message.data.notifyTemplateId,
// emailAddress, {
// personalisation: message.data.personalisation,
// reference: crypto.randomUUID()
// }
// )

// return [response, null]
// } catch (error) {
// const statusCode = error.response?.data?.status_code
// console.error('Error sending email with code:', statusCode)
// return [null, error]
// }
// }

// Simulate a 500 error for testing purposes
try {
const response = await notifyClient.sendEmail(
message.data.notifyTemplateId,
emailAddress, {
personalisation: message.data.personalisation,
reference: crypto.randomUUID()
const error = new Error('Simulated 500 error')
error.response = {
status: 500,
data: {
errors: [
{
error: 'simulated-error'
}
]
}
)

return [response, null]
}
throw error
} catch (error) {
const status = error.response.data.status_code

console.error('Error sending email with code:', status)

const statusCode = error.response?.status
console.error('Error sending email with code:', statusCode)
return [null, error]
}
}

const sendNotification = async (message) => {
const sendNotification = async (message, receiver) => {
const emailAddresses = Array.isArray(message.data.commsAddresses)
? message.data.commsAddresses
: [message.data.commsAddresses]

for (const emailAddress of emailAddresses) {
const duplicate = await checkDuplicateNotification(message.id, emailAddress)

if (duplicate) {
console.warn('Duplicate notification detected')
continue
}

const [response, notifyError] = await trySendViaNotify(message, emailAddress)

try {
if (response) {
await publishStatus(message, emailAddress, notifyStatus.SENDING)
await logCreatedNotification(message, emailAddress, response.data.id)
} else {
const status = notifyStatus.INTERNAL_FAILURE
const notifyErrorData = notifyError.response.data

await publishStatus(message, emailAddress, status, notifyErrorData)
await logRejectedNotification(message, emailAddress, notifyError)
if (notifyError.response.status === 500) {
throw new Error('NOTIFY_RETRY_ERROR')
} else {
const status = notifyStatus.INTERNAL_FAILURE
const notifyErrorData = notifyError.response.data
await publishStatus(message, emailAddress, status, notifyErrorData)
await logRejectedNotification(message, emailAddress, notifyError)
}
}
} catch (error) {
console.error('Error logging notification: ', error)
if (error.message === 'NOTIFY_RETRY_ERROR') {
// Re-throw to be caught by handler
throw error
}
console.error('Error logging notification:', error)
}
}
}
Expand Down
20 changes: 19 additions & 1 deletion app/repos/notification-log.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import db from '../data/index.js'
import notifyStatus from '../constants/notify-statuses.js'

const nonFailureStatuses = [
notifyStatus.CREATED,
notifyStatus.SENDING,
notifyStatus.DELIVERED
]

const logCreatedNotification = async (message, recipient, notificationId) => {
await db.notifyApiRequestSuccess.create({
createdAt: new Date(),
Expand Down Expand Up @@ -54,9 +60,21 @@ const updateNotificationStatus = async (notificationId, status) => {
await notification.save()
}

const checkDuplicateNotification = async (messageId, recipient) => {
const existing = await db.notifyApiRequestSuccess.findOne({
where: {
'message.id': messageId,
recipient
}
})

return nonFailureStatuses.includes(existing?.status)
}

export {
logCreatedNotification,
logRejectedNotification,
getPendingNotifications,
updateNotificationStatus
updateNotificationStatus,
checkDuplicateNotification
}
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "fcp-fd-comms",
"version": "1.0.2",
"version": "1.0.3",
"description": "Common communications service for Single Front Door",
"homepage": "https://github.com/DEFRA/fcp-fd-comms",
"main": "app/index.js",
Expand Down
Loading