From ae3804270d09e19be26e853e349f37de4e7b1c62 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 31 Oct 2023 14:45:08 +0100 Subject: [PATCH] Bug 35199: (bug 34448 follow-up) Fix error handling in http-client.js From bug bug 34448. 386 // Submit the form, get 500 387 cy.intercept("POST", "/api/v1/erm/agreements", { 388 statusCode: 500, 389 error: "Something went wrong", 390 }); 391 cy.get("#agreements_add").contains("Submit").click(); 392 cy.get("main div[class='dialog alert']").contains( 393 "Something went wrong: SyntaxError: Unexpected end of JSON input" 394 ); This is wrong: we are now showing a JS error (SyntaxError) instead of the expected 500: internal server error! The problem was that a regular 500 does not have anything in the body, and _fetchJSON didn't handle that ( JSON.parse(text) ). If the body of the response does not contain anything we need to get the text from statusText (which contains "Internal Server Error" in case of 500). Test plan: 1. Make sure all cypress tests pass 2. Confirm the above: Raise an exception from a given route (/agreements for instance) and confirm that the error displayed on the interface is correct (ie. not SyntaxError, but "Error: Internal Server Error") For QA: * This change is covered properly in Dialog_spec.ts, no need to redo it in every other test files. * Without the following change in count, we see: "Something went wrong: Error: Error: Internal Server Error" because setError is called twice. We don't need to set the error from count, it has been set from _fetchJSON already. - error => { - setError(error.toString()); - } + error => {} Signed-off-by: Tomas Cohen Arazi (cherry picked from commit df3acf3d68cd24d3b1015432206236b563aaa5d1) Signed-off-by: Fridolin Somers --- .../prog/js/vue/fetch/http-client.js | 21 ++++++++++--------- t/cypress/integration/ERM/Agreements_spec.ts | 12 ++++------- t/cypress/integration/ERM/Dialog_spec.ts | 15 +++++++++++-- t/cypress/integration/ERM/Licenses_spec.ts | 12 ++++------- t/cypress/integration/ERM/Packages_spec.ts | 12 ++++------- t/cypress/integration/ERM/Titles_spec.ts | 8 +++---- 6 files changed, 39 insertions(+), 41 deletions(-) diff --git a/koha-tmpl/intranet-tmpl/prog/js/vue/fetch/http-client.js b/koha-tmpl/intranet-tmpl/prog/js/vue/fetch/http-client.js index 875ae2badc..ffee1ce979 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/vue/fetch/http-client.js +++ b/koha-tmpl/intranet-tmpl/prog/js/vue/fetch/http-client.js @@ -24,13 +24,16 @@ class HttpClient { .then(response => { if (!response.ok) { return response.text().then(text => { - let json = JSON.parse(text); - let message = - json.error || - json.errors.map(e => e.message).join("\n") || - json; - console.log("Server returned an error:"); - console.log(response); + let message; + if (text) { + let json = JSON.parse(text); + message = + json.error || + json.errors.map(e => e.message).join("\n") || + json; + } else { + message = response.statusText; + } throw new Error(message); }); } @@ -134,9 +137,7 @@ class HttpClient { return response.headers.get("X-Total-Count"); } }, - error => { - setError(error.toString()); - } + error => {} ); } diff --git a/t/cypress/integration/ERM/Agreements_spec.ts b/t/cypress/integration/ERM/Agreements_spec.ts index 1f507e5414..e7ebfd565f 100644 --- a/t/cypress/integration/ERM/Agreements_spec.ts +++ b/t/cypress/integration/ERM/Agreements_spec.ts @@ -136,12 +136,11 @@ describe("Agreement CRUD operations", () => { // GET agreements returns 500 cy.intercept("GET", "/api/v1/erm/agreements*", { statusCode: 500, - error: "Something went wrong", }); cy.visit("/cgi-bin/koha/erm/erm.pl"); cy.get("#navmenulist").contains("Agreements").click(); cy.get("main div[class='dialog alert']").contains( - /Something went wrong/ + "Something went wrong: Error: Internal Server Error" ); // GET agreements returns empty list @@ -368,11 +367,10 @@ describe("Agreement CRUD operations", () => { // Submit the form, get 500 cy.intercept("POST", "/api/v1/erm/agreements", { statusCode: 500, - error: "Something went wrong", }); cy.get("#agreements_add").contains("Submit").click(); cy.get("main div[class='dialog alert']").contains( - "Something went wrong: SyntaxError: Unexpected end of JSON input" + "Something went wrong: Error: Internal Server Error" ); // Submit the form, success! @@ -533,7 +531,6 @@ describe("Agreement CRUD operations", () => { cy.intercept("PUT", "/api/v1/erm/agreements/*", req => { req.reply({ statusCode: 500, - error: "Something went wrong", delay: 1000, }); }); @@ -541,7 +538,7 @@ describe("Agreement CRUD operations", () => { cy.get("main div[class='modal_centered']").contains("Submitting..."); cy.wait(1000); cy.get("main div[class='dialog alert']").contains( - "Something went wrong: SyntaxError: Unexpected end of JSON input" + "Something went wrong: Error: Internal Server Error" ); // Submit the form, success! @@ -619,11 +616,10 @@ describe("Agreement CRUD operations", () => { // Accept the confirmation dialog, get 500 cy.intercept("DELETE", "/api/v1/erm/agreements/*", { statusCode: 500, - error: "Something went wrong", }); cy.contains("Yes, delete").click(); cy.get("main div[class='dialog alert']").contains( - "Something went wrong: SyntaxError: Unexpected end of JSON input" + "Something went wrong: Error: Internal Server Error" ); // Accept the confirmation dialog, success! diff --git a/t/cypress/integration/ERM/Dialog_spec.ts b/t/cypress/integration/ERM/Dialog_spec.ts index 3333218651..6acd47f726 100644 --- a/t/cypress/integration/ERM/Dialog_spec.ts +++ b/t/cypress/integration/ERM/Dialog_spec.ts @@ -55,12 +55,23 @@ describe("Dialog operations", () => { // GET package returns 500 cy.intercept("GET", "/api/v1/erm/eholdings/local/packages*", { statusCode: 500, - error: "Something went wrong", + body: { + error: "This is a specific error message", + }, + }); + cy.visit("/cgi-bin/koha/erm/erm.pl"); + cy.get("#navmenulist").contains("Packages").click(); + cy.get("main div[class='dialog alert']").contains( + "Something went wrong: Error: This is a specific error message" + ); + + cy.intercept("GET", "/api/v1/erm/eholdings/local/packages*", { + statusCode: 500, // No body, in case of Internal Server Error, we get statusText }); cy.visit("/cgi-bin/koha/erm/erm.pl"); cy.get("#navmenulist").contains("Packages").click(); cy.get("main div[class='dialog alert']").contains( - /Something went wrong/ + "Something went wrong: Error: Internal Server Error" ); cy.intercept("GET", "/api/v1/erm/agreements*", []); diff --git a/t/cypress/integration/ERM/Licenses_spec.ts b/t/cypress/integration/ERM/Licenses_spec.ts index 4c923502cc..a4b7a380d9 100644 --- a/t/cypress/integration/ERM/Licenses_spec.ts +++ b/t/cypress/integration/ERM/Licenses_spec.ts @@ -52,12 +52,11 @@ describe("License CRUD operations", () => { // GET license returns 500 cy.intercept("GET", "/api/v1/erm/licenses*", { statusCode: 500, - error: "Something went wrong", }); cy.visit("/cgi-bin/koha/erm/erm.pl"); cy.get("#navmenulist").contains("Licenses").click(); cy.get("main div[class='dialog alert']").contains( - /Something went wrong/ + "Something went wrong: Error: Internal Server Error" ); // GET licenses returns empty list @@ -138,11 +137,10 @@ describe("License CRUD operations", () => { // Submit the form, get 500 cy.intercept("POST", "/api/v1/erm/licenses", { statusCode: 500, - error: "Something went wrong", }); cy.get("#licenses_add").contains("Submit").click(); cy.get("main div[class='dialog alert']").contains( - "Something went wrong: SyntaxError: Unexpected end of JSON input" + "Something went wrong: Error: Internal Server Error" ); // Submit the form, success! @@ -192,11 +190,10 @@ describe("License CRUD operations", () => { // Submit the form, get 500 cy.intercept("PUT", "/api/v1/erm/licenses/*", { statusCode: 500, - error: "Something went wrong", }); cy.get("#licenses_add").contains("Submit").click(); cy.get("main div[class='dialog alert']").contains( - "Something went wrong: SyntaxError: Unexpected end of JSON input" + "Something went wrong: Error: Internal Server Error" ); // Submit the form, success! @@ -262,11 +259,10 @@ describe("License CRUD operations", () => { // Accept the confirmation dialog, get 500 cy.intercept("DELETE", "/api/v1/erm/licenses/*", { statusCode: 500, - error: "Something went wrong", }); cy.contains("Yes, delete").click(); cy.get("main div[class='dialog alert']").contains( - "Something went wrong: SyntaxError: Unexpected end of JSON input" + "Something went wrong: Error: Internal Server Error" ); // Accept the confirmation dialog, success! diff --git a/t/cypress/integration/ERM/Packages_spec.ts b/t/cypress/integration/ERM/Packages_spec.ts index 88c1465e82..478b261c3e 100644 --- a/t/cypress/integration/ERM/Packages_spec.ts +++ b/t/cypress/integration/ERM/Packages_spec.ts @@ -43,12 +43,11 @@ describe("Package CRUD operations", () => { // GET package returns 500 cy.intercept("GET", "/api/v1/erm/eholdings/local/packages*", { statusCode: 500, - error: "Something went wrong", }); cy.visit("/cgi-bin/koha/erm/erm.pl"); cy.get("#navmenulist").contains("Packages").click(); cy.get("main div[class='dialog alert']").contains( - /Something went wrong/ + "Something went wrong: Error: Internal Server Error" ); // GET packages returns empty list @@ -117,11 +116,10 @@ describe("Package CRUD operations", () => { // Submit the form, get 500 cy.intercept("POST", "/api/v1/erm/eholdings/local/packages", { statusCode: 500, - error: "Something went wrong", }); cy.get("#packages_add").contains("Submit").click(); cy.get("main div[class='dialog alert']").contains( - "Something went wrong: SyntaxError: Unexpected end of JSON input" + "Something went wrong: Error: Internal Server Error" ); // Submit the form, success! @@ -190,11 +188,10 @@ describe("Package CRUD operations", () => { // Submit the form, get 500 cy.intercept("PUT", "/api/v1/erm/eholdings/local/packages/*", { statusCode: 500, - error: "Something went wrong", }); cy.get("#packages_add").contains("Submit").click(); cy.get("main div[class='dialog alert']").contains( - "Something went wrong: SyntaxError: Unexpected end of JSON input" + "Something went wrong: Error: Internal Server Error" ); // Submit the form, success! @@ -305,11 +302,10 @@ describe("Package CRUD operations", () => { // Accept the confirmation dialog, get 500 cy.intercept("DELETE", "/api/v1/erm/eholdings/local/packages/*", { statusCode: 500, - error: "Something went wrong", }); cy.contains("Yes, delete").click(); cy.get("main div[class='dialog alert']").contains( - "Something went wrong: SyntaxError: Unexpected end of JSON input" + "Something went wrong: Error: Internal Server Error" ); // Accept the confirmation dialog, success! diff --git a/t/cypress/integration/ERM/Titles_spec.ts b/t/cypress/integration/ERM/Titles_spec.ts index 413ba60520..3330997710 100644 --- a/t/cypress/integration/ERM/Titles_spec.ts +++ b/t/cypress/integration/ERM/Titles_spec.ts @@ -198,7 +198,7 @@ describe("Title CRUD operations", () => { }); cy.get("#titles_add").contains("Submit").click(); cy.get("main div[class='dialog alert']").contains( - "Something went wrong: SyntaxError: Unexpected end of JSON input" + "Something went wrong: Error: Internal Server Error" ); // Submit the form, success! @@ -352,11 +352,10 @@ describe("Title CRUD operations", () => { // Submit the form, get 500 cy.intercept("PUT", "/api/v1/erm/eholdings/local/titles/*", { statusCode: 500, - error: "Something went wrong", }); cy.get("#titles_add").contains("Submit").click(); cy.get("main div[class='dialog alert']").contains( - "Something went wrong: SyntaxError: Unexpected end of JSON input" + "Something went wrong: Error: Internal Server Error" ); // Submit the form, success! @@ -460,11 +459,10 @@ describe("Title CRUD operations", () => { // Accept the confirmation dialog, get 500 cy.intercept("DELETE", "/api/v1/erm/eholdings/local/titles/*", { statusCode: 500, - error: "Something went wrong", }); cy.contains("Yes, delete").click(); cy.get("main div[class='dialog alert']").contains( - "Something went wrong: SyntaxError: Unexpected end of JSON input" + "Something went wrong: Error: Internal Server Error" ); // Accept the confirmation dialog, success! -- 2.39.5