Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-03-11 02:12:21 -05:00
Code Issues Activity

Reverted "🌐 Updated Portal error handling to be i18n-friendly (#21176)" (#21184)

Browse source 
no ref
- this caused some troubles with error representation and changed
defaulting behavior

Going back to the drawing board on this one. I've been working on a
larger scale refactor so that this could be a hook, which feels much
more appropriate, though a much more substantial change.
This commit is contained in:
Steve Larson 2024-10-02 07:06:09 -05:00 • committed by GitHub
parent 4bd844fc2c
commit 0f7b1567a3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 36 additions and 145 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
apps/portal/src/actions.js
View file

@ -1,5 +1,5 @@
import setupGhostApi from './utils/api';
import {getMessageFromError} from './utils/errors';
import {HumanReadableError} 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: getMessageFromError(e, 'Failed to log in, please try again')
message: HumanReadableError.getMessageFromError(e, 'Failed to log in, please try again')
})
};
}

4
apps/portal/src/components/pages/FeedbackPage.js
View file

@ -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 {getMessageFromError} from '../../utils/errors';
import {HumanReadableError} 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 = getMessageFromError(e, t('There was a problem submitting your feedback. Please try again a little later.'));
const text = HumanReadableError.getMessageFromError(e, t('There was a problem submitting your feedback. Please try again a little later.'));
setError(text);
}
setLoading(false);

6
apps/portal/src/data-attributes.js
View file

@ -1,6 +1,6 @@
/* eslint-disable no-console */
import {getCheckoutSessionDataFromPlanAttribute, getUrlHistory} from './utils/helpers';
import {getErrorFromApiResponse, getMessageFromError} from './utils/errors';
import {HumanReadableError} 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 getErrorFromApiResponse(res).then((e) => {
return HumanReadableError.fromApiResponse(res).then((e) => {
throw e;
});
}
}).catch((err) => {
if (errorEl) {
// This theme supports a custom error element
errorEl.innerText = getMessageFromError(err, 'There was an error sending the email, please try again');
errorEl.innerText = HumanReadableError.getMessageFromError(err, 'There was an error sending the email, please try again');
}
form.classList.add('error');
});

93
apps/portal/src/tests/unit/errors.test.js
View file

@ -1,93 +0,0 @@
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(
<AppContext.Provider value={{t: tMock}}>
<GetMessage message="Hello" />
</AppContext.Provider>
);
expect(getByText('translated: Hello')).toBeInTheDocument();
expect(tMock).toHaveBeenCalledWith('Hello');
});
it('returns original message when t function is not available', () => {
const {getByText} = render(
<AppContext.Provider value={{}}>
<GetMessage message="Hello" />
</AppContext.Provider>
);
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(<AppContext.Provider value={{}}>{result}</AppContext.Provider>);
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(<AppContext.Provider value={{}}>{result}</AppContext.Provider>);
expect(getByText('Test error')).toBeInTheDocument();
});
it('returns GetMessage component with error for non-Error objects', () => {
const result = getMessageFromError('String error');
const {getByText} = render(<AppContext.Provider value={{}}>{result}</AppContext.Provider>);
expect(getByText('String error')).toBeInTheDocument();
});
});

6
apps/portal/src/utils/api.js
View file

@ -1,4 +1,4 @@
import {getErrorFromApiResponse} from './errors';
import {HumanReadableError} 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 getErrorFromApiResponse(res)) ?? new Error('Failed to save feedback');
throw (await HumanReadableError.fromApiResponse(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 getErrorFromApiResponse(res);
const humanError = await HumanReadableError.fromApiResponse(res);
if (humanError) {
throw humanError;
}

68
apps/portal/src/utils/errors.js
View file

@ -1,48 +1,32 @@
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<Error|undefined>} 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(<GetMessage message={json.errors[0].message} />);
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;
}
} catch (e) {
// Failed to decode: ignore
return undefined;
}
}
return undefined;
}
/**
* 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 <GetMessage message={defaultMessage} />;
} else if (error instanceof Error) {
return <GetMessage message={error.message} />;
/**
* 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;
}
return <GetMessage message={error} />;
}