Skip to content
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

feat: add basic error messages if the analyzed page send back a 401 or 403 HTTP status code #322

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

feat: add basic error messages if the analyzed page send back a 401 or 403 HTTP status code #322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c42822e
feat: add basic error messages if the analyzed page send back a 401 o…
bgatellier Feb 4, 2024
7cef654
refactor: improve readability
bgatellier Apr 13, 2024
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
46 changes: 14 additions & 32 deletions assets/js/components/EcoIndexDialog.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,30 +9,16 @@ class EcoIndexDialog {
static #a11yDialog = null;

static ERROR_MESSAGES = {
404: `
{{- i18n "Error404" | markdownify -}}
`,
422: `
{{- i18n "Error422" | markdownify -}}
`,
429: `
{{- i18n "Error429" | markdownify -}}
`,
500: `
{{- i18n "Error500" | markdownify -}}
`,
502: `
{{- i18n "Error502" | markdownify -}}
`,
504: `
{{- i18n "Error504" | markdownify -}}
`,
520: `
{{- i18n "Error520" | markdownify -}}
`,
521: `
{{- i18n "Error521" | markdownify -}}
`,
401: `{{- i18n "Error401" | markdownify -}}`,
403: `{{- i18n "Error403" | markdownify -}}`,
404: `{{- i18n "Error404" | markdownify -}}`,
422: `{{- i18n "Error422" | markdownify -}}`,
429: `{{- i18n "Error429" | markdownify -}}`,
500: `{{- i18n "Error500" | markdownify -}}`,
502: `{{- i18n "Error502" | markdownify -}}`,
504: `{{- i18n "Error504" | markdownify -}}`,
520: `{{- i18n "Error520" | markdownify -}}`,
521: `{{- i18n "Error521" | markdownify -}}`,
Comment on lines +12 to +21
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, the API will not respond with a 401 or 403 status...
The tricky thing is that it will respond with a 521 code and this 521 code refers to the error code of the analyzed page 🤔
So the thing that would be perfect if to read the "referent" status code when a 521 occurs...

};

/**
Expand All @@ -44,8 +30,7 @@ class EcoIndexDialog {
* @returns {boolean} true if success, otherwise true
*/
static openPendingAnalysis(url) {
let title = `
{{- (i18n "AnalysisInProgressFor") -}}`;
let title = `{{- (i18n "AnalysisInProgressFor") -}}`;
title = replaceKeyIn(title, "URL", url);

EcoIndexDialog.#openLoadingRequest(title);
Expand All @@ -56,8 +41,7 @@ class EcoIndexDialog {
* with a loading spinner (abort possible)
*/
static openAnalysisRetrieval() {
const title = `
{{- (i18n "AnalysisRetrieval") -}}`;
const title = `{{- (i18n "AnalysisRetrieval") -}}`;

EcoIndexDialog.#openLoadingRequest(title);
}
Expand All @@ -71,8 +55,7 @@ class EcoIndexDialog {
*/
static openErrorMessage(errorCode, details) {
// Title
const title = `
{{- (i18n "AnalysisErrorTitle") -}}`;
const title = `{{- (i18n "AnalysisErrorTitle") -}}`;

// Body (message)
let errorMessage = errorCode
Expand All @@ -98,8 +81,7 @@ class EcoIndexDialog {
*/
static openResultSharing(url) {
// Title
const title = `
{{- (i18n "ShareTheResult") -}}`;
const title = `{{- (i18n "ShareTheResult") -}}`;

// Body (message)
const body = `
Expand Down
4 changes: 2 additions & 2 deletions assets/js/components/SiteAnalysisResult.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class SiteAnalysisResult {
pageResultData[key] = value;

if (key === "id") {
const screenshotUrl = AnalysisService.fetchAnalysisScreenshotUrlById(urlParams.get("id"))
const screenshotUrl = AnalysisService.getAnalysisScreenshotUrlById(urlParams.get("id"))
screenshotImgElement.setAttribute("src", screenshotUrl)
}
}
Expand All @@ -71,7 +71,7 @@ class SiteAnalysisResult {
// window.location.pathname is something like /resultat (in french) or /en/result (in english)
pageResultData = await AnalysisService.fetchAnalysisById(analysisId, window.location.pathname)

const screenshotUrl = AnalysisService.fetchAnalysisScreenshotUrlById(analysisId)
const screenshotUrl = AnalysisService.getAnalysisScreenshotUrlById(analysisId)
screenshotImgElement.setAttribute("src", screenshotUrl)

// update the link URL of every lang switcher
Expand Down
61 changes: 22 additions & 39 deletions assets/js/services/AnalysisService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,35 +22,25 @@ class AnalysisService {
try {
EcoIndexDialog.openPendingAnalysis(url);

ApiService.newAnalysisTaskByURL(url).then(
(taskId) => {
ApiService.fetchAnalysisTaskById(taskId).then(
(taskResult) => {
const ecoindex = taskResult.ecoindex_result;
const taskId = await ApiService.newAnalysisTaskByURL(url)

if (taskResult.status === "SUCCESS" && ecoindex.status === "SUCCESS") {
ResultCacheService.add(ecoindex.detail);
redirectToResults(taskId, resultUrlPrefix);
}
const taskResult = await ApiService.fetchAnalysisTaskById(taskId)

const ecoindex = taskResult.ecoindex_result;

if (taskResult.status === "SUCCESS" && ecoindex.status === "FAILURE") {
const e = taskResult.ecoindex_result.error;
EcoIndexDialog.openErrorMessage(e.status_code, e);
}
if (taskResult.status === "SUCCESS" && ecoindex.status === "SUCCESS") {
ResultCacheService.add(ecoindex.detail);
redirectToResults(taskId, resultUrlPrefix);
}

if (taskResult.status === "FAILURE") {
EcoIndexDialog.openErrorMessage(599, taskResult.task_error);
}
},
(e) => {
this.#handleError(e);
}
);
},
(e) => {
this.#handleError(e);
}
);
if (taskResult.status === "SUCCESS" && ecoindex.status === "FAILURE") {
const e = taskResult.ecoindex_result.error;
EcoIndexDialog.openErrorMessage(e.status_code, e);
}

if (taskResult.status === "FAILURE") {
EcoIndexDialog.openErrorMessage(599, taskResult.task_error);
}
} catch (e) {
this.#handleError(e);
}
Expand All @@ -60,8 +50,8 @@ class AnalysisService {
* @param {string} id
* @returns {string}
*/
fetchAnalysisScreenshotUrlById(id) {
return ApiService.fetchAnalysisScreenshotUrlById(id)
getAnalysisScreenshotUrlById(id) {
return ApiService.getAnalysisScreenshotUrlById(id)
}

/**
Expand All @@ -79,17 +69,10 @@ class AnalysisService {
// Otherwise fetch from api
try {
EcoIndexDialog.openAnalysisRetrieval();
await ApiService.fetchAnalysisById(id).then(
(result) => {
apiResult = result;
ResultCacheService.add(result);
redirectToResults(result.id, resultPagePrefix);
EcoIndexDialog.close();
},
(e) => {
this.#handleError(e);
}
);
apiResult = await ApiService.fetchAnalysisById(id)
ResultCacheService.add(apiResult);
redirectToResults(apiResult.id, resultPagePrefix);
EcoIndexDialog.close();
} catch (e) {
this.#handleError(e);
return null;
Expand Down
2 changes: 1 addition & 1 deletion assets/js/services/ApiService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class ApiService {
* @param {string} id Analysis Id
* @returns {string} API URL
*/
fetchAnalysisScreenshotUrlById(id) {
getAnalysisScreenshotUrlById(id) {
return `${BASE_URL}ecoindexes/${id}/screenshot`;
}

Expand Down
8 changes: 6 additions & 2 deletions i18n/en.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,12 @@ for “##URL##”"""
other = "Please retry later."
[Close]
other = "Close"
[Error401]
other = "401 error"
[Error403]
other = "403 error"
[Error404]
other = "404 error"
[Error422]
other = "There is an invalid parameter, check your request."
[Error429]
Expand All @@ -140,8 +146,6 @@ More information on page [How it works](/how-it-works/)."""
other = "The page you want to test can not be analyzed because its response status code is not 200"
[NoJSMessage]
other = "**Warning**: JavaScript is currently required for the new version of ecoIndex to work ([enable JavaScript in your browser](https://www.enable-javascript.com/))."
[Error404]
other = "404 error"
[ShareTheResult]
other = "Share my result"
[CopyURL]
Expand Down
8 changes: 6 additions & 2 deletions i18n/fr.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,12 @@ pour « ##URL## »"""
other = "Veuillez réessayer plus tard"
[Close]
other = "Fermer"
[Error401]
other = "Erreur 401"
[Error403]
other = "Erreur 403"
[Error404]
other = "Erreur 404"
[Error422]
other = "Un paramètre n’est pas valide, pouvez-vous vérifier votre requête ?"
[Error429]
Expand All @@ -140,8 +146,6 @@ Pour en savoir plus, veuiller consulter la page [Comment ça marche ?](/commen
other = "La page que vous essayez de tester ne peut pas être analysée car elle est en erreur (son code réponse n'est pas 200)"
[NoJSMessage]
other = "**Attention** : Javascript est actuellement requis pour le fonctionnement de la nouvelle version d'ecoIndex ([activer JavaScript dans votre navigateur](https://www.enable-javascript.com/fr/))."
[Error404]
other = "Erreur 404"
[ShareTheResult]
other = "Partager mon résultat"
[CopyURL]
Expand Down
Loading