ad-freiburg / qlever-ui

A user interface for QLever
Apache License 2.0
21 stars 15 forks source link

Consistently use `fetch` + improve error handling #69

Closed RobinTF closed 9 months ago

RobinTF commented 10 months ago

The JavaScript code of the QLever UI launches queries at many places (to get a query result, to get suggestions, to clear the cache, to get statistics, to get a share link, etc.). In the code so far, each request did this slightly differently. This is now unified as much as possible. In particular, all requests now use fetch and none of the requests use jQuery.ajax anymore.

Along the way, remove code that is no longer needed (like the code that realized SERVICE in the JavaScript) and improve the error messages when something goes wrong.

RobinTF commented 10 months ago

@hannahbast I believe we should discuss this 1:1. A lot of my coding decisions boil down to correcting execution order after using await and preventing so called "unhandled promise rejections" which occur when a promise is rejected somewhere without a catch or something. While browsers typically log such events it's better to explicitly handle this case, which is why you see all the "catch()" snippets throughout the codebase.

hannahbast commented 10 months ago

@RobinTF This query used to give a proper error message (the error message from QLever), now it just says "400 Bad Request" https://qlever.cs.uni-freiburg.de/wikidata/MCyZeF

RobinTF commented 10 months ago

@hannahbast Should be fixed now

hannahbast commented 10 months ago

@RobinTF Awesome, thanks for the quick reaction, I updated the QLever UI instance and now we get a nice error message again: https://qlever.cs.uni-freiburg.de/wikidata/MCyZeF

hannahbast commented 10 months ago

@RobinTF: We have to check the error handling again. I deliberately crashed https://qlever.cs.uni-freiburg.de/dblp . Now if you launch a query, you get

Unexpected token '<', "<!DOCTYPE "... is not valid JSON

Below is the full reply the Qlever UI gets (from the proxy in front of the backend) [1]. The error message used to be:

No reply from backend

That message was in the part, which you deleted: https://github.com/ad-freiburg/qlever-ui/blob/master/backend/static/js/qleverUI.js#L659 . And I now remember that it took some work to get it, but I forgot all the details.

[1] Full reply with headers:

HTTP/1.1 503 Service Unavailable
Date: Sat, 02 Dec 2023 23:59:22 GMT
Server: Apache/2.4.52 (Ubuntu)
Content-Length: 391
Connection: close
Content-Type: text/html; charset=iso-8859-1

<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>503 Service Unavailable</title>
</head><body>
<h1>Service Unavailable</h1>
<p>The server is temporarily unable to service your
request due to maintenance downtime or capacity
problems. Please try again later.</p>
<hr>
<address>Apache/2.4.52 (Ubuntu) Server at qlever.cs.uni-freiburg.de Port 443</address>
</body></html>
RobinTF commented 10 months ago

@hannahbast I added some changes that should make the error message nicer. Note that I deliberately changed the wording because technically at this point we got a response (if we have no internet connection we'll get a network error instead), we just realize that it's not valid JSON and inform the user about the fact that the response was not what we expected.

hannahbast commented 9 months ago

@RobinTF I have merged this now, thanks a lot for your work!