From df3acf3d68cd24d3b1015432206236b563aaa5d1 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 --- .../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 360a3c0bfc..7b6f3742fd 100644 --- a/t/cypress/integration/ERM/Agreements_spec.ts +++ b/t/cypress/integration/ERM/Agreements_spec.ts @@ -132,12 +132,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 @@ -386,11 +385,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! @@ -577,7 +575,6 @@ describe("Agreement CRUD operations", () => { cy.intercept("PUT", "/api/v1/erm/agreements/*", req => { req.reply({ statusCode: 500, - error: "Something went wrong", delay: 1000, }); }); @@ -585,7 +582,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! @@ -664,11 +661,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 131b799c5a..c52da243ef 100644 --- a/t/cypress/integration/ERM/Dialog_spec.ts +++ b/t/cypress/integration/ERM/Dialog_spec.ts @@ -50,12 +50,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 dcb091134c..5718ca4c6c 100644 --- a/t/cypress/integration/ERM/Licenses_spec.ts +++ b/t/cypress/integration/ERM/Licenses_spec.ts @@ -49,12 +49,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 @@ -157,11 +156,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! @@ -238,11 +236,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! @@ -309,11 +306,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 86199a63f7..2ba71e529a 100644 --- a/t/cypress/integration/ERM/Packages_spec.ts +++ b/t/cypress/integration/ERM/Packages_spec.ts @@ -38,12 +38,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 @@ -113,11 +112,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! @@ -187,11 +185,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! @@ -304,11 +301,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 3bcb8d5a0b..22229b339c 100644 --- a/t/cypress/integration/ERM/Titles_spec.ts +++ b/t/cypress/integration/ERM/Titles_spec.ts @@ -195,7 +195,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! @@ -350,11 +350,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! @@ -459,11 +458,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