From a47096ebebcc458e01063fac6a01827eeb5312c1 Mon Sep 17 00:00:00 2001 From: Balduin Landolt <33053745+BalduinLandolt@users.noreply.github.com> Date: Tue, 26 Apr 2022 11:34:52 +0200 Subject: [PATCH] feat(error-handling): return status 504 instead of 500 for triplestore timeout exception (DEV-749) (#2046) * feat: return 504 for triplestore timeout exception * docs: document new timeout error code * feat: more error handling to get timeouts * refactor: remove unnecessary log * feat: ensure timeout is correctly handled even if fuseki aborts during return of resources --- docs/03-apis/api-v2/query-language.md | 2 + .../webapi/http/status/ApiStatusCodesV2.scala | 7 +- .../http/HttpTriplestoreConnector.scala | 64 ++++++++++++++----- 3 files changed, 54 insertions(+), 19 deletions(-) diff --git a/docs/03-apis/api-v2/query-language.md b/docs/03-apis/api-v2/query-language.md index 22de78b824..d41a36978c 100644 --- a/docs/03-apis/api-v2/query-language.md +++ b/docs/03-apis/api-v2/query-language.md @@ -73,6 +73,8 @@ HTTP POST to http://host/v2/searchextended/count The response to a count query request is an object with one predicate, `http://schema.org/numberOfItems`, with an integer value. +If a gravsearch query times out, a `504 Gateway Timeout` will be returned. + ## Gravsearch and API Schemas A Gravsearch query can be written in either of the two diff --git a/webapi/src/main/scala/org/knora/webapi/http/status/ApiStatusCodesV2.scala b/webapi/src/main/scala/org/knora/webapi/http/status/ApiStatusCodesV2.scala index f92761d1c8..b9be8200ec 100644 --- a/webapi/src/main/scala/org/knora/webapi/http/status/ApiStatusCodesV2.scala +++ b/webapi/src/main/scala/org/knora/webapi/http/status/ApiStatusCodesV2.scala @@ -31,9 +31,10 @@ object ApiStatusCodesV2 { case RequestRejectedException(_) => StatusCodes.BadRequest // Subclasses of InternalServerException (which must be last in this group) - case UpdateNotPerformedException(_) => StatusCodes.Conflict - case InternalServerException(_) => StatusCodes.InternalServerError - case _ => StatusCodes.InternalServerError + case UpdateNotPerformedException(_) => StatusCodes.Conflict + case TriplestoreTimeoutException(_, _) => StatusCodes.GatewayTimeout + case InternalServerException(_) => StatusCodes.InternalServerError + case _ => StatusCodes.InternalServerError } } diff --git a/webapi/src/main/scala/org/knora/webapi/store/triplestore/http/HttpTriplestoreConnector.scala b/webapi/src/main/scala/org/knora/webapi/store/triplestore/http/HttpTriplestoreConnector.scala index 77fc37d29c..0d7be38513 100644 --- a/webapi/src/main/scala/org/knora/webapi/store/triplestore/http/HttpTriplestoreConnector.scala +++ b/webapi/src/main/scala/org/knora/webapi/store/triplestore/http/HttpTriplestoreConnector.scala @@ -235,11 +235,22 @@ class HttpTriplestoreConnector extends Actor with ActorLogging with Instrumentat parseTry match { case Success(parsed) => Success(parsed) case Failure(e) => - log.error( - e, - s"Couldn't parse response from triplestore:$logDelimiter$resultStr${logDelimiter}in response to SPARQL query:$logDelimiter$sparql" - ) - Failure(TriplestoreResponseException("Couldn't parse JSON from triplestore", e, log)) + if (resultStr.contains("## Query cancelled due to timeout during execution")) { + log.error(e, "Triplestore timed out while sending a response, after sending statuscode 200.") + Failure( + TriplestoreTimeoutException( + "Triplestore timed out while sending a response, after sending statuscode 200.", + e, + log + ) + ) + } else { + log.error( + e, + s"Couldn't parse response from triplestore:$logDelimiter$resultStr${logDelimiter}in response to SPARQL query:$logDelimiter$sparql" + ) + Failure(TriplestoreResponseException("Couldn't parse Turtle from triplestore", e, log)) + } } } @@ -303,11 +314,22 @@ class HttpTriplestoreConnector extends Actor with ActorLogging with Instrumentat parseTry match { case Success(parsed) => Success(parsed) case Failure(e) => - log.error( - e, - s"Couldn't parse response from triplestore:$logDelimiter$turtleStr${logDelimiter}in response to SPARQL query:$logDelimiter$sparql" - ) - Failure(TriplestoreResponseException("Couldn't parse Turtle from triplestore", e, log)) + if (turtleStr.contains("## Query cancelled due to timeout during execution")) { + log.error(e, "Triplestore timed out while sending a response, after sending statuscode 200.") + Failure( + TriplestoreTimeoutException( + "Triplestore timed out while sending a response, after sending statuscode 200.", + e, + log + ) + ) + } else { + log.error( + e, + s"Couldn't parse response from triplestore:$logDelimiter$turtleStr${logDelimiter}in response to SPARQL query:$logDelimiter$sparql" + ) + Failure(TriplestoreResponseException("Couldn't parse Turtle from triplestore", e, log)) + } } } @@ -381,9 +403,16 @@ class HttpTriplestoreConnector extends Actor with ActorLogging with Instrumentat } yield response parseTry match { - case Success(parsed) => Success(parsed) + case Success(parsed) => Success(parsed) + case Failure(timeout: TriplestoreTimeoutException) => Failure(timeout) case Failure(e) => - Failure(TriplestoreResponseException("Couldn't parse Turtle from triplestore", e, log)) + Failure( + TriplestoreResponseException( + s"Couldn't parse Turtle from triplestore: ${sparqlExtendedConstructRequest}", + e, + log + ) + ) } } @@ -915,10 +944,11 @@ class HttpTriplestoreConnector extends Actor with ActorLogging with Instrumentat Option(response.getEntity) .map(responseEntity => EntityUtils.toString(responseEntity, StandardCharsets.UTF_8)) match { case Some(responseEntityStr) => - log.error(s"Triplestore responded with HTTP code $statusCode: $responseEntityStr") - throw TriplestoreResponseException( - s"Triplestore responded with HTTP code $statusCode: $responseEntityStr" - ) + val msg = s"Triplestore responded with HTTP code $statusCode: $responseEntityStr" + log.error(msg) + if (statusCode == 503 && responseEntityStr.contains("Query timed out")) + throw TriplestoreTimeoutException(msg) + else throw TriplestoreResponseException(msg) case None => log.error(s"Triplestore responded with HTTP code $statusCode") @@ -945,6 +975,8 @@ class HttpTriplestoreConnector extends Actor with ActorLogging with Instrumentat log.error(socketTimeoutException, message) throw TriplestoreTimeoutException(message = message, e = socketTimeoutException, log = log) + case timeout: TriplestoreTimeoutException => throw timeout + case notFound: NotFoundException => throw notFound case e: Exception =>