From bce4b1c27e5481fb2b49d90256fe57c41dc09496 Mon Sep 17 00:00:00 2001 From: Fabian Steeg Date: Thu, 20 Sep 2018 11:53:50 +0200 Subject: [PATCH 1/3] Handle errors on invalid queries See https://github.com/hbz/lobid-organisations/issues/414 --- app/controllers/Application.java | 11 +++++++---- app/controllers/Index.java | 9 ++++++++- app/views/search.scala.html | 4 ++-- conf/application.conf | 2 +- 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/app/controllers/Application.java b/app/controllers/Application.java index 6bf1b800..dc5c9041 100644 --- a/app/controllers/Application.java +++ b/app/controllers/Application.java @@ -402,10 +402,10 @@ private static Result searchResult(String q, String location, int from, results.put("js", () -> { String queryResultString = searchQueryResult(q, location, from, size, "location"); - String queryMetadata = - Json.parse(queryResultString).get("aggregation").toString(); - JavaScript script = - views.js.facet_map.render(queryMetadata, q, location, from, size); + JsonNode queryMetadata = Json.parse(queryResultString).get("aggregation"); + JavaScript script = views.js.facet_map.render( + queryMetadata == null ? "{}" : queryMetadata.toString(), q, location, + from, size); return ok(script).as("application/javascript; charset=utf-8"); }); results.put("csv", () -> { @@ -614,6 +614,9 @@ static String[] defaultAggregations() { } private static String returnAsJson(SearchResponse queryResponse) { + if (queryResponse == null) { + return Json.newObject().toString(); + } List> hits = Arrays.asList(queryResponse.getHits().hits()).stream() .map(hit -> hit.getSource()).collect(Collectors.toList()); diff --git a/app/controllers/Index.java b/app/controllers/Index.java index c6cc1de5..be74edd8 100644 --- a/app/controllers/Index.java +++ b/app/controllers/Index.java @@ -149,7 +149,14 @@ static SearchResponse executeQuery(int from, int size, QueryBuilder query, if (!aggregations.isEmpty()) { searchRequest = withAggregations(searchRequest, aggregations.split(",")); } - return searchRequest.execute().actionGet(); + try { + return searchRequest.execute().actionGet(); + } catch (Throwable t) { + Logger.error("Could not execute request: {}, cause: {}", t.getMessage(), + t.getCause().getMessage()); + Logger.debug("Could not execute request", t); + return null; + } } private static QueryBuilder preprocess(QueryBuilder query) { diff --git a/app/views/search.scala.html b/app/views/search.scala.html index 479cf058..8aa1507e 100644 --- a/app/views/search.scala.html +++ b/app/views/search.scala.html @@ -55,7 +55,7 @@

@Messages.get("search." + key.split("\\.")(0))

@defining(Json.parse(json)) { jsonResponse => @main(q, location, facetMap()) { - @defining((jsonResponse\"member").as[Seq[JsValue]]) { orgs => @if(orgs.size > 0) { + @defining((jsonResponse\"member").asOpt[Seq[JsValue]].getOrElse(Seq())) { orgs => @if(orgs.size > 0) {
@Messages.get("search.loading_locations")
@@ -155,5 +155,5 @@

@Messages.get("search.location") @if(!location.isEmpty){ @defining(if(!q.isEmpty) q else "*") { qParam =>

@Html(Messages.get("search.footer.api_text", routes.Application.search(q=qParam, from=from, format="json", location=location), routes.Application.search(q=qParam, from=from, format="csv", location=location), routes.Application.api()))

- }} else { @if(!q.isEmpty) {

@Html(Messages.get("search.footer.no_results", q))

} } } + }} else { @if(!q.isEmpty) {

@Html(Messages.get("search.footer.no_results", q))

} } } }} \ No newline at end of file diff --git a/conf/application.conf b/conf/application.conf index 7bd9e9e1..5584ac47 100644 --- a/conf/application.conf +++ b/conf/application.conf @@ -88,7 +88,7 @@ logger.root=ERROR logger.play=INFO # Logger provided to your application: -logger.application=DEBUG +logger.application=INFO organisation.icons={ "http://purl.org/lobid/libtype#n34" : "music", From 593beadd7e431ab82e43bf3a948d0168b0ebedaa Mon Sep 17 00:00:00 2001 From: Fabian Steeg Date: Thu, 20 Sep 2018 13:01:05 +0200 Subject: [PATCH 2/3] Use logging setup as in the other lobid services See https://github.com/hbz/lobid-organisations/issues/414 --- conf/application.conf | 14 ----------- conf/logback.xml | 50 --------------------------------------- conf/logger.xml | 55 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 64 deletions(-) delete mode 100644 conf/logback.xml create mode 100644 conf/logger.xml diff --git a/conf/application.conf b/conf/application.conf index 5584ac47..285463a2 100644 --- a/conf/application.conf +++ b/conf/application.conf @@ -76,20 +76,6 @@ application.global=Global # # ebean.default="models.*" -# Logger -# ~~~~~ -# You can also configure logback (http://logback.qos.ch/), -# by providing an application-logger.xml file in the conf directory. - -# Root logger: -logger.root=ERROR - -# Logger used by the framework: -logger.play=INFO - -# Logger provided to your application: -logger.application=INFO - organisation.icons={ "http://purl.org/lobid/libtype#n34" : "music", "http://purl.org/lobid/libtype#n39" : "bus", diff --git a/conf/logback.xml b/conf/logback.xml deleted file mode 100644 index 73268459..00000000 --- a/conf/logback.xml +++ /dev/null @@ -1,50 +0,0 @@ - - - - - - - - ${application.home}/logs/application.log - - - ${application.home}/logs/application-log-%d{yyyy-ww}.gz - - 8 - - - %date{yyyy-MM-dd HH:mm:ss ZZZZ} [%level] from %logger in %thread - %message%n%xException - - - - - - %coloredLevel %logger{15} - %message%n%xException{10} - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/conf/logger.xml b/conf/logger.xml new file mode 100644 index 00000000..8f673bc7 --- /dev/null +++ b/conf/logger.xml @@ -0,0 +1,55 @@ + + + + + + + + ./logs/application.log + + + application-log-%d{yyyy-ww}.gz + + 8 + + + %date{yyyy-MM-dd HH:mm:ss ZZZZ} [%level] from %logger in + %thread - %message%n%xException + + + + + + %coloredLevel %logger{15} - %message%n%xException{10} + + + + + + + + + + + + + + + + + + + + + + + + + + + From 4a47650dd46a3820f7f30590f5af7925e03ea0f1 Mon Sep 17 00:00:00 2001 From: Fabian Steeg Date: Thu, 20 Sep 2018 14:28:14 +0200 Subject: [PATCH 3/3] Return proper error code, embed message in error template See https://github.com/hbz/lobid-organisations/issues/414 --- app/controllers/Application.java | 13 +++++++------ app/controllers/Index.java | 9 +-------- app/views/error.scala.html | 12 ++++++++++++ conf/messages.de | 2 ++ conf/messages.en | 2 ++ 5 files changed, 24 insertions(+), 14 deletions(-) create mode 100644 app/views/error.scala.html diff --git a/app/controllers/Application.java b/app/controllers/Application.java index dc5c9041..6a4597ce 100644 --- a/app/controllers/Application.java +++ b/app/controllers/Application.java @@ -354,9 +354,13 @@ location, from, size, responseFormat, lang().code(), Logger.debug("Caching search result for request: {}", cacheKey); Cache.set(cacheKey, searchResult, ONE_DAY); return searchResult; - } catch (IllegalArgumentException x) { - Logger.warn("Bad request: ", x.getMessage()); - return badRequest("Bad request: " + x.getMessage()); + } catch (Throwable t) { + String message = t.getMessage() + + (t.getCause() != null ? ", cause: " + t.getCause().getMessage() + : ""); + Logger.error("Error: {}", message); + return internalServerError( + views.html.error.render(q, "Error: " + message)); } } @@ -614,9 +618,6 @@ static String[] defaultAggregations() { } private static String returnAsJson(SearchResponse queryResponse) { - if (queryResponse == null) { - return Json.newObject().toString(); - } List> hits = Arrays.asList(queryResponse.getHits().hits()).stream() .map(hit -> hit.getSource()).collect(Collectors.toList()); diff --git a/app/controllers/Index.java b/app/controllers/Index.java index be74edd8..c6cc1de5 100644 --- a/app/controllers/Index.java +++ b/app/controllers/Index.java @@ -149,14 +149,7 @@ static SearchResponse executeQuery(int from, int size, QueryBuilder query, if (!aggregations.isEmpty()) { searchRequest = withAggregations(searchRequest, aggregations.split(",")); } - try { - return searchRequest.execute().actionGet(); - } catch (Throwable t) { - Logger.error("Could not execute request: {}, cause: {}", t.getMessage(), - t.getCause().getMessage()); - Logger.debug("Could not execute request", t); - return null; - } + return searchRequest.execute().actionGet(); } private static QueryBuilder preprocess(QueryBuilder query) { diff --git a/app/views/error.scala.html b/app/views/error.scala.html new file mode 100644 index 00000000..d784c6aa --- /dev/null +++ b/app/views/error.scala.html @@ -0,0 +1,12 @@ +@* Copyright 2018 Fabian Steeg, hbz. Licensed under the GPLv2 *@ + +@(q: String, message: String) + +@import play.i18n._ + +@main(q) { +
+
@Messages.get("error")
+
@message
+
+} diff --git a/conf/messages.de b/conf/messages.de index c65edbc2..84bae50f 100644 --- a/conf/messages.de +++ b/conf/messages.de @@ -150,3 +150,5 @@ dataset.distribution.documentation = Dokumentation dataset.distribution.url = URL dataset.distribution.license = Lizenz dataset.distribution.media = Medientypen + +error = Fehler diff --git a/conf/messages.en b/conf/messages.en index 39b4f0ac..200f8505 100644 --- a/conf/messages.en +++ b/conf/messages.en @@ -150,3 +150,5 @@ dataset.distribution.documentation = Documentation dataset.distribution.url = URL dataset.distribution.license = License dataset.distribution.media = Media types + +error = Error