From d539342b6480fe649a9ea6fbf483147756a7a618 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Mon, 27 Apr 2026 09:43:07 +0100 Subject: [PATCH 1/7] Stash with retry --- .../src/routes/admin/dead-letter-queues.js | 61 +++++++++++++++---- 1 file changed, 49 insertions(+), 12 deletions(-) diff --git a/designer/server/src/routes/admin/dead-letter-queues.js b/designer/server/src/routes/admin/dead-letter-queues.js index 1f71b861f..f816622b8 100644 --- a/designer/server/src/routes/admin/dead-letter-queues.js +++ b/designer/server/src/routes/admin/dead-letter-queues.js @@ -30,6 +30,9 @@ const REDRIVE = 'redrive' const DELETE = 'delete' const CONFIRM = 'confirm' +const MAX_DLQ_READ_ATTEMPTS = 5 +const WAIT_TIME_IN_MILLIS = 1000 + const dlqSchema = Joi.string() .required() .valid(...Object.values(DeadLetterQueues)) @@ -118,17 +121,43 @@ export function validateMessageJson(messageJson) { } } +/** + * @param {DeadLetterQueues} dlq + * @param {string} token + * @param {string} messageId + * @returns {Promise} + */ +export async function readDlqWithRetry(dlq, token, messageId) { + let attempts = 0 + let foundMessage + while (attempts < MAX_DLQ_READ_ATTEMPTS) { + attempts++ + const messages = await getDeadLetterQueueMessages(dlq, token) + + foundMessage = messages.find((m) => m.json.MessageId === messageId) + if (foundMessage) { + break + } + + await new Promise((resolve) => + setTimeout(resolve, WAIT_TIME_IN_MILLIS) + ) + } + return foundMessage +} + /** * Keep specific properties - * @param {any[]} messages + * @param {any} message + * @returns {{ json: { MessageId: string, Body: any }}} */ -export function dlqMessageMapper(messages) { - return messages.map((m) => ({ +export function dlqMessageMapper(message) { + return { json: { - MessageId: m.MessageId, - Body: JSON.parse(m.Body) + MessageId: message.MessageId, + Body: JSON.parse(message.Body) } - })) + } } export function generateTitling() { @@ -451,11 +480,18 @@ export default [ .flash(sessionNames.validationFailure.deadLetterQueues) .at(0) - const messages = await getDeadLetterQueueMessages(dlq, token) + const foundMessage = await readDlqWithRetry(dlq, token, messageId) - const mappedMessage = dlqMessageMapper(messages).find( - (m) => m.json.MessageId === messageId - ) + if (!foundMessage) { + return redirectWithErrors( + request, + h, + error, + sessionNames.validationFailure.deadLetterQueues + ) + } + + const mappedMessage = dlqMessageMapper(foundMessage) const { formValues, formErrors } = validation ?? {} @@ -467,7 +503,7 @@ export default [ }, value: formValues?.messageJson ?? - JSON.stringify(mappedMessage?.json, null, 2), + JSON.stringify(mappedMessage.json, null, 2), rows: 30, errorMessage: formErrors?.messageJson ? { text: formErrors.messageJson.text } @@ -525,9 +561,10 @@ export default [ } const messages = await getDeadLetterQueueMessages(dlq, token) - const originalMessage = dlqMessageMapper(messages).find( + const message = messages.find( (m) => m.json.MessageId === messageId ) + const originalMessage = dlqMessageMapper(message) await resubmitDeadLetterQueueMessage(dlq, messageId, body, token) From 5ae463e5cac70d8bcef25091b356670b8a2845c5 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Mon, 27 Apr 2026 11:43:07 +0100 Subject: [PATCH 2/7] Handles message not found --- .../src/routes/admin/dead-letter-queues.js | 61 ++++++++++++++----- .../routes/admin/dead-letter-queues.test.js | 54 +++++++++++++++- 2 files changed, 97 insertions(+), 18 deletions(-) diff --git a/designer/server/src/routes/admin/dead-letter-queues.js b/designer/server/src/routes/admin/dead-letter-queues.js index f816622b8..21c0206c0 100644 --- a/designer/server/src/routes/admin/dead-letter-queues.js +++ b/designer/server/src/routes/admin/dead-letter-queues.js @@ -14,6 +14,7 @@ import { resubmitDeadLetterQueueMessage } from '~/src/lib/dead-letter-queue.js' import { createJoiError } from '~/src/lib/error-boom-helper.js' +import { addErrorsToSession } from '~/src/lib/error-helper.js' import { redirectWithErrors } from '~/src/lib/redirect-helper.js' import { publishDlqActionEvent } from '~/src/messaging/publish.js' import { @@ -134,14 +135,12 @@ export async function readDlqWithRetry(dlq, token, messageId) { attempts++ const messages = await getDeadLetterQueueMessages(dlq, token) - foundMessage = messages.find((m) => m.json.MessageId === messageId) + foundMessage = messages.find((m) => m.MessageId === messageId) if (foundMessage) { break } - await new Promise((resolve) => - setTimeout(resolve, WAIT_TIME_IN_MILLIS) - ) + await new Promise((resolve) => setTimeout(resolve, WAIT_TIME_IN_MILLIS)) } return foundMessage } @@ -160,6 +159,14 @@ export function dlqMessageMapper(message) { } } +/** + * @param {any[]} messages + * @returns {{ json: { MessageId: string, Body: any }}[]} + */ +export function dlqMessagesMapper(messages) { + return messages.map(dlqMessageMapper) +} + export function generateTitling() { const pageHeading = ADMIN_TOOLS @@ -312,9 +319,14 @@ export default [ const navigation = buildAdminNavigation(ADMIN_TOOLS) + // Validation errors + const validation = yar + .flash(sessionNames.validationFailure.deadLetterQueues) + .at(0) + const messages = await getDeadLetterQueueMessages(dlq, token) - const mappedMessages = dlqMessageMapper(messages) + const mappedMessages = dlqMessagesMapper(messages) // Saved banner const notification = /** @type {string[] | undefined} */ ( @@ -333,7 +345,8 @@ export default [ text: dlq }, messages: mappedMessages, - queueName: dlq + queueName: dlq, + errorList: buildErrorList(validation?.formErrors) }) }, options: { @@ -463,7 +476,7 @@ export default [ }), /** - * @satisfies {ServerRoute<{ Params: { dlq: DeadLetterQueues, messageId: string } }>} + * @satisfies {ServerRoute<{ Params: { dlq: DeadLetterQueues, messageId: string }, Payload: any }>} */ ({ method: 'GET', @@ -483,12 +496,18 @@ export default [ const foundMessage = await readDlqWithRetry(dlq, token, messageId) if (!foundMessage) { - return redirectWithErrors( + const error = createJoiError( + 'messageJson', + `Unable to find message ${messageId} in dead letter queue` + ) + addErrorsToSession( request, - h, - error, - sessionNames.validationFailure.deadLetterQueues + sessionNames.validationFailure.deadLetterQueues, + error ) + return h + .redirect(`${ROUTE_FULL_PATH}/${dlq}`) + .code(StatusCodes.SEE_OTHER) } const mappedMessage = dlqMessageMapper(foundMessage) @@ -560,11 +579,21 @@ export default [ ) } - const messages = await getDeadLetterQueueMessages(dlq, token) - const message = messages.find( - (m) => m.json.MessageId === messageId - ) - const originalMessage = dlqMessageMapper(message) + const foundMessage = await readDlqWithRetry(dlq, token, messageId) + if (!foundMessage) { + const error = createJoiError( + 'messageJson', + `Unable to find message ${messageId} in dead letter queue` + ) + return redirectWithErrors( + request, + h, + error, + sessionNames.validationFailure.deadLetterQueues + ) + } + + const originalMessage = dlqMessageMapper(foundMessage) await resubmitDeadLetterQueueMessage(dlq, messageId, body, token) diff --git a/designer/server/src/routes/admin/dead-letter-queues.test.js b/designer/server/src/routes/admin/dead-letter-queues.test.js index d88c6d4f6..c84d8d342 100644 --- a/designer/server/src/routes/admin/dead-letter-queues.test.js +++ b/designer/server/src/routes/admin/dead-letter-queues.test.js @@ -376,6 +376,23 @@ describe('Dead-letter queues routes', () => { expect(response.result).toMatchSnapshot() }) + test('should show errors when message no longer in queue', async () => { + jest.mocked(getDeadLetterQueueMessages).mockResolvedValue([]) + + const options = { + method: 'get', + url: '/admin/dead-letter-queues/audit-api/modify/message-id', + auth + } + + const { + response: { statusCode, headers } + } = await renderResponse(server, options) + + expect(statusCode).toBe(StatusCodes.SEE_OTHER) + expect(headers.location).toBe('/admin/dead-letter-queues/audit-api') + }, 10000) + test('should show errors if resubmit button pressed when invalid JSON', async () => { jest.mocked(resubmitDeadLetterQueueMessage).mockResolvedValue() @@ -399,9 +416,11 @@ describe('Dead-letter queues routes', () => { ) }) - test('should resubmit if resubmit button pressed when valid JSON', async () => { + test('should show errors if resubmit button pressed when message no longer in queue', async () => { jest.mocked(resubmitDeadLetterQueueMessage).mockResolvedValue() + jest.mocked(getDeadLetterQueueMessages).mockResolvedValue([]) + const options = { method: 'post', url: '/admin/dead-letter-queues/audit-api/modify/12345', @@ -415,10 +434,41 @@ describe('Dead-letter queues routes', () => { response: { statusCode, headers } } = await renderResponse(server, options) + expect(statusCode).toBe(StatusCodes.SEE_OTHER) + expect(resubmitDeadLetterQueueMessage).not.toHaveBeenCalled() + expect(headers.location).toBe( + '/admin/dead-letter-queues/audit-api/modify/12345' + ) + }, 10000) + + test('should resubmit if resubmit button pressed when valid JSON', async () => { + jest.mocked(resubmitDeadLetterQueueMessage).mockResolvedValue() + + jest.mocked(getDeadLetterQueueMessages).mockResolvedValue([ + { + MessageId: 'message-id', + Body: '{ "field1": "value1" }', + ReceiptHandle: 'rec-handle' + } + ]) + + const options = { + method: 'post', + url: '/admin/dead-letter-queues/audit-api/modify/message-id', + auth, + payload: { + messageJson: JSON.stringify(validJsonMessage) + } + } + + const { + response: { statusCode, headers } + } = await renderResponse(server, options) + expect(statusCode).toBe(StatusCodes.MOVED_TEMPORARILY) expect(resubmitDeadLetterQueueMessage).toHaveBeenCalledWith( 'audit-api', - '12345', + 'message-id', validJsonMessage.Body, expect.any(String) ) From 6573f350200570aed05969f1a110204b6db11941 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Mon, 27 Apr 2026 11:58:32 +0100 Subject: [PATCH 3/7] Sonar fixes --- .../routes/admin/dead-letter-queue-helper.js | 109 +++++++++++++++++ .../src/routes/admin/dead-letter-queues.js | 112 ++---------------- .../routes/admin/dead-letter-queues.test.js | 2 +- 3 files changed, 118 insertions(+), 105 deletions(-) create mode 100644 designer/server/src/routes/admin/dead-letter-queue-helper.js diff --git a/designer/server/src/routes/admin/dead-letter-queue-helper.js b/designer/server/src/routes/admin/dead-letter-queue-helper.js new file mode 100644 index 000000000..3aef4f4ed --- /dev/null +++ b/designer/server/src/routes/admin/dead-letter-queue-helper.js @@ -0,0 +1,109 @@ +import { formAdapterSubmissionMessagePayloadSchema } from '@defra/forms-engine-plugin/engine/types/schema.js' + +import { getDeadLetterQueueMessages } from '~/src/lib/dead-letter-queue.js' +import { createJoiError } from '~/src/lib/error-boom-helper.js' + +const MAX_DLQ_READ_ATTEMPTS = 5 +const WAIT_TIME_IN_MILLIS = 1000 + +/** + * @param {string} messageJson + */ +export function validateMessageJson(messageJson) { + let json + try { + json = JSON.parse(messageJson) + } catch (err) { + const typedError = /** @type {{ message?: string }} */ (err) + return { + error: createJoiError( + 'messageJson', + `Invalid JSON: ${typedError.message}` + ) + } + } + + /** + * @type { FormAdapterSubmissionMessagePayload | undefined } + */ + const messageBody = json.Body + if (!messageBody) { + return { + error: createJoiError( + 'messageJson', + 'Invalid JSON: Missing "Body" element' + ) + } + } + + const { error } = formAdapterSubmissionMessagePayloadSchema.validate( + messageBody, + { + abortEarly: false, + stripUnknown: true + } + ) + if (error) { + const errorText = error.details.map((d) => d.message).join(', ') + const joiError = createJoiError( + 'messageJson', + `JSON does not match the schema: ${errorText}` + ) + return { + error: joiError + } + } + return { + body: messageBody + } +} + +/** + * @param {DeadLetterQueues} dlq + * @param {string} token + * @param {string} messageId + * @returns {Promise} + */ +export async function readDlqWithRetry(dlq, token, messageId) { + let attempts = 0 + let foundMessage + while (attempts < MAX_DLQ_READ_ATTEMPTS) { + attempts++ + const messages = await getDeadLetterQueueMessages(dlq, token) + + foundMessage = messages.find((m) => m.MessageId === messageId) + if (foundMessage) { + break + } + + await new Promise((resolve) => setTimeout(resolve, WAIT_TIME_IN_MILLIS)) + } + return foundMessage +} + +/** + * Keep specific properties + * @param {any} message + * @returns {{ json: { MessageId: string, Body: any }}} + */ +export function dlqMessageMapper(message) { + return { + json: { + MessageId: message.MessageId, + Body: JSON.parse(message.Body) + } + } +} + +/** + * @param {any[]} messages + * @returns {{ json: { MessageId: string, Body: any }}[]} + */ +export function dlqMessagesMapper(messages) { + return messages.map(dlqMessageMapper) +} + +/** + * @import { DeadLetterQueues } from '@defra/forms-model' + * @import { FormAdapterSubmissionMessagePayload } from '@defra/forms-engine-plugin/engine/types.js' + */ diff --git a/designer/server/src/routes/admin/dead-letter-queues.js b/designer/server/src/routes/admin/dead-letter-queues.js index 21c0206c0..602780782 100644 --- a/designer/server/src/routes/admin/dead-letter-queues.js +++ b/designer/server/src/routes/admin/dead-letter-queues.js @@ -1,4 +1,3 @@ -import { formAdapterSubmissionMessagePayloadSchema } from '@defra/forms-engine-plugin/engine/types/schema.js' import { DeadLetterQueues, Scopes } from '@defra/forms-model' import { StatusCodes } from 'http-status-codes' import Joi from 'joi' @@ -21,6 +20,12 @@ import { deleteDeadLetterMessageConfirmationViewModel, redriveDeadLetterQueueConfirmationViewModel } from '~/src/models/manage/dead-letter-queue.js' +import { + dlqMessageMapper, + dlqMessagesMapper, + readDlqWithRetry, + validateMessageJson +} from '~/src/routes/admin/dead-letter-queue-helper.js' export const ROUTE_FULL_PATH = '/admin/dead-letter-queues' const CONFIRMATION_PAGE_PATH = 'forms/confirmation-page' @@ -31,9 +36,6 @@ const REDRIVE = 'redrive' const DELETE = 'delete' const CONFIRM = 'confirm' -const MAX_DLQ_READ_ATTEMPTS = 5 -const WAIT_TIME_IN_MILLIS = 1000 - const dlqSchema = Joi.string() .required() .valid(...Object.values(DeadLetterQueues)) @@ -70,103 +72,6 @@ const dlqActionPayloadSchema = Joi.object().keys({ messageId: messageIdSchema }) -/** - * @param {string} messageJson - */ -export function validateMessageJson(messageJson) { - let json - try { - json = JSON.parse(messageJson) - } catch (err) { - const typedError = /** @type {{ message?: string }} */ (err) - return { - error: createJoiError( - 'messageJson', - `Invalid JSON: ${typedError.message}` - ) - } - } - - /** - * @type { FormAdapterSubmissionMessagePayload | undefined } - */ - const messageBody = json.Body - if (!messageBody) { - return { - error: createJoiError( - 'messageJson', - 'Invalid JSON: Missing "Body" element' - ) - } - } - - const { error } = formAdapterSubmissionMessagePayloadSchema.validate( - messageBody, - { - abortEarly: false, - stripUnknown: true - } - ) - if (error) { - const errorText = error.details.map((d) => d.message).join(', ') - const joiError = createJoiError( - 'messageJson', - `JSON does not match the schema: ${errorText}` - ) - return { - error: joiError - } - } - return { - body: messageBody - } -} - -/** - * @param {DeadLetterQueues} dlq - * @param {string} token - * @param {string} messageId - * @returns {Promise} - */ -export async function readDlqWithRetry(dlq, token, messageId) { - let attempts = 0 - let foundMessage - while (attempts < MAX_DLQ_READ_ATTEMPTS) { - attempts++ - const messages = await getDeadLetterQueueMessages(dlq, token) - - foundMessage = messages.find((m) => m.MessageId === messageId) - if (foundMessage) { - break - } - - await new Promise((resolve) => setTimeout(resolve, WAIT_TIME_IN_MILLIS)) - } - return foundMessage -} - -/** - * Keep specific properties - * @param {any} message - * @returns {{ json: { MessageId: string, Body: any }}} - */ -export function dlqMessageMapper(message) { - return { - json: { - MessageId: message.MessageId, - Body: JSON.parse(message.Body) - } - } -} - -/** - * @param {any[]} messages - * @returns {{ json: { MessageId: string, Body: any }}[]} - */ -export function dlqMessagesMapper(messages) { - return messages.map(dlqMessageMapper) -} - export function generateTitling() { const pageHeading = ADMIN_TOOLS @@ -581,14 +486,14 @@ export default [ const foundMessage = await readDlqWithRetry(dlq, token, messageId) if (!foundMessage) { - const error = createJoiError( + const notFoundError = createJoiError( 'messageJson', `Unable to find message ${messageId} in dead letter queue` ) return redirectWithErrors( request, h, - error, + notFoundError, sessionNames.validationFailure.deadLetterQueues ) } @@ -634,5 +539,4 @@ export default [ /** * @import { ServerRoute } from '@hapi/hapi' - * @import { FormAdapterSubmissionMessagePayload } from '@defra/forms-engine-plugin/engine/types.js' */ diff --git a/designer/server/src/routes/admin/dead-letter-queues.test.js b/designer/server/src/routes/admin/dead-letter-queues.test.js index c84d8d342..1ff2769e4 100644 --- a/designer/server/src/routes/admin/dead-letter-queues.test.js +++ b/designer/server/src/routes/admin/dead-letter-queues.test.js @@ -8,7 +8,7 @@ import { redriveDeadLetterQueueMessages, resubmitDeadLetterQueueMessage } from '~/src/lib/dead-letter-queue.js' -import { validateMessageJson } from '~/src/routes/admin/dead-letter-queues.js' +import { validateMessageJson } from '~/src/routes/admin/dead-letter-queue-helper.js' import { authSuperAdmin as auth } from '~/test/fixtures/auth.js' import { renderResponse } from '~/test/helpers/component-helpers.js' From f9690a4f92b88db77718688cf0c54c09606d3b65 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Mon, 27 Apr 2026 15:21:52 +0100 Subject: [PATCH 4/7] Post message content between screens Co-authored-by: Copilot --- .../dead-letter-queues.test.js.snap | 15 ++-- .../src/routes/admin/dead-letter-queues.js | 69 ++++++++++++------- .../routes/admin/dead-letter-queues.test.js | 17 ----- designer/server/src/typings/hapi/index.d.ts | 1 + .../views/admin/dead-letter-queue-modify.njk | 2 +- .../views/admin/dead-letter-queue-view.njk | 6 +- 6 files changed, 58 insertions(+), 52 deletions(-) diff --git a/designer/server/src/routes/admin/__snapshots__/dead-letter-queues.test.js.snap b/designer/server/src/routes/admin/__snapshots__/dead-letter-queues.test.js.snap index 741ada13b..5e4b1a539 100644 --- a/designer/server/src/routes/admin/__snapshots__/dead-letter-queues.test.js.snap +++ b/designer/server/src/routes/admin/__snapshots__/dead-letter-queues.test.js.snap @@ -215,7 +215,11 @@ exports[`Dead-letter queues routes Journey should render form with messages and