From 1301f6ba6c6ea31bdbcd3619d7f235912726f30a Mon Sep 17 00:00:00 2001 From: Bryan Newbold Date: Tue, 28 Jul 2020 15:54:44 -0700 Subject: refactor search macros into new file --- python/fatcat_web/templates/container_search.html | 6 +-- python/fatcat_web/templates/entity_macros.html | 40 -------------- python/fatcat_web/templates/release_search.html | 5 +- python/fatcat_web/templates/search_macros.html | 66 +++++++++++++++++++++++ 4 files changed, 72 insertions(+), 45 deletions(-) create mode 100644 python/fatcat_web/templates/search_macros.html (limited to 'python/fatcat_web/templates') diff --git a/python/fatcat_web/templates/container_search.html b/python/fatcat_web/templates/container_search.html index 2566f542..ce868991 100644 --- a/python/fatcat_web/templates/container_search.html +++ b/python/fatcat_web/templates/container_search.html @@ -1,4 +1,4 @@ -{% import "entity_macros.html" as entity_macros %} +{% import "search_macros.html" as search_macros %} {% extends "base.html" %} {% block title %} @@ -32,7 +32,7 @@ {% if found %} {% if found.results %} - {{ entity_macros.top_results(query, found) }} + {{ search_macros.top_results(query, found) }} {% for entity in found.results %}
@@ -55,7 +55,7 @@ {% if found.results|length > 8 %}
- {{ entity_macros.bottom_results(query, found, endpoint='container_search') }} + {{ search_macros.bottom_results(query, found, endpoint='container_search') }}
{% endif %} diff --git a/python/fatcat_web/templates/entity_macros.html b/python/fatcat_web/templates/entity_macros.html index 0e7f135a..ab0e817a 100644 --- a/python/fatcat_web/templates/entity_macros.html +++ b/python/fatcat_web/templates/entity_macros.html @@ -262,43 +262,3 @@ yellow {% endif %} {%- endmacro %} -{% macro top_results(query, found) -%} - -Showing - {% if found.offset == 0 %} - first - {% else %} - results {{ found.offset }} — - {% endif %} - - {{ found.offset + found.count_returned }} - out of {{ found.count_found }} results - - -{%- endmacro %} - - -{% macro bottom_results(query, found, endpoint='release_search') -%} - -{% if found.offset > 0 %} - {% if found.offset - found.limit < 0 %} - « Previous - {% else %} - « Previous - {% endif %} -{% else %} - « Previous -{% endif %} - -  Showing results {{ found.offset }} — {{ found.offset + -found.count_returned }} out of {{ found.count_found }} results   - -{% if found.offset + found.limit < found.count_found and found.offset + found.limit < found.deep_page_limit %} - Next » - {% else %} - Next » -{% endif %} - -
- -{%- endmacro %} diff --git a/python/fatcat_web/templates/release_search.html b/python/fatcat_web/templates/release_search.html index 58aa35d6..e004efc1 100644 --- a/python/fatcat_web/templates/release_search.html +++ b/python/fatcat_web/templates/release_search.html @@ -1,4 +1,5 @@ {% import "entity_macros.html" as entity_macros %} +{% import "search_macros.html" as search_macros %} {% extends "base.html" %} {% block title %} @@ -37,7 +38,7 @@ {% if found %} {% if found.results %} - {{ entity_macros.top_results(query, found) }} + {{ search_macros.top_results(query, found) }} {% for paper in found.results %} {{ entity_macros.release_search_result_row(paper) }} @@ -46,7 +47,7 @@ {% if found.results|length > 8 %}
- {{ entity_macros.bottom_results(query, found, endpoint='release_search') }} + {{ search_macros.bottom_results(query, found, endpoint='release_search') }}
{% endif %} diff --git a/python/fatcat_web/templates/search_macros.html b/python/fatcat_web/templates/search_macros.html new file mode 100644 index 00000000..383c271c --- /dev/null +++ b/python/fatcat_web/templates/search_macros.html @@ -0,0 +1,66 @@ + +{% macro top_results(query, found) -%} + +Showing + {% if found.offset == 0 %} + first + {% else %} + results {{ found.offset }} — + {% endif %} + + {{ found.offset + found.count_returned }} + out of {{ found.count_found }} results + + +{%- endmacro %} + + +{% macro bottom_results(query, found, endpoint='release_search') -%} + +{% if found.offset > 0 %} + {% if found.offset - found.limit < 0 %} + « Previous + {% else %} + « Previous + {% endif %} +{% else %} + « Previous +{% endif %} + +  Showing results {{ found.offset }} — {{ found.offset + +found.count_returned }} out of {{ found.count_found }} results   + +{% if found.offset + found.limit < found.count_found and found.offset + found.limit < found.deep_page_limit %} + Next » + {% else %} + Next » +{% endif %} + + + +{%- endmacro %} + + +{% macro es_error_msg(es_error) %} +
+ + {% if es_error.status_code == 400 %} +
+
+ Query Error +
+

Computer said: {{ es_error.description }} +

Query parsing is currently very naive. Sometimes you can fix this + problem by adding quotes around terms or entire phrases. +

+ {% else %} +
+
+ Search Index Error ({{ es_error.status_code }}) +
+

Computer said: {{ es_error.description }} +

+ {% endif %} +
+{% endmacro %} + -- cgit v1.2.3 From 76cec8852f6e431b1c87f7d25f8bd4404f1c67e1 Mon Sep 17 00:00:00 2001 From: Bryan Newbold Date: Tue, 28 Jul 2020 16:50:24 -0700 Subject: search: catch ES errors and display better --- python/fatcat_web/routes.py | 12 +++++++--- python/fatcat_web/search.py | 22 ++++++++++++++---- python/fatcat_web/templates/container_search.html | 2 ++ python/fatcat_web/templates/release_search.html | 2 ++ python/fatcat_web/templates/search_macros.html | 28 ++++++++++++----------- 5 files changed, 46 insertions(+), 20 deletions(-) (limited to 'python/fatcat_web/templates') diff --git a/python/fatcat_web/routes.py b/python/fatcat_web/routes.py index 4a66b3c2..77b5dc55 100644 --- a/python/fatcat_web/routes.py +++ b/python/fatcat_web/routes.py @@ -14,7 +14,7 @@ from fatcat_tools.normal import * from fatcat_web import app, api, auth_api, priv_api, mwoauth, Config from fatcat_web.auth import handle_token_login, handle_logout, load_user, handle_ia_xauth, handle_wmoauth from fatcat_web.cors import crossdomain -from fatcat_web.search import ReleaseQuery, GenericQuery, do_release_search, do_container_search, get_elastic_entity_stats, get_elastic_container_stats, get_elastic_container_histogram +from fatcat_web.search import ReleaseQuery, GenericQuery, do_release_search, do_container_search, get_elastic_entity_stats, get_elastic_container_stats, get_elastic_container_histogram, FatcatSearchError from fatcat_web.entity_helpers import * from fatcat_web.graphics import * from fatcat_web.kafka import * @@ -710,7 +710,10 @@ def release_search(): return render_template('release_search.html', query=ReleaseQuery(), found=None) query = ReleaseQuery.from_args(request.args) - found = do_release_search(query) + try: + found = do_release_search(query) + except FatcatSearchError as fse: + return render_template('release_search.html', query=query, es_error=fse), fse.status_code return render_template('release_search.html', query=query, found=found) @app.route('/container/search', methods=['GET', 'POST']) @@ -720,7 +723,10 @@ def container_search(): return render_template('container_search.html', query=GenericQuery(), found=None) query = GenericQuery.from_args(request.args) - found = do_container_search(query) + try: + found = do_container_search(query) + except FatcatSearchError as fse: + return render_template('container_search.html', query=query, es_error=fse), fse.status_code return render_template('container_search.html', query=query, found=found) def get_changelog_stats(): diff --git a/python/fatcat_web/search.py b/python/fatcat_web/search.py index fe0610e5..3fd7f9dc 100644 --- a/python/fatcat_web/search.py +++ b/python/fatcat_web/search.py @@ -15,6 +15,15 @@ import elasticsearch_dsl.response from fatcat_web import app +class FatcatSearchError(Exception): + + def __init__(self, status_code: int, name: str, description: str = None): + if status_code == "N/A": + status_code = 503 + self.status_code = status_code + self.name = name + self.description = description + @dataclass class ReleaseQuery: q: Optional[str] = None @@ -109,14 +118,19 @@ def wrap_es_execution(search: Search) -> Any: except elasticsearch.exceptions.RequestError as e: # this is a "user" error print("elasticsearch 400: " + str(e.info), file=sys.stderr) + description = None if e.info.get("error", {}).get("root_cause", {}): - raise ValueError(str(e.info["error"]["root_cause"][0].get("reason"))) - else: - raise ValueError(str(e.info)) + description = str(e.info["error"]["root_cause"][0].get("reason")) + raise FatcatSearchError(e.status_code, str(e.error), description) + except elasticsearch.exceptions.ConnectionError as e: + raise FatcatSearchError(e.status_code, "ConnectionError: search engine not available") except elasticsearch.exceptions.TransportError as e: # all other errors print("elasticsearch non-200 status code: {}".format(e.info), file=sys.stderr) - raise IOError(str(e.info)) + description = None + if e.info.get("error", {}).get("root_cause", {}): + description = str(e.info["error"]["root_cause"][0].get("reason")) + raise FatcatSearchError(e.status_code, str(e.error), description) return resp def do_container_search( diff --git a/python/fatcat_web/templates/container_search.html b/python/fatcat_web/templates/container_search.html index ce868991..bd92dc2b 100644 --- a/python/fatcat_web/templates/container_search.html +++ b/python/fatcat_web/templates/container_search.html @@ -80,6 +80,8 @@ {% endif %} +{% elif es_error %} + {{ search_macros.es_error_msg(es_error) }} {% endif %} diff --git a/python/fatcat_web/templates/release_search.html b/python/fatcat_web/templates/release_search.html index e004efc1..7fb475e3 100644 --- a/python/fatcat_web/templates/release_search.html +++ b/python/fatcat_web/templates/release_search.html @@ -74,6 +74,8 @@ {% endif %} +{% elif es_error %} + {{ search_macros.es_error_msg(es_error) }} {% endif %} diff --git a/python/fatcat_web/templates/search_macros.html b/python/fatcat_web/templates/search_macros.html index 383c271c..a207bfbc 100644 --- a/python/fatcat_web/templates/search_macros.html +++ b/python/fatcat_web/templates/search_macros.html @@ -44,23 +44,25 @@ found.count_returned }} out of {{ found.count_found }} results   {% macro es_error_msg(es_error) %}
- {% if es_error.status_code == 400 %} -
-
+
+
+ {% if es_error.status_code == 400 %} Query Error -
+ {% else %} + Search Index Error + {% if es_error.status_code %}({{ es_error.status_code }}){% endif %} + {% endif %} +
+ {% if es_error.description %}

Computer said: {{ es_error.description }} + {% elif es_error.name %} +

{{ es_error.name }} + {% endif %} + {% if es_error.status_code == 400 %}

Query parsing is currently very naive. Sometimes you can fix this problem by adding quotes around terms or entire phrases. -

- {% else %} -
-
- Search Index Error ({{ es_error.status_code }}) -
-

Computer said: {{ es_error.description }} -

- {% endif %} + {% endif %} +
{% endmacro %} -- cgit v1.2.3 From ab80084f1cd90faf5e069edc4203668c81e75d63 Mon Sep 17 00:00:00 2001 From: Bryan Newbold Date: Tue, 28 Jul 2020 16:59:32 -0700 Subject: generic API error page This error handler and view page currently works much better than the "flash()" infrastructure built-in to flask, which uses cookies and mostly does not work with our views and layouts. Would like to gradually migrate almost all API errors in the web interface to just raising errors that get rendered on an error page, instead of calling `abort(ae.status)`. --- python/fatcat_web/routes.py | 30 ++++++++++++++++++++++++++++++ python/fatcat_web/templates/api_error.html | 20 ++++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 python/fatcat_web/templates/api_error.html (limited to 'python/fatcat_web/templates') diff --git a/python/fatcat_web/routes.py b/python/fatcat_web/routes.py index 77b5dc55..a6634292 100644 --- a/python/fatcat_web/routes.py +++ b/python/fatcat_web/routes.py @@ -1005,6 +1005,36 @@ def page_server_error(e): def page_server_down(e): return render_template('503.html'), 503 +@app.errorhandler(ApiException) +def page_fatcat_api_error(ae): + """ + Generic error handler for fatcat API problems. With this error handler, + don't need to explicitly catch API exceptions: they should get caught and + routed correctly here. + """ + if ae.status == 404: + return page_not_found(ae) + elif ae.status in [401, 403]: + return page_not_authorized(ae) + elif ae.status in [405]: + return page_method_not_allowed(ae) + elif ae.status in [409]: + return page_edit_conflict(ae) + try: + json_body = json.loads(ae.body) + ae.error_name = json_body.get('error') + ae.message = json_body.get('message') + except ValueError: + pass + return render_template('api_error.html', api_error=ae), ae.status + +@app.errorhandler(ApiValueError) +def page_fatcat_api_value_error(ae): + ae.status = 400 + ae.error_name = "ValueError" + ae.message = str(ae) + return render_template('api_error.html', api_error=ae), 400 + @app.errorhandler(CSRFError) def page_csrf_error(e): return render_template('csrf_error.html', reason=e.description), 400 diff --git a/python/fatcat_web/templates/api_error.html b/python/fatcat_web/templates/api_error.html new file mode 100644 index 00000000..1a44f610 --- /dev/null +++ b/python/fatcat_web/templates/api_error.html @@ -0,0 +1,20 @@ +{% extends "base.html" %} +{% block body %} + +
+
{{ api_error.status or "" }}
+
{{ api_error.reason or "" }}
+
+ +
+ +
+
+ API Error: {{ api_error.error_name or "" }} +
+

{{ api_error.message or "" }} +

+
+ + +{% endblock %} -- cgit v1.2.3 From 555ea734bb79a4dac23f6b601a5f6a69cb427fab Mon Sep 17 00:00:00 2001 From: Bryan Newbold Date: Tue, 28 Jul 2020 17:28:04 -0700 Subject: error handling: use 400 page with error passed instead of flash() --- python/fatcat_web/editing_routes.py | 6 ++---- python/fatcat_web/routes.py | 20 +++++++------------- python/fatcat_web/templates/400.html | 18 ++++++++++++++---- 3 files changed, 23 insertions(+), 21 deletions(-) (limited to 'python/fatcat_web/templates') diff --git a/python/fatcat_web/editing_routes.py b/python/fatcat_web/editing_routes.py index e66f29c9..ffce35f3 100644 --- a/python/fatcat_web/editing_routes.py +++ b/python/fatcat_web/editing_routes.py @@ -79,8 +79,7 @@ def generic_entity_edit(editgroup_id, entity_type, existing_ident, edit_template # check that editgroup is edit-able if editgroup.changelog_index != None: - flash("Editgroup already merged") - abort(400) + abort(400, "Editgroup already merged") # fetch entity (if set) or 404 existing = None @@ -206,8 +205,7 @@ def generic_edit_delete(editgroup_id, entity_type, edit_id): # check that editgroup is edit-able if editgroup.changelog_index != None: - flash("Editgroup already merged") - abort(400) + abort(400, "Editgroup already merged") # API on behalf of user user_api = auth_api(session['api_token']) diff --git a/python/fatcat_web/routes.py b/python/fatcat_web/routes.py index 71946d14..6f3ec21b 100644 --- a/python/fatcat_web/routes.py +++ b/python/fatcat_web/routes.py @@ -503,8 +503,7 @@ def editgroup_create_annotation(ident): try: eg = user_api.get_editgroup(str(ident)) if eg.changelog_index: - flash("Editgroup already accepted") - abort(400) + abort(400, "Editgroup already accepted") ega = EditgroupAnnotation( comment_markdown=comment_markdown, extra=None, @@ -525,8 +524,7 @@ def editgroup_accept(ident): try: eg = user_api.get_editgroup(str(ident)) if eg.changelog_index: - flash("Editgroup already accepted") - abort(400) + abort(400, "Editgroup already accepted") user_api.accept_editgroup(str(ident)) except ApiException as ae: app.log.info(ae) @@ -543,8 +541,7 @@ def editgroup_unsubmit(ident): try: eg = user_api.get_editgroup(str(ident)) if eg.changelog_index: - flash("Editgroup already accepted") - abort(400) + abort(400, "Editgroup already accepted") user_api.update_editgroup(eg.editgroup_id, eg, submit=False) except ApiException as ae: app.log.info(ae) @@ -561,8 +558,7 @@ def editgroup_submit(ident): try: eg = user_api.get_editgroup(str(ident)) if eg.changelog_index: - flash("Editgroup already accepted") - abort(400) + abort(400, "Editgroup already accepted") user_api.update_editgroup(eg.editgroup_id, eg, submit=True) except ApiException as ae: app.log.info(ae) @@ -762,8 +758,7 @@ def stats_json(): @crossdomain(origin='*',headers=['access-control-allow-origin','Content-Type']) def container_issnl_stats(issnl): if not (len(issnl) == 9 and issnl[4] == '-'): - flash("Not a valid ISSN-L: {}".format(issnl)) - abort(400) + abort(400, "Not a valid ISSN-L: {}".format(issnl)) try: container = api.lookup_container(issnl=issnl) except ApiException as ae: @@ -917,8 +912,7 @@ def create_auth_token(): duration_seconds = int(duration_seconds) assert duration_seconds >= 1 except (ValueError, AssertionError): - flash("duration_seconds must be a positive non-zero integer") - abort(400) + abort(400, "duration_seconds must be a positive non-zero integer") # check user's auth. api_token and editor_id are signed together in session # cookie, so if api_token is valid editor_id is assumed to match. If that @@ -986,7 +980,7 @@ def page_method_not_allowed(e): @app.errorhandler(400) def page_bad_request(e): - return render_template('400.html'), 400 + return render_template('400.html', err=e), 400 @app.errorhandler(409) def page_edit_conflict(e): diff --git a/python/fatcat_web/templates/400.html b/python/fatcat_web/templates/400.html index f2659ca2..21f98aad 100644 --- a/python/fatcat_web/templates/400.html +++ b/python/fatcat_web/templates/400.html @@ -4,10 +4,20 @@
400
Bad Request
- -

Wasn't able to handle the request, either due to incorrect or unexpected -input. Usually more context should be available; if you hit this page it means -you've discovered a new corner case!

+
+ +
+
+ {% if err and err.description %} +

{{ err.description }} + {% else %} +

Wasn't able to handle the request, either due to incorrect or unexpected + input. Usually more context should be available; if you hit this page it means + you've discovered a new corner case! + {% endif %} +

+
+ {% endblock %} -- cgit v1.2.3 From 720ab57430e48d7e84b5dac3b56ffeb67ee87967 Mon Sep 17 00:00:00 2001 From: Bryan Newbold Date: Tue, 28 Jul 2020 18:43:48 -0700 Subject: switch SERP stage coloring to brown uppercase To match fatcat-scholor, where this scheme has been successful. --- python/fatcat_web/templates/entity_macros.html | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) (limited to 'python/fatcat_web/templates') diff --git a/python/fatcat_web/templates/entity_macros.html b/python/fatcat_web/templates/entity_macros.html index ab0e817a..8e4c4f6a 100644 --- a/python/fatcat_web/templates/entity_macros.html +++ b/python/fatcat_web/templates/entity_macros.html @@ -162,12 +162,10 @@ {# release type suffix #} {% if paper.release_type in ("article-journal", "paper-conference") %} {# pass #} - {% elif paper.release_type in ("book", "chapter", "dataset") %} - [{{ paper.release_type }}] {% elif not paper.release_type %} - [unknown-media] + [unknown] {% else %} - [{{ paper.release_type }}] + [{{ paper.release_type }}] {% endif %} {# show original_title #} @@ -212,17 +210,17 @@ {% endif %} {% if paper.container_is_oa %}{% endif %} {% endif %} - {% if paper.withdrawn_status %} - [{{ paper.withdrawn_status }}] - {% endif %} - {% if paper.release_stage == "accepted" %} - [{{ paper.release_stage }} manuscript] - {% elif paper.release_stage == "submitted" %} - [pre-print] + + {% if paper.release_stage == "submitted" %} + pre-print {% elif paper.release_stage and paper.release_stage != "published" %} - [{{ paper.release_stage }}] + {{ paper.release_stage }} version {% elif not paper.release_stage %} - [unpublished?] + unpublished + {% endif %} + + {% if paper.withdrawn_status %} + {{ paper.withdrawn_status }} {% endif %} {# ### IDENTIFIERS #} -- cgit v1.2.3 From 118e16a54bd5cc0db4b24b46a2d5db990f60ea19 Mon Sep 17 00:00:00 2001 From: Bryan Newbold Date: Tue, 28 Jul 2020 18:45:18 -0700 Subject: update front-page counts --- python/fatcat_web/templates/home.html | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'python/fatcat_web/templates') diff --git a/python/fatcat_web/templates/home.html b/python/fatcat_web/templates/home.html index 698230d3..4557e212 100644 --- a/python/fatcat_web/templates/home.html +++ b/python/fatcat_web/templates/home.html @@ -35,17 +35,17 @@
-

106,283,000
Papers

+

110,814,532
Papers
-

23,036,825
Fulltext

+

26,173,743
Fulltext
-

148,757
Journals

+

151,707
Journals
-- cgit v1.2.3