From cb9b3db81fdeefe792cd2ca87f38c106a5641d79 Mon Sep 17 00:00:00 2001 From: Thomas Nabord Date: Tue, 21 Jan 2025 13:50:30 +0000 Subject: [PATCH 01/11] Add error management on front end --- _dev/src/ts/api/RequestHandler.ts | 31 ++--- _dev/src/ts/api/baseApi.ts | 10 +- _dev/src/ts/api/requestInterceptor.ts | 13 ++ _dev/src/ts/api/responseInterceptor.ts | 35 +++++ _dev/src/ts/pages/ErrorPage.ts | 122 ++++++++++++++++-- _dev/src/ts/pages/HomePage.ts | 18 +++ _dev/src/ts/pages/error/ErrorPage404.ts | 71 ---------- _dev/src/ts/types/apiTypes.ts | 12 ++ _dev/src/ts/utils/Hydration.ts | 12 +- classes/AjaxResponse.php | 1 + classes/AjaxResponseBuilder.php | 2 + classes/Router/Routes.php | 6 + .../self-managed/AbstractPageController.php | 3 + .../admin/self-managed/Error404Controller.php | 3 - .../self-managed/ErrorGeneratorController.php | 53 ++++++++ views/templates/layouts/error.html.twig | 16 ++- views/templates/layouts/layout.html.twig | 4 + views/templates/pages/errors/404.html.twig | 19 --- views/templates/pages/errors/500.html.twig | 41 ------ .../templates/pages/errors/generic.html.twig | 21 +++ views/templates/pages/home.html.twig | 5 + 21 files changed, 334 insertions(+), 164 deletions(-) create mode 100644 _dev/src/ts/api/requestInterceptor.ts create mode 100644 _dev/src/ts/api/responseInterceptor.ts delete mode 100644 _dev/src/ts/pages/error/ErrorPage404.ts create mode 100644 controllers/admin/self-managed/ErrorGeneratorController.php delete mode 100644 views/templates/pages/errors/500.html.twig create mode 100644 views/templates/pages/errors/generic.html.twig diff --git a/_dev/src/ts/api/RequestHandler.ts b/_dev/src/ts/api/RequestHandler.ts index 0f1bd7af6..a4e93682f 100644 --- a/_dev/src/ts/api/RequestHandler.ts +++ b/_dev/src/ts/api/RequestHandler.ts @@ -41,7 +41,7 @@ export class RequestHandler { */ public async post( route: string, - data: FormData = new FormData(), + data?: FormData, fromPopState?: boolean ): Promise { this.abortCurrentPost(); @@ -50,9 +50,6 @@ export class RequestHandler { this.#currentRequestAbortController = new AbortController(); const { signal } = this.#currentRequestAbortController; - // Append admin dir required by backend - data.append('dir', window.AutoUpgradeVariables.admin_dir); - try { const response = await baseApi.post('', data, { params: { route }, @@ -62,16 +59,8 @@ export class RequestHandler { const responseData = response.data; await this.#handleResponse(responseData, fromPopState); } catch (error) { - // A couple or errors are returned in an actual response (i.e 404 or 500) - if (error instanceof AxiosError) { - if (error.response?.data) { - const responseData = error.response.data; - responseData.new_route = 'error-page'; - await this.#handleResponse(responseData, true); - } - } else { - // TODO: catch errors - console.error(error); + if (error) { + await this.#handleError(error as AxiosError); } } } @@ -85,13 +74,11 @@ export class RequestHandler { */ public async postAction(action: string): Promise { const data = new FormData(); - - data.append('dir', window.AutoUpgradeVariables.admin_dir); data.append('action', action); try { - const response = await baseApi.post('', data); - return response.data as ApiResponseAction; + const response = await baseApi.post('', data); + return response.data; } catch (error: unknown) { if (error instanceof AxiosError && error?.response?.data?.error) { return error.response.data as ApiResponseAction; @@ -116,6 +103,14 @@ export class RequestHandler { new Hydration().hydrate(response, fromPopState); } } + + async #handleError(error: AxiosError): Promise { + new Hydration().hydrateError({ + code: error.status, + type: error.code, + additionalContents: error.response?.data, + }); + } } const api = new RequestHandler(); diff --git a/_dev/src/ts/api/baseApi.ts b/_dev/src/ts/api/baseApi.ts index 835c8c47d..447c34fda 100644 --- a/_dev/src/ts/api/baseApi.ts +++ b/_dev/src/ts/api/baseApi.ts @@ -17,13 +17,21 @@ * @license https://opensource.org/licenses/AFL-3.0 Academic Free License version 3.0 */ import axios from 'axios'; +import { addRequestInterceptor } from './requestInterceptor'; +import { addResponseInterceptor } from './responseInterceptor'; const baseApi = axios.create({ baseURL: `${window.AutoUpgradeVariables.admin_url}/autoupgrade/ajax-upgradetab.php`, headers: { 'X-Requested-With': 'XMLHttpRequest', Authorization: `Bearer ${() => window.AutoUpgradeVariables.token}` - } + }, + transitional: { + clarifyTimeoutError: true, + }, }); +addRequestInterceptor(baseApi); +addResponseInterceptor(baseApi); + export default baseApi; diff --git a/_dev/src/ts/api/requestInterceptor.ts b/_dev/src/ts/api/requestInterceptor.ts new file mode 100644 index 000000000..f8074bb71 --- /dev/null +++ b/_dev/src/ts/api/requestInterceptor.ts @@ -0,0 +1,13 @@ +import { AxiosInstance, InternalAxiosRequestConfig } from "axios"; + +const requestFulfilledInterceptor = (config: InternalAxiosRequestConfig) => { + if (!config.data) { + config.data = new FormData(); + } + config.data?.append('dir', window.AutoUpgradeVariables.admin_dir); + return config; + }; + +export const addRequestInterceptor = (axios: AxiosInstance): void => { + axios.interceptors.request.use(requestFulfilledInterceptor); +} \ No newline at end of file diff --git a/_dev/src/ts/api/responseInterceptor.ts b/_dev/src/ts/api/responseInterceptor.ts new file mode 100644 index 000000000..24e6e01fb --- /dev/null +++ b/_dev/src/ts/api/responseInterceptor.ts @@ -0,0 +1,35 @@ +import { AxiosError, AxiosInstance, AxiosResponse } from "axios"; +import { APP_ERR_RESPONSE_BAD_TYPE, APP_ERR_RESPONSE_INVALID } from "../types/apiTypes"; + +const responseFulfilledInterceptor = (response: AxiosResponse) => { + console.log('Checking response', response); + + // All responses must be a parsed JSON. If we get another type of response, + // this means something went wrong, i.e Another software answered. + if (Object.prototype.toString.call(response.data) !== '[object Object]') { + throw new AxiosError('The response does not have a valid type', APP_ERR_RESPONSE_BAD_TYPE, response.config, response.request, response); + } + + // Make sure the response contains the expected data + if (!response.data.kind) { + throw new AxiosError('The response contents is invalid', APP_ERR_RESPONSE_INVALID, response.config, response.request, response); + } + + return response; +}; + +const responseErroredInterceptor = (error: any) => { + const errorSilenced = [AxiosError.ERR_CANCELED]; + // Ignore some errors + if (error instanceof AxiosError) { + if (error.code && errorSilenced.includes(error.code)) { + return Promise.reject(null); + } + } + + return Promise.reject(error); +}; + +export const addResponseInterceptor = (axios: AxiosInstance): void => { + axios.interceptors.response.use(responseFulfilledInterceptor, responseErroredInterceptor); +} diff --git a/_dev/src/ts/pages/ErrorPage.ts b/_dev/src/ts/pages/ErrorPage.ts index d5bf7f784..1b0fd049e 100644 --- a/_dev/src/ts/pages/ErrorPage.ts +++ b/_dev/src/ts/pages/ErrorPage.ts @@ -16,26 +16,132 @@ * @copyright Since 2007 PrestaShop SA and Contributors * @license https://opensource.org/licenses/AFL-3.0 Academic Free License version 3.0 */ -import DomLifecycle from '../types/DomLifecycle'; -import ErrorPage404 from './error/ErrorPage404'; +import api from '../api/RequestHandler'; +import { ApiError } from '../types/apiTypes'; +import Hydration from '../utils/Hydration'; import PageAbstract from './PageAbstract'; export default class ErrorPage extends PageAbstract { - errorPage?: DomLifecycle; + public static templateId: string = 'error-page-template'; + // TODO: Improve this by putting the target in the template and sent it from the back end + public static targetElementIdToUpdate: string = 'ua_page'; + + isOnHomePage: boolean = false; constructor() { super(); - if (document.getElementById('ua_error_404')) { - this.errorPage = new ErrorPage404(); - } + this.isOnHomePage = new URLSearchParams(window.location.search).get('route') === 'home-page'; } public mount = (): void => { - this.errorPage?.mount(); + + // If the error page is already present on the DOM (For instance on a whole page refresh), + // initalize it at once instead of waiting for an event. + const errorPageFromBackEnd = document.querySelector('.error-page'); + if (errorPageFromBackEnd) { + this.#mountErrorPage(errorPageFromBackEnd); + } else { + this.#errorTemplateElement.addEventListener(Hydration.hydrationEventName, this.#onError.bind(this), {once: true}); + } }; public beforeDestroy = (): void => { - this.errorPage?.beforeDestroy(); + this.#errorTemplateElement.removeEventListener(Hydration.hydrationEventName, this.#onError.bind(this)); + }; + + get #errorTemplateElement(): HTMLTemplateElement { + const element = document.getElementById(ErrorPage.templateId); + + if (!element) { + throw new Error('Error template not found'); + } + + return element as HTMLTemplateElement; + } + + #onError = async (event: CustomEvent): Promise => { + this.#createErrorPage(event); + } + + #createErrorPage(event: CustomEvent): void { + // Duplicate the error template before alteration + const errorElement = this.#errorTemplateElement.content.cloneNode(true) as DocumentFragment; + + // Set the id of the cloned element + const errorChild = errorElement.getElementById('ua_error_placeholder'); + if (errorChild) { + errorChild.id = `ua_error_${event.detail.type}`; + } + + // If code is a HTTP error number (i.e 404, 500 etc.), let's change the text in the left column with it. + if (typeof event.detail.code === 'number' && event.detail.code >= 300 && event.detail.code.toString().length === 3) { + const strigifiedCode = event.detail.code.toString().replaceAll('0', 'O'); + const errorCodeSlotElements = errorElement.querySelectorAll('.error-page__code-char'); + errorCodeSlotElements.forEach((element: Element, index: number) => { + element.innerHTML = strigifiedCode[index]; + }); + errorElement.querySelector('.error-page__code-missing')?.classList.add('hidden'); + } + + // Display a user friendly text related to the code if it exists, otherwise write the error code. + const errorDescriptionElement = errorElement.querySelector('.error-page__desc'); + const userFriendlyDescriptionElement = errorDescriptionElement?.querySelector(`.error-page__desc-${event.detail.code || event.detail.type}`); + if (userFriendlyDescriptionElement) { + userFriendlyDescriptionElement.classList.remove('hidden'); + } else if (errorDescriptionElement && event.detail.type) { + errorDescriptionElement.innerHTML = event.detail.type; + } + + // Store the contents in the hidden field so it can be used in the error reporting modal + const additionalContentsElement = errorElement.querySelector('.error-page__contents'); + if (additionalContentsElement && event.detail.additionalContents) { + additionalContentsElement.innerHTML = new String(event.detail.additionalContents).toString(); + } + + // Finally, append the result on the page + const targetElementToUpdate = document.getElementById(ErrorPage.targetElementIdToUpdate); + if (!targetElementToUpdate) { + throw new Error('Target element cannot be found'); + } + targetElementToUpdate.replaceChildren(errorElement); + + // Enable events and page features + this.#mountErrorPage(document.querySelector('.error-page')!); + } + + #mountErrorPage(errorPage: Element): void { + console.log('mounting', errorPage); + this.#form.addEventListener('submit', this.#onSubmit, {once: true}); + + // Display the proper action buttons + const activeButtonElement = this.isOnHomePage + ? errorPage.querySelector('.error-page__exit-button') + : errorPage.querySelector('.error-page__home-page-form'); + + if (activeButtonElement) { + activeButtonElement.classList.remove('hidden'); + } + } + + get #form(): HTMLFormElement { + const form = document.forms.namedItem('home-page-form'); + if (!form) { + throw new Error('Form not found'); + } + + ['routeToSubmit'].forEach((data) => { + if (!form.dataset[data]) { + throw new Error(`Missing data ${data} from form dataset.`); + } + }); + + return form; + } + + readonly #onSubmit = async (event: Event): Promise => { + event.preventDefault(); + + await api.post(this.#form.dataset.routeToSubmit!, new FormData(this.#form)); }; } diff --git a/_dev/src/ts/pages/HomePage.ts b/_dev/src/ts/pages/HomePage.ts index 783df6277..7387d89f4 100644 --- a/_dev/src/ts/pages/HomePage.ts +++ b/_dev/src/ts/pages/HomePage.ts @@ -37,6 +37,8 @@ export default class HomePage extends PageAbstract { this.checkForm(); this.form.addEventListener('change', this.checkForm); this.form.addEventListener('submit', this.handleSubmit); + + document.getElementById('update_assistant')?.addEventListener('click', this.#onClick); } }; @@ -45,6 +47,22 @@ export default class HomePage extends PageAbstract { this.form.removeEventListener('change', this.checkForm); this.form.removeEventListener('submit', this.handleSubmit); } + document.getElementById('update_assistant')?.removeEventListener('click', this.#onClick); + }; + + readonly #onClick = async (ev: Event): Promise => { + if ((ev.target as HTMLElement).id === 'trigger-1') { + await api.post('fake-error-500'); + } + if ((ev.target as HTMLElement).id === 'trigger-2') { + await api.post('fake-error-502'); + } + if ((ev.target as HTMLElement).id === 'trigger-3') { + await api.post('fake-invalid-response'); + } + if ((ev.target as HTMLElement).id === 'trigger-4') { + await api.post('fake-timeout'); + } }; private checkForm = () => { diff --git a/_dev/src/ts/pages/error/ErrorPage404.ts b/_dev/src/ts/pages/error/ErrorPage404.ts deleted file mode 100644 index a89c850c2..000000000 --- a/_dev/src/ts/pages/error/ErrorPage404.ts +++ /dev/null @@ -1,71 +0,0 @@ -/** - * Copyright since 2007 PrestaShop SA and Contributors - * PrestaShop is an International Registered Trademark & Property of PrestaShop SA - * - * NOTICE OF LICENSE - * - * This source file is subject to the Academic Free License version 3.0 - * that is bundled with this package in the file LICENSE.md. - * It is also available through the world-wide-web at this URL: - * https://opensource.org/licenses/AFL-3.0 - * If you did not receive a copy of the license and are unable to - * obtain it through the world-wide-web, please send an email - * to license@prestashop.com so we can send you a copy immediately. - * - * @author PrestaShop SA and Contributors - * @copyright Since 2007 PrestaShop SA and Contributors - * @license https://opensource.org/licenses/AFL-3.0 Academic Free License version 3.0 - */ -import api from '../../api/RequestHandler'; -import DomLifecycle from '../../types/DomLifecycle'; - -export default class ErrorPage404 implements DomLifecycle { - isOnHomePage: boolean = false; - - public constructor() { - this.isOnHomePage = new URLSearchParams(window.location.search).get('route') === 'home-page'; - } - - public mount = (): void => { - this.#activeActionButton.classList.remove('hidden'); - this.#form.addEventListener('submit', this.#onSubmit); - }; - - public beforeDestroy = (): void => { - this.#form.removeEventListener('submit', this.#onSubmit); - }; - - get #activeActionButton(): HTMLFormElement | HTMLAnchorElement { - return this.isOnHomePage ? this.#exitButton : this.#form; - } - - get #form(): HTMLFormElement { - const form = document.forms.namedItem('home-page-form'); - if (!form) { - throw new Error('Form not found'); - } - - ['routeToSubmit'].forEach((data) => { - if (!form.dataset[data]) { - throw new Error(`Missing data ${data} from form dataset.`); - } - }); - - return form; - } - - get #exitButton(): HTMLAnchorElement { - const link = document.getElementById('exit-button'); - - if (!link || !(link instanceof HTMLAnchorElement)) { - throw new Error('Link is not found or invalid'); - } - return link; - } - - readonly #onSubmit = async (event: Event): Promise => { - event.preventDefault(); - - await api.post(this.#form.dataset.routeToSubmit!, new FormData(this.#form)); - }; -} diff --git a/_dev/src/ts/types/apiTypes.ts b/_dev/src/ts/types/apiTypes.ts index f333902ab..5049ac46e 100644 --- a/_dev/src/ts/types/apiTypes.ts +++ b/_dev/src/ts/types/apiTypes.ts @@ -17,6 +17,7 @@ * @license https://opensource.org/licenses/AFL-3.0 Academic Free License version 3.0 */ interface ApiResponseHydration { + kind: 'hydrate', hydration: boolean; new_content: string; new_route?: string; @@ -25,10 +26,12 @@ interface ApiResponseHydration { } interface ApiResponseNextRoute { + kind: 'next', next_route: string; } interface ApiResponseAction { + kind: 'action', error: null | boolean; stepDone: null | boolean; next: string; @@ -42,6 +45,15 @@ interface ApiResponseAction { }; } +export interface ApiError { + code?: number, + type?: string, + additionalContents?: string|object +} + type ApiResponse = ApiResponseHydration | ApiResponseNextRoute | ApiResponseAction; +export const APP_ERR_RESPONSE_BAD_TYPE = 'APP_ERR_RESPONSE_BAD_TYPE'; +export const APP_ERR_RESPONSE_INVALID = 'APP_ERR_RESPONSE_INVALID'; + export type { ApiResponseHydration, ApiResponseNextRoute, ApiResponseAction, ApiResponse }; diff --git a/_dev/src/ts/utils/Hydration.ts b/_dev/src/ts/utils/Hydration.ts index 9145fbe2f..8b404a80d 100644 --- a/_dev/src/ts/utils/Hydration.ts +++ b/_dev/src/ts/utils/Hydration.ts @@ -16,9 +16,11 @@ * @copyright Since 2007 PrestaShop SA and Contributors * @license https://opensource.org/licenses/AFL-3.0 Academic Free License version 3.0 */ -import { ApiResponseHydration } from '../types/apiTypes'; +import { ApiError, ApiResponseHydration } from '../types/apiTypes'; import { dialogContainer, routeHandler, scriptHandler } from '../autoUpgrade'; import { ScriptType } from '../types/scriptHandlerTypes'; +import { AxiosError } from 'axios'; +import ErrorPage from '../pages/ErrorPage'; export default class Hydration { /** @@ -71,4 +73,12 @@ export default class Hydration { elementToUpdate.dispatchEvent(this.hydrationEvent); } } + + public hydrateError(error: ApiError): void { + scriptHandler.unloadScriptType(ScriptType.PAGE); + scriptHandler.loadScript('error-page'); + + const elementToUpdate = document.getElementById(ErrorPage.templateId); + elementToUpdate?.dispatchEvent(new CustomEvent(Hydration.hydrationEventName, {detail: error})); + } } diff --git a/classes/AjaxResponse.php b/classes/AjaxResponse.php index 930089f75..ad9de98c8 100644 --- a/classes/AjaxResponse.php +++ b/classes/AjaxResponse.php @@ -89,6 +89,7 @@ public function __construct(AbstractState $state, Logger $logger) public function getResponse(): array { return [ + 'kind' => 'action', 'error' => $this->error, 'stepDone' => $this->stepDone, 'next' => $this->next, diff --git a/classes/AjaxResponseBuilder.php b/classes/AjaxResponseBuilder.php index 2820cf509..1431b757e 100644 --- a/classes/AjaxResponseBuilder.php +++ b/classes/AjaxResponseBuilder.php @@ -30,6 +30,7 @@ class AjaxResponseBuilder public static function hydrationResponse(string $parentToUpdate, string $newContent, ?array $options = []): JsonResponse { $arrayToReturn = [ + 'kind' => 'hydrate', 'hydration' => true, 'parent_to_update' => $parentToUpdate, 'new_content' => $newContent, @@ -49,6 +50,7 @@ public static function hydrationResponse(string $parentToUpdate, string $newCont public static function nextRouteResponse(string $nextRoute): JsonResponse { return new JsonResponse([ + 'kind' => 'next', 'next_route' => $nextRoute, ]); } diff --git a/classes/Router/Routes.php b/classes/Router/Routes.php index 1c4dc02ab..b9f512987 100644 --- a/classes/Router/Routes.php +++ b/classes/Router/Routes.php @@ -88,4 +88,10 @@ class Routes /* errors */ const ERROR_404 = 'error-404'; + + const FAKE_ERROR_500 = 'fake-error-500'; + const FAKE_ERROR_502 = 'fake-error-502'; + const FAKE_INVALID_RESPONSE = 'fake-invalid-response'; + const FAKE_TIMEOUT = 'fake-timeout'; + } diff --git a/controllers/admin/self-managed/AbstractPageController.php b/controllers/admin/self-managed/AbstractPageController.php index f56398f81..ce5e7943f 100644 --- a/controllers/admin/self-managed/AbstractPageController.php +++ b/controllers/admin/self-managed/AbstractPageController.php @@ -62,6 +62,9 @@ public function renderPage(string $page, array $params): string 'page' => $page, 'ps_version' => $this->getPsVersionClass(), 'data_transparency_link' => DocumentationLinks::PRESTASHOP_PROJECT_DATA_TRANSPARENCY_URL, + + 'exit_to_shop_admin' => $this->upgradeContainer->getUrlGenerator()->getShopAdminAbsolutePathFromRequest($this->request), + 'exit_to_app_home' => Routes::HOME_PAGE, ], $pageSelectors::getAllSelectors(), $params diff --git a/controllers/admin/self-managed/Error404Controller.php b/controllers/admin/self-managed/Error404Controller.php index a7e87ef88..27b0db107 100644 --- a/controllers/admin/self-managed/Error404Controller.php +++ b/controllers/admin/self-managed/Error404Controller.php @@ -51,9 +51,6 @@ protected function getParams(): array 'assets_base_path' => $this->upgradeContainer->getAssetsEnvironment()->getAssetsBaseUrl($this->request), 'error_code' => Response::HTTP_NOT_FOUND, - - 'exit_to_shop_admin' => $this->upgradeContainer->getUrlGenerator()->getShopAdminAbsolutePathFromRequest($this->request), - 'exit_to_app_home' => Routes::HOME_PAGE, ]; } } diff --git a/controllers/admin/self-managed/ErrorGeneratorController.php b/controllers/admin/self-managed/ErrorGeneratorController.php new file mode 100644 index 000000000..fc60779c9 --- /dev/null +++ b/controllers/admin/self-managed/ErrorGeneratorController.php @@ -0,0 +1,53 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/AFL-3.0 Academic Free License 3.0 (AFL-3.0) + */ + +namespace PrestaShop\Module\AutoUpgrade\Controller; + +use Symfony\Component\HttpFoundation\Response; + +class ErrorGeneratorController extends AbstractGlobalController +{ + public function generate500(): Response + { + return new Response('Internal Server Error', 500); + } + + public function generate502(): Response + { + return new Response('Bad gateway', 502); + } + + public function generateBadResponse(): Response + { + return new Response('[CONTENTS] Not a valid JSON, only a basic string'); + } + + public function generateTimeout(): never + { + sleep(60); + } +} \ No newline at end of file diff --git a/views/templates/layouts/error.html.twig b/views/templates/layouts/error.html.twig index 0aecdbdcc..63e15d0be 100644 --- a/views/templates/layouts/error.html.twig +++ b/views/templates/layouts/error.html.twig @@ -29,7 +29,9 @@
{% block title %} - {# Default title can go here, or it can be left empty so that children can override it #} +

+ {{ 'Something went wrong...'|trans({}) }} +

{% endblock %} {% block description %} @@ -38,7 +40,17 @@
{% block button %} - {# Default button can go here, or it can be left empty so that children can override it #} + {# Default buttons go here, and children can override them #} + + + {% endblock %}
diff --git a/views/templates/layouts/layout.html.twig b/views/templates/layouts/layout.html.twig index fae22242b..6254e4302 100644 --- a/views/templates/layouts/layout.html.twig +++ b/views/templates/layouts/layout.html.twig @@ -21,4 +21,8 @@ {% include "@ModuleAutoUpgrade/pages/" ~ page ~ ".html.twig" %}
+ diff --git a/views/templates/pages/errors/404.html.twig b/views/templates/pages/errors/404.html.twig index daf92f5ad..23f05ece8 100644 --- a/views/templates/pages/errors/404.html.twig +++ b/views/templates/pages/errors/404.html.twig @@ -18,27 +18,8 @@ *#} {% extends "@ModuleAutoUpgrade/layouts/error.html.twig" %} -{% block title %} -

- {{ 'Something went wrong...'|trans({}) }} -

-{% endblock %} - {% block description %}

{{ 'The page you requested cannot be found.'|trans({}) }}

{% endblock %} - -{% block button %} - - - -{% endblock %} diff --git a/views/templates/pages/errors/500.html.twig b/views/templates/pages/errors/500.html.twig deleted file mode 100644 index fdc8c05d7..000000000 --- a/views/templates/pages/errors/500.html.twig +++ /dev/null @@ -1,41 +0,0 @@ -{#** - * Copyright since 2007 PrestaShop SA and Contributors - * PrestaShop is an International Registered Trademark & Property of PrestaShop SA - * - * NOTICE OF LICENSE - * - * This source file is subject to the Academic Free License version 3.0 - * that is bundled with this package in the file LICENSE.md. - * It is also available through the world-wide-web at this URL: - * https://opensource.org/licenses/AFL-3.0 - * If you did not receive a copy of the license and are unable to - * obtain it through the world-wide-web, please send an email - * to license@prestashop.com so we can send you a copy immediately. - * - * @author PrestaShop SA and Contributors - * @copyright Since 2007 PrestaShop SA and Contributors - * @license https://opensource.org/licenses/AFL-3.0 Academic Free License version 3.0 - *#} -{% extends "@ModuleAutoUpgrade/layouts/error.html.twig" %} - -{% block title %} -

- {{ 'Something went wrong...'|trans({}) }} -

-{% endblock %} - -{% block description %} -

- {{ 'The module is no longer responding. You can try again in a few moments.'|trans({}) }} -

-{% endblock %} - -{% block button %} - - {{ 'Send error report'|trans({}) }} - - - - {{ 'Go back to Update assistant'|trans({}) }} - -{% endblock %} diff --git a/views/templates/pages/errors/generic.html.twig b/views/templates/pages/errors/generic.html.twig new file mode 100644 index 000000000..d85cbcafe --- /dev/null +++ b/views/templates/pages/errors/generic.html.twig @@ -0,0 +1,21 @@ +{% extends "@ModuleAutoUpgrade/layouts/error.html.twig" %} + +{% block error_code %} +
+ + + + + {{ 'Oh no!'|trans({}) }} + +
+{% endblock %} + +{% block description %} +

+ + +

+ + +{% endblock %} diff --git a/views/templates/pages/home.html.twig b/views/templates/pages/home.html.twig index 125fa4ff1..4b4b7ccb9 100644 --- a/views/templates/pages/home.html.twig +++ b/views/templates/pages/home.html.twig @@ -22,4 +22,9 @@ {% block container_inner %} {% include "@ModuleAutoUpgrade/steps/home.html.twig" %} + + + + + {% endblock %} From 72ca619da6c501f8ba274262855f4e34865c4c58 Mon Sep 17 00:00:00 2001 From: Thomas Nabord Date: Fri, 24 Jan 2025 15:19:32 +0000 Subject: [PATCH 02/11] Start adding the error reporting form --- _dev/src/ts/api/RequestHandler.ts | 3 +- _dev/src/ts/pages/ErrorPage.ts | 45 ++++++++++++++++--- _dev/src/ts/types/apiTypes.ts | 1 + _dev/src/ts/utils/Hydration.ts | 1 - .../self-managed/AbstractPageController.php | 2 + .../form-open-error-report.html.twig | 10 +++++ views/templates/layouts/layout.html.twig | 3 +- .../templates/pages/errors/generic.html.twig | 9 ++-- views/templates/steps/backup.html.twig | 12 +---- views/templates/steps/restore.html.twig | 12 +---- views/templates/steps/update.html.twig | 12 +---- 11 files changed, 66 insertions(+), 44 deletions(-) create mode 100644 views/templates/components/form-open-error-report.html.twig diff --git a/_dev/src/ts/api/RequestHandler.ts b/_dev/src/ts/api/RequestHandler.ts index a4e93682f..d98355fca 100644 --- a/_dev/src/ts/api/RequestHandler.ts +++ b/_dev/src/ts/api/RequestHandler.ts @@ -104,10 +104,11 @@ export class RequestHandler { } } - async #handleError(error: AxiosError): Promise { + async #handleError(error: AxiosError): Promise { new Hydration().hydrateError({ code: error.status, type: error.code, + requestParams: error.request, additionalContents: error.response?.data, }); } diff --git a/_dev/src/ts/pages/ErrorPage.ts b/_dev/src/ts/pages/ErrorPage.ts index 1b0fd049e..e32343708 100644 --- a/_dev/src/ts/pages/ErrorPage.ts +++ b/_dev/src/ts/pages/ErrorPage.ts @@ -17,7 +17,9 @@ * @license https://opensource.org/licenses/AFL-3.0 Academic Free License version 3.0 */ import api from '../api/RequestHandler'; +import { logStore } from '../store/LogStore'; import { ApiError } from '../types/apiTypes'; +import { Severity } from '../types/logsTypes'; import Hydration from '../utils/Hydration'; import PageAbstract from './PageAbstract'; @@ -48,6 +50,8 @@ export default class ErrorPage extends PageAbstract { public beforeDestroy = (): void => { this.#errorTemplateElement.removeEventListener(Hydration.hydrationEventName, this.#onError.bind(this)); + this.#submitErrorReportForm?.removeEventListener('submit', this.#onSubmit); + logStore.clearLogs(); }; get #errorTemplateElement(): HTMLTemplateElement { @@ -93,10 +97,18 @@ export default class ErrorPage extends PageAbstract { errorDescriptionElement.innerHTML = event.detail.type; } - // Store the contents in the hidden field so it can be used in the error reporting modal - const additionalContentsElement = errorElement.querySelector('.error-page__contents'); - if (additionalContentsElement && event.detail.additionalContents) { - additionalContentsElement.innerHTML = new String(event.detail.additionalContents).toString(); + // Store the contents in the logs so it can be used in the error reporting modal + if (event.detail.additionalContents) { + const logsContents = typeof event.detail.additionalContents === 'object' + ? JSON.stringify(event.detail.additionalContents) + : event.detail.additionalContents; + + logStore.addLog({ + severity: Severity.SUCCESS, + height: 0, + offsetTop: 0, + message: logsContents, + }); } // Finally, append the result on the page @@ -106,14 +118,29 @@ export default class ErrorPage extends PageAbstract { } targetElementToUpdate.replaceChildren(errorElement); + // Retrieve the route we called to fill in the context. + let route: string|null = null; + if (event.detail.requestParams?.responseURL) { + const params = new URLSearchParams(new URL(event.detail.requestParams?.responseURL)?.search); + route = params?.get('route'); + } + + logStore.addLog({ + severity: Severity.ERROR, + height: 0, + offsetTop: 0, + message: `An HTTP request failed on route ${route || 'N/A'}. Type: ${event.detail.type || 'N/A'} - Code ${event.detail.code || 'N/A'}`, + }); + // Enable events and page features this.#mountErrorPage(document.querySelector('.error-page')!); } #mountErrorPage(errorPage: Element): void { - console.log('mounting', errorPage); this.#form.addEventListener('submit', this.#onSubmit, {once: true}); + this.#submitErrorReportForm?.addEventListener('submit', this.#onSubmit); + // Display the proper action buttons const activeButtonElement = this.isOnHomePage ? errorPage.querySelector('.error-page__exit-button') @@ -139,9 +166,13 @@ export default class ErrorPage extends PageAbstract { return form; } - readonly #onSubmit = async (event: Event): Promise => { + get #submitErrorReportForm(): HTMLFormElement|null { + return document.forms.namedItem('submit-error-report'); + } + + readonly #onSubmit = async (event: SubmitEvent): Promise => { event.preventDefault(); - await api.post(this.#form.dataset.routeToSubmit!, new FormData(this.#form)); + await api.post((event.target as HTMLFormElement).dataset.routeToSubmit!, new FormData(this.#form)); }; } diff --git a/_dev/src/ts/types/apiTypes.ts b/_dev/src/ts/types/apiTypes.ts index 5049ac46e..404494552 100644 --- a/_dev/src/ts/types/apiTypes.ts +++ b/_dev/src/ts/types/apiTypes.ts @@ -48,6 +48,7 @@ interface ApiResponseAction { export interface ApiError { code?: number, type?: string, + requestParams?: XMLHttpRequest, additionalContents?: string|object } diff --git a/_dev/src/ts/utils/Hydration.ts b/_dev/src/ts/utils/Hydration.ts index 8b404a80d..b0309e8ab 100644 --- a/_dev/src/ts/utils/Hydration.ts +++ b/_dev/src/ts/utils/Hydration.ts @@ -19,7 +19,6 @@ import { ApiError, ApiResponseHydration } from '../types/apiTypes'; import { dialogContainer, routeHandler, scriptHandler } from '../autoUpgrade'; import { ScriptType } from '../types/scriptHandlerTypes'; -import { AxiosError } from 'axios'; import ErrorPage from '../pages/ErrorPage'; export default class Hydration { diff --git a/controllers/admin/self-managed/AbstractPageController.php b/controllers/admin/self-managed/AbstractPageController.php index ce5e7943f..34326a6ce 100644 --- a/controllers/admin/self-managed/AbstractPageController.php +++ b/controllers/admin/self-managed/AbstractPageController.php @@ -63,8 +63,10 @@ public function renderPage(string $page, array $params): string 'ps_version' => $this->getPsVersionClass(), 'data_transparency_link' => DocumentationLinks::PRESTASHOP_PROJECT_DATA_TRANSPARENCY_URL, + // Data for generic error page 'exit_to_shop_admin' => $this->upgradeContainer->getUrlGenerator()->getShopAdminAbsolutePathFromRequest($this->request), 'exit_to_app_home' => Routes::HOME_PAGE, + 'submit_error_report_route' => Routes::DISPLAY_ERROR_REPORT_MODAL, ], $pageSelectors::getAllSelectors(), $params diff --git a/views/templates/components/form-open-error-report.html.twig b/views/templates/components/form-open-error-report.html.twig new file mode 100644 index 000000000..ac07b926f --- /dev/null +++ b/views/templates/components/form-open-error-report.html.twig @@ -0,0 +1,10 @@ +
+ +
diff --git a/views/templates/layouts/layout.html.twig b/views/templates/layouts/layout.html.twig index 6254e4302..bcb286a69 100644 --- a/views/templates/layouts/layout.html.twig +++ b/views/templates/layouts/layout.html.twig @@ -22,7 +22,6 @@
diff --git a/views/templates/pages/errors/generic.html.twig b/views/templates/pages/errors/generic.html.twig index d85cbcafe..fde219eab 100644 --- a/views/templates/pages/errors/generic.html.twig +++ b/views/templates/pages/errors/generic.html.twig @@ -14,8 +14,11 @@ {% block description %}

- +

- - {% endblock %} + +{% block button %} + {% include "@ModuleAutoUpgrade/components/form-open-error-report.html.twig" %} + {{ parent() }} +{% endblock %} \ No newline at end of file diff --git a/views/templates/steps/backup.html.twig b/views/templates/steps/backup.html.twig index fac14bd4a..334fbc706 100644 --- a/views/templates/steps/backup.html.twig +++ b/views/templates/steps/backup.html.twig @@ -60,16 +60,8 @@ {{ 'Update without backup'|trans({}) }} -
- -
+ {% include "@ModuleAutoUpgrade/components/form-open-error-report.html.twig" %} + + {% include "@ModuleAutoUpgrade/components/form-open-error-report.html.twig" %} + + {% include "@ModuleAutoUpgrade/components/form-open-error-report.html.twig" %} + {% if backup_available %}
Date: Mon, 27 Jan 2025 17:00:16 +0100 Subject: [PATCH 03/11] feat: rework errors display --- _dev/.stylelintrc.cjs | 3 +- _dev/src/scss/layouts/_error.scss | 34 ++++++++++++++----- _dev/src/ts/pages/ErrorPage.ts | 7 ++-- views/templates/layouts/error.html.twig | 6 ++-- .../templates/pages/errors/generic.html.twig | 4 --- 5 files changed, 34 insertions(+), 20 deletions(-) diff --git a/_dev/.stylelintrc.cjs b/_dev/.stylelintrc.cjs index 798612146..c0b60603a 100644 --- a/_dev/.stylelintrc.cjs +++ b/_dev/.stylelintrc.cjs @@ -4,6 +4,7 @@ module.exports = { 'comment-empty-line-before': null, 'no-unknown-animations': null, 'scss/at-import-no-partial-leading-underscore': null, - 'scss/function-color-relative': null + 'scss/function-color-relative': null, + 'scss/percent-placeholder-pattern': null } }; diff --git a/_dev/src/scss/layouts/_error.scss b/_dev/src/scss/layouts/_error.scss index 08758abeb..beba1080d 100644 --- a/_dev/src/scss/layouts/_error.scss +++ b/_dev/src/scss/layouts/_error.scss @@ -31,6 +31,11 @@ $e: ".error-page"; padding: 4rem; background-color: var(--#{$ua-prefix}white); border: 1px solid var(--#{$ua-prefix}border-color); + + &:has(#{$e}__code.hidden) { + grid-template-columns: auto; + place-content: center; + } } &__code { @@ -67,13 +72,10 @@ $e: ".error-page"; &__buttons { display: flex; gap: 1rem 2rem; - } - &__button { - padding: 0.875rem 1rem; - font-size: 0.875rem; - font-weight: 400; - white-space: initial; + .btn { + @extend %btn--error-page; + } } @container ua-error (max-width: 700px) { @@ -92,10 +94,15 @@ $e: ".error-page"; &__buttons { flex-direction: column; - } - &__button { - justify-content: center; + > * { + width: 100%; + } + + .btn { + justify-content: center; + width: 100%; + } } } } @@ -156,3 +163,12 @@ html { } } } + +// Placeholder for buttons +%btn--error-page { + padding: 0.875rem 1rem; + font-size: 0.875rem; + font-weight: 400; + line-height: 1.25rem; + white-space: initial; +} diff --git a/_dev/src/ts/pages/ErrorPage.ts b/_dev/src/ts/pages/ErrorPage.ts index e32343708..771db9132 100644 --- a/_dev/src/ts/pages/ErrorPage.ts +++ b/_dev/src/ts/pages/ErrorPage.ts @@ -85,7 +85,8 @@ export default class ErrorPage extends PageAbstract { errorCodeSlotElements.forEach((element: Element, index: number) => { element.innerHTML = strigifiedCode[index]; }); - errorElement.querySelector('.error-page__code-missing')?.classList.add('hidden'); + } else { + errorElement.querySelector('.error-page__code')?.classList.add('hidden'); } // Display a user friendly text related to the code if it exists, otherwise write the error code. @@ -143,8 +144,8 @@ export default class ErrorPage extends PageAbstract { // Display the proper action buttons const activeButtonElement = this.isOnHomePage - ? errorPage.querySelector('.error-page__exit-button') - : errorPage.querySelector('.error-page__home-page-form'); + ? errorPage.querySelector('#exit-button') + : errorPage.querySelector('#home-page-form'); if (activeButtonElement) { activeButtonElement.classList.remove('hidden'); diff --git a/views/templates/layouts/error.html.twig b/views/templates/layouts/error.html.twig index 63e15d0be..f02f44e66 100644 --- a/views/templates/layouts/error.html.twig +++ b/views/templates/layouts/error.html.twig @@ -41,13 +41,13 @@
{% block button %} {# Default buttons go here, and children can override them #} - - - diff --git a/views/templates/pages/errors/generic.html.twig b/views/templates/pages/errors/generic.html.twig index fde219eab..47d0ecc4d 100644 --- a/views/templates/pages/errors/generic.html.twig +++ b/views/templates/pages/errors/generic.html.twig @@ -4,10 +4,6 @@
- - - {{ 'Oh no!'|trans({}) }} -
{% endblock %} From abf4b76d99a2c5b7bcf3a753d8549543ab8d8b50 Mon Sep 17 00:00:00 2001 From: Thomas Nabord Date: Mon, 27 Jan 2025 17:20:34 +0000 Subject: [PATCH 04/11] Run tools and fix feedbacks --- _dev/src/ts/api/RequestHandler.ts | 16 ++-- _dev/src/ts/api/baseApi.ts | 4 +- _dev/src/ts/api/requestInterceptor.ts | 18 ++--- _dev/src/ts/api/responseInterceptor.ts | 54 ++++++++++---- _dev/src/ts/pages/ErrorPage.ts | 73 +++++++++++++------ _dev/src/ts/routing/ScriptHandler.ts | 4 +- _dev/src/ts/types/apiTypes.ts | 21 ++++-- _dev/src/ts/utils/Hydration.ts | 4 +- _dev/tests/api/RequestHandler.test.ts | 20 +---- _dev/tests/components/LogsViewer.test.ts | 10 +-- _dev/tests/utils/Hydration.test.ts | 19 +++++ classes/Router/Routes.php | 1 - .../self-managed/AbstractPageController.php | 1 + .../admin/self-managed/Error404Controller.php | 1 - .../self-managed/ErrorGeneratorController.php | 4 +- views/templates/layouts/layout.html.twig | 2 +- .../templates/pages/errors/generic.html.twig | 48 +++++++++++- 17 files changed, 198 insertions(+), 102 deletions(-) diff --git a/_dev/src/ts/api/RequestHandler.ts b/_dev/src/ts/api/RequestHandler.ts index d98355fca..5fcc6a795 100644 --- a/_dev/src/ts/api/RequestHandler.ts +++ b/_dev/src/ts/api/RequestHandler.ts @@ -17,7 +17,7 @@ * @license https://opensource.org/licenses/AFL-3.0 Academic Free License version 3.0 */ import baseApi from './baseApi'; -import { ApiResponse, ApiResponseAction } from '../types/apiTypes'; +import { ApiResponse, ApiResponseAction, ApiResponseUnknown } from '../types/apiTypes'; import Hydration from '../utils/Hydration'; import { AxiosError } from 'axios'; @@ -39,11 +39,7 @@ export class RequestHandler { * @returns {Promise} * @description Sends a POST request to the specified route with optional data and pop state indicator. Cancels any ongoing request before initiating a new one. */ - public async post( - route: string, - data?: FormData, - fromPopState?: boolean - ): Promise { + public async post(route: string, data?: FormData, fromPopState?: boolean): Promise { this.abortCurrentPost(); // Create a new AbortController for the current request (used to cancel previous request) @@ -59,8 +55,8 @@ export class RequestHandler { const responseData = response.data; await this.#handleResponse(responseData, fromPopState); } catch (error) { - if (error) { - await this.#handleError(error as AxiosError); + if (error instanceof AxiosError) { + await this.#handleError(error); } } } @@ -104,12 +100,12 @@ export class RequestHandler { } } - async #handleError(error: AxiosError): Promise { + async #handleError(error: AxiosError): Promise { new Hydration().hydrateError({ code: error.status, type: error.code, requestParams: error.request, - additionalContents: error.response?.data, + additionalContents: error.response?.data }); } } diff --git a/_dev/src/ts/api/baseApi.ts b/_dev/src/ts/api/baseApi.ts index 447c34fda..c673d8639 100644 --- a/_dev/src/ts/api/baseApi.ts +++ b/_dev/src/ts/api/baseApi.ts @@ -27,8 +27,8 @@ const baseApi = axios.create({ Authorization: `Bearer ${() => window.AutoUpgradeVariables.token}` }, transitional: { - clarifyTimeoutError: true, - }, + clarifyTimeoutError: true + } }); addRequestInterceptor(baseApi); diff --git a/_dev/src/ts/api/requestInterceptor.ts b/_dev/src/ts/api/requestInterceptor.ts index f8074bb71..5efb8f1b7 100644 --- a/_dev/src/ts/api/requestInterceptor.ts +++ b/_dev/src/ts/api/requestInterceptor.ts @@ -1,13 +1,13 @@ -import { AxiosInstance, InternalAxiosRequestConfig } from "axios"; +import { AxiosInstance, InternalAxiosRequestConfig } from 'axios'; const requestFulfilledInterceptor = (config: InternalAxiosRequestConfig) => { - if (!config.data) { - config.data = new FormData(); - } - config.data?.append('dir', window.AutoUpgradeVariables.admin_dir); - return config; - }; + if (!config.data) { + config.data = new FormData(); + } + config.data?.append('dir', window.AutoUpgradeVariables.admin_dir); + return config; +}; export const addRequestInterceptor = (axios: AxiosInstance): void => { - axios.interceptors.request.use(requestFulfilledInterceptor); -} \ No newline at end of file + axios.interceptors.request.use(requestFulfilledInterceptor); +}; diff --git a/_dev/src/ts/api/responseInterceptor.ts b/_dev/src/ts/api/responseInterceptor.ts index 24e6e01fb..0391027cd 100644 --- a/_dev/src/ts/api/responseInterceptor.ts +++ b/_dev/src/ts/api/responseInterceptor.ts @@ -1,35 +1,59 @@ -import { AxiosError, AxiosInstance, AxiosResponse } from "axios"; -import { APP_ERR_RESPONSE_BAD_TYPE, APP_ERR_RESPONSE_INVALID } from "../types/apiTypes"; - -const responseFulfilledInterceptor = (response: AxiosResponse) => { - console.log('Checking response', response); +import { AxiosError, AxiosInstance, AxiosResponse } from 'axios'; +import { + ApiResponseUnknown, + ApiResponseUnknownObject, + APP_ERR_RESPONSE_BAD_TYPE, + APP_ERR_RESPONSE_EMPTY, + APP_ERR_RESPONSE_INVALID, + SilencedApiError +} from '../types/apiTypes'; +const responseFulfilledInterceptor = (response: AxiosResponse) => { + if (!response?.data) { + throw new AxiosError( + 'The response is empty', + APP_ERR_RESPONSE_EMPTY, + response.config, + response.request, + response + ); + } // All responses must be a parsed JSON. If we get another type of response, // this means something went wrong, i.e Another software answered. if (Object.prototype.toString.call(response.data) !== '[object Object]') { - throw new AxiosError('The response does not have a valid type', APP_ERR_RESPONSE_BAD_TYPE, response.config, response.request, response); + throw new AxiosError( + 'The response does not have a valid type', + APP_ERR_RESPONSE_BAD_TYPE, + response.config, + response.request, + response + ); } // Make sure the response contains the expected data - if (!response.data.kind) { - throw new AxiosError('The response contents is invalid', APP_ERR_RESPONSE_INVALID, response.config, response.request, response); + if (!(response.data as ApiResponseUnknownObject)?.kind) { + throw new AxiosError( + 'The response contents is invalid', + APP_ERR_RESPONSE_INVALID, + response.config, + response.request, + response + ); } return response; }; -const responseErroredInterceptor = (error: any) => { +const responseErroredInterceptor = (error: Error) => { const errorSilenced = [AxiosError.ERR_CANCELED]; // Ignore some errors - if (error instanceof AxiosError) { - if (error.code && errorSilenced.includes(error.code)) { - return Promise.reject(null); - } + if (error instanceof AxiosError && error.code && errorSilenced.includes(error.code)) { + return Promise.reject(new SilencedApiError()); } return Promise.reject(error); -}; +}; export const addResponseInterceptor = (axios: AxiosInstance): void => { axios.interceptors.response.use(responseFulfilledInterceptor, responseErroredInterceptor); -} +}; diff --git a/_dev/src/ts/pages/ErrorPage.ts b/_dev/src/ts/pages/ErrorPage.ts index 771db9132..49bcf6327 100644 --- a/_dev/src/ts/pages/ErrorPage.ts +++ b/_dev/src/ts/pages/ErrorPage.ts @@ -24,9 +24,7 @@ import Hydration from '../utils/Hydration'; import PageAbstract from './PageAbstract'; export default class ErrorPage extends PageAbstract { - public static templateId: string = 'error-page-template'; - // TODO: Improve this by putting the target in the template and sent it from the back end - public static targetElementIdToUpdate: string = 'ua_page'; + public static readonly templateId: string = 'error-page-template'; isOnHomePage: boolean = false; @@ -37,19 +35,25 @@ export default class ErrorPage extends PageAbstract { } public mount = (): void => { - // If the error page is already present on the DOM (For instance on a whole page refresh), // initalize it at once instead of waiting for an event. const errorPageFromBackEnd = document.querySelector('.error-page'); if (errorPageFromBackEnd) { this.#mountErrorPage(errorPageFromBackEnd); } else { - this.#errorTemplateElement.addEventListener(Hydration.hydrationEventName, this.#onError.bind(this), {once: true}); + this.#errorTemplateElement.addEventListener( + Hydration.hydrationEventName, + this.#onError.bind(this), + { once: true } + ); } }; public beforeDestroy = (): void => { - this.#errorTemplateElement.removeEventListener(Hydration.hydrationEventName, this.#onError.bind(this)); + this.#errorTemplateElement.removeEventListener( + Hydration.hydrationEventName, + this.#onError.bind(this) + ); this.#submitErrorReportForm?.removeEventListener('submit', this.#onSubmit); logStore.clearLogs(); }; @@ -61,11 +65,13 @@ export default class ErrorPage extends PageAbstract { throw new Error('Error template not found'); } - return element as HTMLTemplateElement; - } + ['target'].forEach((data) => { + if (!element.dataset[data]) { + throw new Error(`Missing data ${data} from element dataset.`); + } + }); - #onError = async (event: CustomEvent): Promise => { - this.#createErrorPage(event); + return element as HTMLTemplateElement; } #createErrorPage(event: CustomEvent): void { @@ -78,9 +84,14 @@ export default class ErrorPage extends PageAbstract { errorChild.id = `ua_error_${event.detail.type}`; } + const IsCodeAnHttpErrorCode = + typeof event.detail.code === 'number' && + event.detail.code >= 300 && + event.detail.code.toString().length === 3; + // If code is a HTTP error number (i.e 404, 500 etc.), let's change the text in the left column with it. - if (typeof event.detail.code === 'number' && event.detail.code >= 300 && event.detail.code.toString().length === 3) { - const strigifiedCode = event.detail.code.toString().replaceAll('0', 'O'); + if (IsCodeAnHttpErrorCode) { + const strigifiedCode = (event.detail.code as number).toString().replaceAll('0', 'O'); const errorCodeSlotElements = errorElement.querySelectorAll('.error-page__code-char'); errorCodeSlotElements.forEach((element: Element, index: number) => { element.innerHTML = strigifiedCode[index]; @@ -91,7 +102,9 @@ export default class ErrorPage extends PageAbstract { // Display a user friendly text related to the code if it exists, otherwise write the error code. const errorDescriptionElement = errorElement.querySelector('.error-page__desc'); - const userFriendlyDescriptionElement = errorDescriptionElement?.querySelector(`.error-page__desc-${event.detail.code || event.detail.type}`); + const userFriendlyDescriptionElement = errorDescriptionElement?.querySelector( + `.error-page__desc-${IsCodeAnHttpErrorCode ? event.detail.code : event.detail.type}` + ); if (userFriendlyDescriptionElement) { userFriendlyDescriptionElement.classList.remove('hidden'); } else if (errorDescriptionElement && event.detail.type) { @@ -100,27 +113,30 @@ export default class ErrorPage extends PageAbstract { // Store the contents in the logs so it can be used in the error reporting modal if (event.detail.additionalContents) { - const logsContents = typeof event.detail.additionalContents === 'object' - ? JSON.stringify(event.detail.additionalContents) - : event.detail.additionalContents; + const logsContents = + typeof event.detail.additionalContents === 'object' + ? JSON.stringify(event.detail.additionalContents) + : event.detail.additionalContents; logStore.addLog({ severity: Severity.SUCCESS, height: 0, offsetTop: 0, - message: logsContents, + message: logsContents }); } // Finally, append the result on the page - const targetElementToUpdate = document.getElementById(ErrorPage.targetElementIdToUpdate); + const targetElementToUpdate = document.getElementById( + this.#errorTemplateElement.dataset.target! + ); if (!targetElementToUpdate) { throw new Error('Target element cannot be found'); } targetElementToUpdate.replaceChildren(errorElement); // Retrieve the route we called to fill in the context. - let route: string|null = null; + let route: string | null = null; if (event.detail.requestParams?.responseURL) { const params = new URLSearchParams(new URL(event.detail.requestParams?.responseURL)?.search); route = params?.get('route'); @@ -130,7 +146,7 @@ export default class ErrorPage extends PageAbstract { severity: Severity.ERROR, height: 0, offsetTop: 0, - message: `An HTTP request failed on route ${route || 'N/A'}. Type: ${event.detail.type || 'N/A'} - Code ${event.detail.code || 'N/A'}`, + message: `HTTP request failed: Route ${route ?? 'N/A'} - Type: ${event.detail.type ?? 'N/A'} - Code ${event.detail.code ?? 'N/A'}` }); // Enable events and page features @@ -138,7 +154,7 @@ export default class ErrorPage extends PageAbstract { } #mountErrorPage(errorPage: Element): void { - this.#form.addEventListener('submit', this.#onSubmit, {once: true}); + this.#form.addEventListener('submit', this.#onSubmit, { once: true }); this.#submitErrorReportForm?.addEventListener('submit', this.#onSubmit); @@ -167,13 +183,24 @@ export default class ErrorPage extends PageAbstract { return form; } - get #submitErrorReportForm(): HTMLFormElement|null { + get #submitErrorReportForm(): HTMLFormElement | null { return document.forms.namedItem('submit-error-report'); } + readonly #onError = async (event: Event | CustomEvent): Promise => { + if (!(event instanceof CustomEvent)) { + console.debug('Unexpected type of event received.'); + return; + } + this.#createErrorPage(event); + }; + readonly #onSubmit = async (event: SubmitEvent): Promise => { event.preventDefault(); - await api.post((event.target as HTMLFormElement).dataset.routeToSubmit!, new FormData(this.#form)); + await api.post( + (event.target as HTMLFormElement).dataset.routeToSubmit!, + new FormData(this.#form) + ); }; } diff --git a/_dev/src/ts/routing/ScriptHandler.ts b/_dev/src/ts/routing/ScriptHandler.ts index b24565d25..1bb26d7dc 100644 --- a/_dev/src/ts/routing/ScriptHandler.ts +++ b/_dev/src/ts/routing/ScriptHandler.ts @@ -99,7 +99,9 @@ export default class ScriptHandler { console.debug(`No matching class found for ID: ${scriptID}`); // Outside a hydration, the scriptID matches the route query param. // If it does not exist, we load the error management script instead. - this.loadScript('error-page'); + if (!this.#currentScripts[ScriptType.PAGE]) { + this.loadScript('error-page'); + } return; } diff --git a/_dev/src/ts/types/apiTypes.ts b/_dev/src/ts/types/apiTypes.ts index 404494552..07de0540d 100644 --- a/_dev/src/ts/types/apiTypes.ts +++ b/_dev/src/ts/types/apiTypes.ts @@ -17,7 +17,7 @@ * @license https://opensource.org/licenses/AFL-3.0 Academic Free License version 3.0 */ interface ApiResponseHydration { - kind: 'hydrate', + kind: 'hydrate'; hydration: boolean; new_content: string; new_route?: string; @@ -26,12 +26,12 @@ interface ApiResponseHydration { } interface ApiResponseNextRoute { - kind: 'next', + kind: 'next'; next_route: string; } interface ApiResponseAction { - kind: 'action', + kind: 'action'; error: null | boolean; stepDone: null | boolean; next: string; @@ -46,15 +46,22 @@ interface ApiResponseAction { } export interface ApiError { - code?: number, - type?: string, - requestParams?: XMLHttpRequest, - additionalContents?: string|object + code?: number; + type?: string; + requestParams?: XMLHttpRequest; + additionalContents?: string | object; } +export class SilencedApiError extends Error {} + +export type ApiResponseUnknownObject = { + kind?: Pick; +}; +export type ApiResponseUnknown = string | ApiResponseUnknownObject | undefined; type ApiResponse = ApiResponseHydration | ApiResponseNextRoute | ApiResponseAction; export const APP_ERR_RESPONSE_BAD_TYPE = 'APP_ERR_RESPONSE_BAD_TYPE'; export const APP_ERR_RESPONSE_INVALID = 'APP_ERR_RESPONSE_INVALID'; +export const APP_ERR_RESPONSE_EMPTY = 'APP_ERR_RESPONSE_EMPTY'; export type { ApiResponseHydration, ApiResponseNextRoute, ApiResponseAction, ApiResponse }; diff --git a/_dev/src/ts/utils/Hydration.ts b/_dev/src/ts/utils/Hydration.ts index b0309e8ab..7805bc580 100644 --- a/_dev/src/ts/utils/Hydration.ts +++ b/_dev/src/ts/utils/Hydration.ts @@ -78,6 +78,8 @@ export default class Hydration { scriptHandler.loadScript('error-page'); const elementToUpdate = document.getElementById(ErrorPage.templateId); - elementToUpdate?.dispatchEvent(new CustomEvent(Hydration.hydrationEventName, {detail: error})); + elementToUpdate?.dispatchEvent( + new CustomEvent(Hydration.hydrationEventName, { detail: error }) + ); } } diff --git a/_dev/tests/api/RequestHandler.test.ts b/_dev/tests/api/RequestHandler.test.ts index c587a87cf..7ca6984b6 100644 --- a/_dev/tests/api/RequestHandler.test.ts +++ b/_dev/tests/api/RequestHandler.test.ts @@ -41,22 +41,8 @@ describe('RequestHandler', () => { mockHydrate.mockClear(); }); - it('should append admin_dir to FormData and call baseApi.post', async () => { - const formData = new FormData(); - const route = 'some_route'; - (baseApi.post as jest.Mock).mockResolvedValue({ data: {} }); - - await requestHandler.post(route, formData); - - expect(formData.get('dir')).toBe(window.AutoUpgradeVariables.admin_dir); - expect(baseApi.post).toHaveBeenCalledWith('', formData, { - params: { route }, - signal: expect.any(AbortSignal) - }); - }); - it('should handle response with next_route and make two API calls', async () => { - const response: ApiResponse = { next_route: 'next_route' }; + const response: ApiResponse = { kind: 'next', next_route: 'next_route' }; (baseApi.post as jest.Mock).mockResolvedValueOnce({ data: response }); const formData = new FormData(); @@ -69,7 +55,7 @@ describe('RequestHandler', () => { params: { route }, signal: expect.any(AbortSignal) }); - expect(baseApi.post).toHaveBeenNthCalledWith(2, '', formData, { + expect(baseApi.post).toHaveBeenNthCalledWith(2, '', undefined, { params: { route: 'next_route' }, signal: expect.any(AbortSignal) }); @@ -77,6 +63,7 @@ describe('RequestHandler', () => { it('should handle hydration response', async () => { const response: ApiResponse = { + kind: 'hydrate', hydration: true, new_content: 'new content', parent_to_update: 'parent', @@ -96,6 +83,7 @@ describe('RequestHandler', () => { it('should handle action response', async () => { const response: ApiResponseAction = { + kind: 'action', error: null, stepDone: false, next: 'Update', diff --git a/_dev/tests/components/LogsViewer.test.ts b/_dev/tests/components/LogsViewer.test.ts index 46ee0f4b9..061114dfc 100644 --- a/_dev/tests/components/LogsViewer.test.ts +++ b/_dev/tests/components/LogsViewer.test.ts @@ -18,7 +18,6 @@ */ import LogsViewer from '../../src/ts/components/LogsViewer'; import { logStore } from '../../src/ts/store/LogStore'; -import SpyInstance = jest.SpyInstance; // add this mock to avoid unnecessary error jest.mock('../../src/ts/routing/ScriptHandler', () => { @@ -30,10 +29,9 @@ jest.mock('../../src/ts/routing/ScriptHandler', () => { describe('LogsViewer', () => { let logsViewer: LogsViewer; let container: HTMLElement; - let errorSpy: SpyInstance; beforeEach(() => { - errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + jest.spyOn(console, 'error').mockImplementation(() => {}); logStore.clearLogs(); container = document.createElement('div'); @@ -129,12 +127,6 @@ describe('LogsViewer', () => { 'WARNING - Second warning' ]); await logsViewer.displaySummary(); - // add this spy to avoid error return - expect(errorSpy).toHaveBeenCalledWith( - expect.objectContaining({ - message: 'The string did not match the expected pattern.' - }) - ); const summaryContainer = container.querySelector('[data-slot-component="summary"]'); expect(summaryContainer).not.toBeNull(); diff --git a/_dev/tests/utils/Hydration.test.ts b/_dev/tests/utils/Hydration.test.ts index ecf3143af..da3e1cfde 100644 --- a/_dev/tests/utils/Hydration.test.ts +++ b/_dev/tests/utils/Hydration.test.ts @@ -36,6 +36,15 @@ jest.mock('../../src/ts/components/DialogContainer', () => { }); }); +jest.mock('../../src/ts/pages/ErrorPage', () => { + return jest.fn().mockImplementation(() => { + return { + mount: () => {}, + beforeDestroy: () => {} + }; + }); +}); + jest.mock('../../src/ts/pages/HomePage', () => { return jest.fn().mockImplementation(() => { return { @@ -79,6 +88,7 @@ describe('Hydration', () => { it('should update the innerHTML of the target element', () => { const response: ApiResponseHydration = { + kind: 'hydrate', hydration: true, new_content: `

New Content

`, parent_to_update: 'parent', @@ -93,6 +103,7 @@ describe('Hydration', () => { it('should call scriptHandler.loadScript when new_route is provided', () => { const response: ApiResponseHydration = { + kind: 'hydrate', hydration: true, new_content: `

New Content

`, parent_to_update: 'parent', @@ -109,6 +120,7 @@ describe('Hydration', () => { it('should call scriptHandler.loadScript when add_script is provided', () => { const response: ApiResponseHydration = { + kind: 'hydrate', hydration: true, new_content: `

New Content

`, parent_to_update: 'parent', @@ -122,6 +134,7 @@ describe('Hydration', () => { it('should call routeHandler.setNewRoute when new_route is provided and fromPopState is false', () => { const response: ApiResponseHydration = { + kind: 'hydrate', hydration: true, new_content: `

New Content

`, parent_to_update: 'parent', @@ -135,6 +148,7 @@ describe('Hydration', () => { it('should not call routeHandler.setNewRoute when fromPopState is true', () => { const response: ApiResponseHydration = { + kind: 'hydrate', hydration: true, new_content: `

New Content

`, parent_to_update: 'parent', @@ -148,6 +162,7 @@ describe('Hydration', () => { it('should not update the content if the element does not exist', () => { const response: ApiResponseHydration = { + kind: 'hydrate', hydration: true, new_content: `

New Content

`, parent_to_update: 'non_existent_id' @@ -163,6 +178,7 @@ describe('Hydration', () => { it('should dispatch the hydration event on the updated element', () => { const response: ApiResponseHydration = { + kind: 'hydrate', hydration: true, new_content: `

New Content

`, parent_to_update: 'parent', @@ -183,6 +199,7 @@ describe('Hydration', () => { it('should not refresh the dialog container if the DOM is untouched', () => { const response: ApiResponseHydration = { + kind: 'hydrate', hydration: true, new_content: `

New Content

`, parent_to_update: 'non_existent_id' @@ -213,6 +230,7 @@ describe('Hydration and scripts lifecycle', () => { it('should unload the current script safely before loading the next one', () => { const initialResponse: ApiResponseHydration = { + kind: 'hydrate', hydration: true, new_content: `

Old Content

`, parent_to_update: 'parent', @@ -224,6 +242,7 @@ describe('Hydration and scripts lifecycle', () => { expect(unloadRouteScriptMock).toHaveBeenCalledTimes(1); const nextResponse: ApiResponseHydration = { + kind: 'hydrate', hydration: true, new_content: `

New Content

`, parent_to_update: 'parent', diff --git a/classes/Router/Routes.php b/classes/Router/Routes.php index b9f512987..d9e24ab7d 100644 --- a/classes/Router/Routes.php +++ b/classes/Router/Routes.php @@ -93,5 +93,4 @@ class Routes const FAKE_ERROR_502 = 'fake-error-502'; const FAKE_INVALID_RESPONSE = 'fake-invalid-response'; const FAKE_TIMEOUT = 'fake-timeout'; - } diff --git a/controllers/admin/self-managed/AbstractPageController.php b/controllers/admin/self-managed/AbstractPageController.php index 34326a6ce..ef5b80142 100644 --- a/controllers/admin/self-managed/AbstractPageController.php +++ b/controllers/admin/self-managed/AbstractPageController.php @@ -64,6 +64,7 @@ public function renderPage(string $page, array $params): string 'data_transparency_link' => DocumentationLinks::PRESTASHOP_PROJECT_DATA_TRANSPARENCY_URL, // Data for generic error page + 'error_template_target' => PageSelectors::PAGE_PARENT_ID, 'exit_to_shop_admin' => $this->upgradeContainer->getUrlGenerator()->getShopAdminAbsolutePathFromRequest($this->request), 'exit_to_app_home' => Routes::HOME_PAGE, 'submit_error_report_route' => Routes::DISPLAY_ERROR_REPORT_MODAL, diff --git a/controllers/admin/self-managed/Error404Controller.php b/controllers/admin/self-managed/Error404Controller.php index 27b0db107..5f9e2b377 100644 --- a/controllers/admin/self-managed/Error404Controller.php +++ b/controllers/admin/self-managed/Error404Controller.php @@ -21,7 +21,6 @@ namespace PrestaShop\Module\AutoUpgrade\Controller; -use PrestaShop\Module\AutoUpgrade\Router\Routes; use Symfony\Component\HttpFoundation\Response; class Error404Controller extends AbstractPageController diff --git a/controllers/admin/self-managed/ErrorGeneratorController.php b/controllers/admin/self-managed/ErrorGeneratorController.php index fc60779c9..08395b1c8 100644 --- a/controllers/admin/self-managed/ErrorGeneratorController.php +++ b/controllers/admin/self-managed/ErrorGeneratorController.php @@ -46,8 +46,8 @@ public function generateBadResponse(): Response return new Response('[CONTENTS] Not a valid JSON, only a basic string'); } - public function generateTimeout(): never + public function generateTimeout(): void { sleep(60); } -} \ No newline at end of file +} diff --git a/views/templates/layouts/layout.html.twig b/views/templates/layouts/layout.html.twig index bcb286a69..b90e971f4 100644 --- a/views/templates/layouts/layout.html.twig +++ b/views/templates/layouts/layout.html.twig @@ -21,7 +21,7 @@ {% include "@ModuleAutoUpgrade/pages/" ~ page ~ ".html.twig" %}
-