Skip to content

excluding html from error description, using error header for logging #16331

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.

Sign up for GitHub

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

Open
mgosz-guardian wants to merge 8 commits into
base: main
Choose a base branch
from
+108 −34
Open

excluding html from error description, using error header for logging #16331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
bca2a56
excluding html from error description, using error header for logging
mgosz-guardian Jul 6, 2026
208a1a4
lint fix
mgosz-guardian Jul 6, 2026
0978765
Merge branch 'main' into email-endpoint-logs-improvements
mgosz-guardian Jul 6, 2026
9f6684c
Merge branch 'main' into email-endpoint-logs-improvements
mgosz-guardian Jul 6, 2026
2ec4c79
Merge branch 'main' into email-endpoint-logs-improvements
mgosz-guardian Jul 8, 2026
d47b1be
Merge branch 'main' into email-endpoint-logs-improvements
mgosz-guardian Jul 8, 2026
7aa934a
Merge branch 'main' into email-endpoint-logs-improvements
mgosz-guardian Jul 8, 2026
d8c88fc
Merge branch 'main' into email-endpoint-logs-improvements
mgosz-guardian Jul 8, 2026
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
11 changes: 5 additions & 6 deletions dotcom-rendering/src/components/SecureSignup.island.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
} from '../lib/newsletter-marketing-opt-in';
import {
getErrorType,
getResponseErrorDescription,
getResponseFailureDetails,
} from '../lib/newsletterSignupFailureDetails';
import {
EVENT_DESCRIPTION_TO_ACTION,
Expand Down Expand Up @@ -394,11 +394,10 @@ export const SecureSignup = ({

if (!response.ok) {
trackingDetails.responseStatus = response.status;
const errorDescription =
await getResponseErrorDescription(response);
if (errorDescription) {
trackingDetails.errorDescription = errorDescription;
}
Object.assign(
trackingDetails,
await getResponseFailureDetails(response),
);
}

sendTracking(
Expand Down
80 changes: 63 additions & 17 deletions dotcom-rendering/src/lib/newsletterSignupFailureDetails.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,47 +1,93 @@
import {
getErrorType,
getResponseErrorDescription,
getResponseFailureDetails,
} from './newsletterSignupFailureDetails';

const mockResponse = (text: string): Response =>
type MockHeaders = Partial<Record<string, string>>;

const mockHeaders = (headers: MockHeaders) => ({
get: jest.fn().mockImplementation((headerName: string) => {
return headers[headerName.toLowerCase()] ?? null;
}),
});

const mockResponse = (
text: string,
additionalHeaders: MockHeaders = {},
): Response =>
({
text: jest.fn().mockResolvedValue(text),
headers: mockHeaders(additionalHeaders),
}) as unknown as Response;

describe('newsletterSignupFailureDetails', () => {
describe('getResponseErrorDescription', () => {
describe('getResponseFailureDetails', () => {
it('returns undefined for an empty response body', async () => {
const response = mockResponse(' ');
const response = mockResponse(' ', {
'content-type': 'text/plain',
});

await expect(getResponseErrorDescription(response)).resolves.toBe(
undefined,
await expect(getResponseFailureDetails(response)).resolves.toEqual(
{},
);
});

it('trims and collapses whitespace in the response body', async () => {
const response = mockResponse(' too\n\n many\t spaces ');
const response = mockResponse(' too\n\n many\t spaces ', {
'content-type': 'text/plain',
});

await expect(getResponseErrorDescription(response)).resolves.toBe(
'too many spaces',
);
await expect(getResponseFailureDetails(response)).resolves.toEqual({
errorDescription: 'too many spaces',
});
});

it('truncates the response body to 60 characters', async () => {
const longMessage = 'a'.repeat(70);
const response = mockResponse(longMessage, {
'content-type': 'text/plain',
});

await expect(getResponseFailureDetails(response)).resolves.toEqual({
errorDescription: 'a'.repeat(60),
});
});

it('truncates the response body to 120 characters', async () => {
const longMessage = 'a'.repeat(130);
const response = mockResponse(longMessage);
it('returns the error code header when present', async () => {
const textMock = jest.fn().mockResolvedValue('ignored body');
const response = {
text: textMock,
headers: mockHeaders({
'content-type': 'text/plain',
'email-signup-error-code': 'already-subscribed',
}),
} as unknown as Response;

await expect(getResponseFailureDetails(response)).resolves.toEqual({
errorCode: 'already-subscribed',
});
expect(textMock).not.toHaveBeenCalled();
});

it('returns undefined for html error pages based on content type', async () => {
const response = mockResponse(
'<!DOCTYPE html><html><head><title>Error</title></head><body>Oops</body></html>',
{ 'content-type': 'text/html; charset=utf-8' },
);

await expect(getResponseErrorDescription(response)).resolves.toBe(
'a'.repeat(120),
await expect(getResponseFailureDetails(response)).resolves.toEqual(
{},
);
});

it('returns undefined when reading the response body throws', async () => {
const response = {
text: jest.fn().mockRejectedValue(new Error('read failed')),
headers: mockHeaders({ 'content-type': 'text/plain' }),
} as unknown as Response;

await expect(getResponseErrorDescription(response)).resolves.toBe(
undefined,
await expect(getResponseFailureDetails(response)).resolves.toEqual(
{},
);
});
});
Expand Down
40 changes: 35 additions & 5 deletions dotcom-rendering/src/lib/newsletterSignupFailureDetails.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,42 @@
export const getResponseErrorDescription = async (
const MAX_FAILURE_DETAIL_LENGTH = 60;
const ERROR_CODE_HEADER = 'Email-Signup-Error-Code';

export type NewsletterSignupFailureDetails = {
errorCode?: string;
errorDescription?: string;
};

export const getResponseFailureDetails = async (
response: Response,
): Promise<string | undefined> => {
): Promise<NewsletterSignupFailureDetails> => {
try {
const errorCode = response.headers
.get(ERROR_CODE_HEADER)
?.trim()
.substring(0, MAX_FAILURE_DETAIL_LENGTH);

if (errorCode !== undefined && errorCode !== '') {
return { errorCode };
}

const contentType = response.headers.get('content-type')?.toLowerCase();

if (contentType?.includes('text/html') === true) {
return {};
}

const responseText = (await response.text()).trim();
if (!responseText) return;
return responseText.replace(/\s+/g, ' ').substring(0, 120);
if (!responseText) return {};

const normalisedResponseText = responseText.replace(/\s+/g, ' ');
return {
errorDescription: normalisedResponseText.substring(
0,
MAX_FAILURE_DETAIL_LENGTH,
),
};
} catch {
return;
return {};
}
};

Expand Down
11 changes: 5 additions & 6 deletions dotcom-rendering/src/lib/useNewsletterSignupForm.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
} from './newsletter-marketing-opt-in';
import {
getErrorType,
getResponseErrorDescription,
getResponseFailureDetails,
} from './newsletterSignupFailureDetails';
import {
EVENT_DESCRIPTION_TO_ACTION,
Expand Down Expand Up @@ -379,11 +379,10 @@ export const useNewsletterSignupForm = (

if (!response.ok) {
trackingDetails.responseStatus = response.status;
const errorDescription =
await getResponseErrorDescription(response);
if (errorDescription) {
trackingDetails.errorDescription = errorDescription;
}
Object.assign(
trackingDetails,
await getResponseFailureDetails(response),
);
}

sendTracking(
Expand Down
Loading