From d8b136807d8d875a38966dd52597bc5a63661272 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20D=C3=BCsing?= Date: Wed, 17 Jun 2020 14:03:51 +0200 Subject: [PATCH 1/2] Better error handling for client-side query errors --- build.sbt | 2 +- .../cs/swt/delphi/webapi/DelphiRoutes.scala | 5 ++++- .../delphi/webapi/search/SearchQuery.scala | 14 ++++++++++---- .../cs/swt/delphi/webapi/search/package.scala | 19 +++++++++++++++++++ 4 files changed, 34 insertions(+), 6 deletions(-) diff --git a/build.sbt b/build.sbt index 4f28c28..e57884b 100644 --- a/build.sbt +++ b/build.sbt @@ -1,6 +1,6 @@ name := "delphi-webapi" -version := "0.9.5.1" +version := "0.9.6-SNAPSHOT" scalaVersion := "2.13.1" diff --git a/src/main/scala/de/upb/cs/swt/delphi/webapi/DelphiRoutes.scala b/src/main/scala/de/upb/cs/swt/delphi/webapi/DelphiRoutes.scala index 683425a..83affe3 100644 --- a/src/main/scala/de/upb/cs/swt/delphi/webapi/DelphiRoutes.scala +++ b/src/main/scala/de/upb/cs/swt/delphi/webapi/DelphiRoutes.scala @@ -29,7 +29,7 @@ import de.upb.cs.swt.delphi.webapi.FeatureJson._ import de.upb.cs.swt.delphi.webapi.IpLogActor._ import de.upb.cs.swt.delphi.webapi.StatisticsJson._ import de.upb.cs.swt.delphi.webapi.search.QueryRequestJson._ -import de.upb.cs.swt.delphi.webapi.search.{QueryRequest, SearchError, SearchQuery} +import de.upb.cs.swt.delphi.webapi.search.{InvalidQueryError, QueryRequest, SearchError, SearchQuery} import spray.json._ import scala.concurrent.duration._ @@ -113,6 +113,9 @@ class DelphiRoutes(requestLimiter: RequestLimitScheduler) extends JsonSupport wi case se: SearchError => { HttpResponse(StatusCodes.ServerError(StatusCodes.InternalServerError.intValue)(se.toJson.toString(), "")) } + case iqe: InvalidQueryError => { + HttpResponse(StatusCodes.ClientError(StatusCodes.BadRequest.intValue)(iqe.toJson.toString(), "")) + } case _ => { HttpResponse(StatusCodes.ServerError(StatusCodes.InternalServerError.intValue)("Search query failed", "")) } diff --git a/src/main/scala/de/upb/cs/swt/delphi/webapi/search/SearchQuery.scala b/src/main/scala/de/upb/cs/swt/delphi/webapi/search/SearchQuery.scala index 64b56a8..1f5fbc9 100644 --- a/src/main/scala/de/upb/cs/swt/delphi/webapi/search/SearchQuery.scala +++ b/src/main/scala/de/upb/cs/swt/delphi/webapi/search/SearchQuery.scala @@ -31,6 +31,7 @@ import scala.io.{Codec, Source} import scala.util.{Failure, Success, Try} import spray.json._ import de.upb.cs.swt.delphi.webapi.FeatureJson._ +import org.parboiled2.{ParseError, Position} class SearchQuery(configuration: Configuration, featureExtractor: FeatureQuery) { @@ -52,7 +53,7 @@ class SearchQuery(configuration: Configuration, featureExtractor: FeatureQuery) val publicFieldNames = internalFeatures.map(i => i.name) val invalidFields = fields.toSet.filter(f => !publicFieldNames.contains(f)) - if (invalidFields.size > 0) return Failure(new IllegalArgumentException(s"Unknown field name(s) used. (${invalidFields.mkString(",")})")) + if (invalidFields.size > 0) return Failure(new InvalidQueryFieldsError(parsedQuery.toString, invalidFields)) val translatedFields = fields.toSet.map(externalToInternalFeature(_)) def getPrefix (in: String) : String = { @@ -80,7 +81,7 @@ class SearchQuery(configuration: Configuration, featureExtractor: FeatureQuery) response match { case RequestSuccess(_, body, _, result) => Success(result.hits) - case r => Failure(new IllegalArgumentException(r.toString)) + case r => Failure(new SearchError(f"Failed to query database: ${r.toString}")) } } @@ -166,8 +167,12 @@ class SearchQuery(configuration: Configuration, featureExtractor: FeatureQuery) val validSize = size.exists(query.limit.getOrElse(defaultFetchSize) <= _) if (validSize) { val parserResult = new Syntax(query.query).QueryRule.run() + parserResult match { - case Failure(e) => Failure(e) + case Failure(ParseError(Position(_, line, column), _, _)) => + Failure(new InvalidQueryError(f"Syntax error in query (near line $line, colum $column)", query.query)) + case Failure(e) => + Failure(e) case Success(parsedQuery) => { checkAndExecuteParsedQuery(parsedQuery, query.limit.getOrElse(defaultFetchSize)) match { case Failure(e) => Failure(e) @@ -177,7 +182,8 @@ class SearchQuery(configuration: Configuration, featureExtractor: FeatureQuery) } } else { - val errorMsg = new SearchError(s"Query limit exceeded default limit: ${query.limit}>${size}") + val errorMsg = new InvalidQueryError( + s"Query limit exceeded default limit: ${query.limit.getOrElse(defaultFetchSize)} > ${size.getOrElse("None")}", query.query) Failure(errorMsg) } } diff --git a/src/main/scala/de/upb/cs/swt/delphi/webapi/search/package.scala b/src/main/scala/de/upb/cs/swt/delphi/webapi/search/package.scala index e0b8189..42dc8cc 100644 --- a/src/main/scala/de/upb/cs/swt/delphi/webapi/search/package.scala +++ b/src/main/scala/de/upb/cs/swt/delphi/webapi/search/package.scala @@ -24,10 +24,29 @@ package object search { class SearchError(msg: String) extends RuntimeException(msg) with JsonSupport + class InvalidQueryError(val msg:String, val query: String) extends RuntimeException(msg) with JsonSupport + + class InvalidQueryFieldsError(override val query: String, val invalidFields: Set[String]) + extends InvalidQueryError("Query contained invalid query field names!", query) + implicit val searchErrorWriter = new JsonWriter[SearchError] { override def write(obj: SearchError): JsValue = { JsObject("msg" -> JsString(obj.getMessage)) } } + implicit val invalidQueryErrorWriter = new JsonWriter[InvalidQueryError] { + override def write(obj: InvalidQueryError): JsValue = { + + obj match { + case error: InvalidQueryFieldsError => + JsObject("msg" -> JsString(obj.getMessage), "query" -> JsString(obj.query), + "invalid_fields" -> JsArray(error.invalidFields.toVector.map(JsString(_)))) + case _ => + JsObject("msg" -> JsString(obj.getMessage), "query" -> JsString(obj.query)) + } + + } + } + } From c65971e47261587c1b84389e0f3701446e3b5578 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20D=C3=BCsing?= Date: Thu, 18 Jun 2020 12:28:26 +0200 Subject: [PATCH 2/2] Fixed spelling error in error message. Fixed improper query string in error message --- .../de/upb/cs/swt/delphi/webapi/search/SearchQuery.scala | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/scala/de/upb/cs/swt/delphi/webapi/search/SearchQuery.scala b/src/main/scala/de/upb/cs/swt/delphi/webapi/search/SearchQuery.scala index 1f5fbc9..bee7536 100644 --- a/src/main/scala/de/upb/cs/swt/delphi/webapi/search/SearchQuery.scala +++ b/src/main/scala/de/upb/cs/swt/delphi/webapi/search/SearchQuery.scala @@ -170,11 +170,14 @@ class SearchQuery(configuration: Configuration, featureExtractor: FeatureQuery) parserResult match { case Failure(ParseError(Position(_, line, column), _, _)) => - Failure(new InvalidQueryError(f"Syntax error in query (near line $line, colum $column)", query.query)) + Failure(new InvalidQueryError(f"Syntax error in query (near line $line, column $column)", query.query)) case Failure(e) => Failure(e) case Success(parsedQuery) => { checkAndExecuteParsedQuery(parsedQuery, query.limit.getOrElse(defaultFetchSize)) match { + case Failure(e) if e.isInstanceOf[InvalidQueryFieldsError] => + // Required to set a valid 'query' attribute to the error + Failure(new InvalidQueryFieldsError(query.query, e.asInstanceOf[InvalidQueryFieldsError].invalidFields)) case Failure(e) => Failure(e) case Success(hits) => Success(ArtifactTransformer.transformResults(hits)) }