From 49debef3da244e3b1e1163df477ac3f91dff98ea Mon Sep 17 00:00:00 2001 From: Steve Larson <9larsons@gmail.com> Date: Tue, 1 Oct 2024 16:19:09 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=8C=90=20Updated=20Portal=20error=20handl?= =?UTF-8?q?ing=20to=20be=20i18n-friendly=20(#21176)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit no ref - Portal was not set up in a way to allow for easy use of the i18n module for errors, as they weren't a React component - moved away from the class model to a functional component that could utilize React state (AppContext) I'm working on a different refactor that would convert more of Portal to hooks & functional components so that the codebase is more consistent and easier to read. This will have to work for the moment while that is being done, as that's no small task. --- apps/portal/src/actions.js | 4 +- .../src/components/pages/FeedbackPage.js | 4 +- apps/portal/src/data-attributes.js | 6 +- apps/portal/src/tests/unit/errors.test.js | 93 +++++++++++++++++++ apps/portal/src/utils/api.js | 6 +- apps/portal/src/utils/errors.js | 68 ++++++++------ 6 files changed, 145 insertions(+), 36 deletions(-) create mode 100644 apps/portal/src/tests/unit/errors.test.js diff --git a/apps/portal/src/actions.js b/apps/portal/src/actions.js index 96cc784d9a1..22e5d820264 100644 --- a/apps/portal/src/actions.js +++ b/apps/portal/src/actions.js @@ -1,5 +1,5 @@ import setupGhostApi from './utils/api'; -import {HumanReadableError} from './utils/errors'; +import {getMessageFromError} from './utils/errors'; import {createPopupNotification, getMemberEmail, getMemberName, getProductCadenceFromPrice, removePortalLinkFromUrl, getRefDomain} from './utils/helpers'; function switchPage({data, state}) { @@ -90,7 +90,7 @@ async function signin({data, api, state}) { action: 'signin:failed', popupNotification: createPopupNotification({ type: 'signin:failed', autoHide: false, closeable: true, state, status: 'error', - message: HumanReadableError.getMessageFromError(e, 'Failed to log in, please try again') + message: getMessageFromError(e, 'Failed to log in, please try again') }) }; } diff --git a/apps/portal/src/components/pages/FeedbackPage.js b/apps/portal/src/components/pages/FeedbackPage.js index 1720ac52d40..8cad18b0387 100644 --- a/apps/portal/src/components/pages/FeedbackPage.js +++ b/apps/portal/src/components/pages/FeedbackPage.js @@ -4,7 +4,7 @@ import {ReactComponent as ThumbDownIcon} from '../../images/icons/thumbs-down.sv import {ReactComponent as ThumbUpIcon} from '../../images/icons/thumbs-up.svg'; import {ReactComponent as ThumbErrorIcon} from '../../images/icons/thumbs-error.svg'; import setupGhostApi from '../../utils/api'; -import {HumanReadableError} from '../../utils/errors'; +import {getMessageFromError} from '../../utils/errors'; import ActionButton from '../common/ActionButton'; import CloseButton from '../common/CloseButton'; import LoadingPage from './LoadingPage'; @@ -317,7 +317,7 @@ export default function FeedbackPage() { await sendFeedback({siteUrl: site.url, uuid, key, postId, score: selectedScore}, api); setScore(selectedScore); } catch (e) { - const text = HumanReadableError.getMessageFromError(e, t('There was a problem submitting your feedback. Please try again a little later.')); + const text = getMessageFromError(e, t('There was a problem submitting your feedback. Please try again a little later.')); setError(text); } setLoading(false); diff --git a/apps/portal/src/data-attributes.js b/apps/portal/src/data-attributes.js index 9f4e9daefeb..3b7d700eb8c 100644 --- a/apps/portal/src/data-attributes.js +++ b/apps/portal/src/data-attributes.js @@ -1,6 +1,6 @@ /* eslint-disable no-console */ import {getCheckoutSessionDataFromPlanAttribute, getUrlHistory} from './utils/helpers'; -import {HumanReadableError} from './utils/errors'; +import {getErrorFromApiResponse, getMessageFromError} from './utils/errors'; export function formSubmitHandler({event, form, errorEl, siteUrl, submitHandler}) { form.removeEventListener('submit', submitHandler); @@ -77,14 +77,14 @@ export function formSubmitHandler({event, form, errorEl, siteUrl, submitHandler} if (res.ok) { form.classList.add('success'); } else { - return HumanReadableError.fromApiResponse(res).then((e) => { + return getErrorFromApiResponse(res).then((e) => { throw e; }); } }).catch((err) => { if (errorEl) { // This theme supports a custom error element - errorEl.innerText = HumanReadableError.getMessageFromError(err, 'There was an error sending the email, please try again'); + errorEl.innerText = getMessageFromError(err, 'There was an error sending the email, please try again'); } form.classList.add('error'); }); diff --git a/apps/portal/src/tests/unit/errors.test.js b/apps/portal/src/tests/unit/errors.test.js new file mode 100644 index 00000000000..7b707097be6 --- /dev/null +++ b/apps/portal/src/tests/unit/errors.test.js @@ -0,0 +1,93 @@ +import React from 'react'; +import {render} from '@testing-library/react'; +import AppContext from '../../AppContext'; +import {GetMessage, getErrorFromApiResponse, getMessageFromError} from '../../utils/errors'; + +// Mock AppContext +jest.mock('../../AppContext', () => ({ + __esModule: true, + default: React.createContext({t: jest.fn()}) +})); + +describe('GetMessage', () => { + it('returns translated message when t function is available', () => { + const tMock = jest.fn().mockImplementation(msg => `translated: ${msg}`); + const {getByText} = render( + + + + ); + expect(getByText('translated: Hello')).toBeInTheDocument(); + expect(tMock).toHaveBeenCalledWith('Hello'); + }); + + it('returns original message when t function is not available', () => { + const {getByText} = render( + + + + ); + expect(getByText('Hello')).toBeInTheDocument(); + }); +}); + +describe('getErrorFromApiResponse', () => { + // These tests are a little goofy because we're not testing the translation, we're testing that the function + // returns an Error with the correct message. We're not testing the message itself because that's handled in the + // getMessage function. And this doesn't run in react so the react component gets odd. + it('returns Error with translated message for 400 status', async () => { + const res = { + status: 400, + json: jest.fn().mockResolvedValue({errors: [{message: 'Bad Request'}]}) + }; + const error = await getErrorFromApiResponse(res); + expect(error).toBeInstanceOf(Error); + }); + + // These tests are a little goofy because we're not testing the translation, we're testing that the function + // returns an Error with the correct message. We're not testing the message itself because that's handled in the + // getMessage function. And this doesn't run in react so the react component gets odd. + it('returns Error with translated message for 429 status', async () => { + const res = { + status: 429, + json: jest.fn().mockResolvedValue({errors: [{message: 'Too Many Requests'}]}) + }; + const error = await getErrorFromApiResponse(res); + expect(error).toBeInstanceOf(Error); + }); + + it('returns undefined for other status codes', async () => { + const res = {status: 200}; + const error = await getErrorFromApiResponse(res); + expect(error).toBeUndefined(); + }); + + it('returns undefined when json parsing fails', async () => { + const res = { + status: 400, + json: jest.fn().mockRejectedValue(new Error('JSON parse error')) + }; + const error = await getErrorFromApiResponse(res); + expect(error).toBeUndefined(); + }); +}); + +describe('getMessageFromError', () => { + it('returns GetMessage component with default message when provided', () => { + const result = getMessageFromError(new Error('Test error'), 'Default message'); + const {getByText} = render({result}); + expect(getByText('Default message')).toBeInTheDocument(); + }); + + it('returns GetMessage component with error message for Error instance', () => { + const result = getMessageFromError(new Error('Test error')); + const {getByText} = render({result}); + expect(getByText('Test error')).toBeInTheDocument(); + }); + + it('returns GetMessage component with error for non-Error objects', () => { + const result = getMessageFromError('String error'); + const {getByText} = render({result}); + expect(getByText('String error')).toBeInTheDocument(); + }); +}); \ No newline at end of file diff --git a/apps/portal/src/utils/api.js b/apps/portal/src/utils/api.js index 9d8f32e082d..2c283cb3119 100644 --- a/apps/portal/src/utils/api.js +++ b/apps/portal/src/utils/api.js @@ -1,4 +1,4 @@ -import {HumanReadableError} from './errors'; +import {getErrorFromApiResponse} from './errors'; import {transformApiSiteData, transformApiTiersData, getUrlHistory} from './helpers'; function setupGhostApi({siteUrl = window.location.origin, apiUrl, apiKey}) { @@ -159,7 +159,7 @@ function setupGhostApi({siteUrl = window.location.origin, apiUrl, apiKey}) { if (res.ok) { return res.json(); } else { - throw (await HumanReadableError.fromApiResponse(res)) ?? new Error('Failed to save feedback'); + throw (await getErrorFromApiResponse(res)) ?? new Error('Failed to save feedback'); } } }; @@ -291,7 +291,7 @@ function setupGhostApi({siteUrl = window.location.origin, apiUrl, apiKey}) { return 'Success'; } else { // Try to read body error message that is human readable and should be shown to the user - const humanError = await HumanReadableError.fromApiResponse(res); + const humanError = await getErrorFromApiResponse(res); if (humanError) { throw humanError; } diff --git a/apps/portal/src/utils/errors.js b/apps/portal/src/utils/errors.js index 7f695160c33..e6d70fd12d4 100644 --- a/apps/portal/src/utils/errors.js +++ b/apps/portal/src/utils/errors.js @@ -1,32 +1,48 @@ -export class HumanReadableError extends Error { - /** - * Returns whether this response from the server is a human readable error and should be shown to the user. - * @param {Response} res - * @returns {HumanReadableError|undefined} - */ - static async fromApiResponse(res) { - // Bad request + Too many requests - if (res.status === 400 || res.status === 429) { - try { - const json = await res.json(); - if (json.errors && Array.isArray(json.errors) && json.errors.length > 0 && json.errors[0].message) { - return new HumanReadableError(json.errors[0].message); - } - } catch (e) { - // Failed to decode: ignore - return false; +import {useContext} from 'react'; +import AppContext from '../AppContext'; + +/** + * A component that gets the internationalized (i18n) message from the app context. + * @param {{message: string}} props - The props object containing the message to be translated. + * @returns {string} The translated message if AppContext.t is available, otherwise the original message. + */ +export const GetMessage = ({message}) => { + const {t} = useContext(AppContext); + return t ? t(message) : message; +}; + +/** + * Creates a human-readable error from an API response. + * @param {Response} res - The API response object. + * @returns {Promise} A promise that resolves to an Error if one can be created, or undefined otherwise. + */ +export async function getErrorFromApiResponse(res) { + // Bad request + Too many requests + if (res.status === 400 || res.status === 429) { + try { + const json = await res.json(); + if (json.errors && Array.isArray(json.errors) && json.errors.length > 0 && json.errors[0].message) { + return new Error(); } + } catch (e) { + // Failed to decode: ignore + return undefined; } } + return undefined; +} - /** - * Only output the message of an error if it is a human readable error and should be exposed to the user. - * Otherwise it returns a default generic message. - */ - static getMessageFromError(error, defaultMessage) { - if (error instanceof HumanReadableError) { - return error.message; - } - return defaultMessage; +/** + * Creates a human-readable error message from an error object. + * @param {Error} error - The error object. + * @param {string} defaultMessage - The default message to use if a human-readable message can't be extracted. + * @returns {React.ReactElement} A React element containing the human-readable error message. + */ +export function getMessageFromError(error, defaultMessage) { + if (defaultMessage) { + return ; + } else if (error instanceof Error) { + return ; } + return ; }