[Bug 35199] New: Fix error handling in http-client.js
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35199 Bug ID: 35199 Summary: Fix error handling in http-client.js Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: ASSIGNED Severity: major Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart+koha@gmail.com Reporter: jonathan.druart+koha@gmail.com QA Contact: testopia@bugs.koha-community.org Depends on: 34448 Coming from 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 totally wrong. We are now showing a JS error instead of the expected 500: internal server error! Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34448 [Bug 34448] ERM should be able to display error messages coming from the API -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35199 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35199 --- Comment #1 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 158121 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=158121&action=edit 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 => {} -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35199 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |35201 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35201 [Bug 35201] Cypress tests for the Preservation module are failing -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35199 --- Comment #2 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #1)
1. Make sure all cypress tests pass
Note that "Preservation" tests will be fixed on bug 35201. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35199 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #158121|0 |1 is obsolete| | --- Comment #3 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 158125 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=158125&action=edit 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 => {} -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35199 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35199 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #158125|0 |1 is obsolete| | --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 158129 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=158129&action=edit 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 <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35199 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA QA Contact|testopia@bugs.koha-communit |tomascohen@gmail.com |y.org | CC| |tomascohen@gmail.com --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- All things (but Preservation) pass, the changes are simple and make sense. QA script happy as well. Straight QA. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35199 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |23.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35199 --- Comment #6 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35199 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|23.11.00 |23.11.00,23.05.05 released in| | Status|Pushed to master |Pushed to stable CC| |fridolin.somers@biblibre.co | |m --- Comment #7 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.05.x for 23.05.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35199 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable Version(s)|23.11.00,23.05.05 |23.11.00,23.05.05,22.11.12 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35199 --- Comment #8 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Nice work everyone! Pushed to oldstable for 22.11.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org