Bugzilla – Attachment 158125 Details for
Bug 35199
Fix error handling in http-client.js
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 35199: (bug 34448 follow-up) Fix error handling in http-client.js
Bug-35199-bug-34448-follow-up-Fix-error-handling-i.patch (text/plain), 14.52 KB, created by
Jonathan Druart
on 2023-10-31 14:31:48 UTC
(
hide
)
Description:
Bug 35199: (bug 34448 follow-up) Fix error handling in http-client.js
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2023-10-31 14:31:48 UTC
Size:
14.52 KB
patch
obsolete
>From 7fd6b7ffa58a7327a49d8e85f6d6cdf7d582f57c Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >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 => {} >--- > .../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 875ae2badc2..ffee1ce979d 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 360a3c0bfc9..7b6f3742fd7 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 131b799c5a7..c52da243ef7 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 dcb091134c3..5718ca4c6c4 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 86199a63f7a..2ba71e529ae 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 3bcb8d5a0bf..22229b339c4 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.34.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 35199
:
158121
|
158125
|
158129