Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(error-handling): return status 504 instead of 500 for triplestore timeout exception (DEV-749) #2046

Merged
merged 6 commits into from Apr 26, 2022
Merged

feat(error-handling): return status 504 instead of 500 for triplestore timeout exception (DEV-749) #2046

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
de7282d
feat: return 504 for triplestore timeout exception
BalduinLandolt Apr 21, 2022
e0ca005
docs: document new timeout error code
BalduinLandolt Apr 21, 2022
bebf6bd
feat: more error handling to get timeouts
BalduinLandolt Apr 21, 2022
d43f400
refactor: remove unnecessary log
BalduinLandolt Apr 25, 2022
f9542d9
feat: ensure timeout is correctly handled even if fuseki aborts durin…
BalduinLandolt Apr 26, 2022
8fee133
Merge branch 'main' into wip/DEV-749-return-specific-statuscode-for-t…
BalduinLandolt Apr 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/03-apis/api-v2/query-language.md
View file
Open in desktop
Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions webapi/src/main/scala/org/knora/webapi/http/status/ApiStatusCodesV2.scala
View file
Open in desktop
Expand Up @@ -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
}

}
64 changes: 48 additions & 16 deletions webapi/src/main/scala/org/knora/webapi/store/triplestore/http/HttpTriplestoreConnector.scala
View file
Open in desktop
Expand Up @@ -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))
}
}
}

Expand Down Expand Up @@ -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"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does logDelimiter once is wrapped into ${...} and once not? Can it be unified? I think both $... and ${...} are the same.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're right, that's just how it is in the place where I copy-pasted it from... ;) I'll unify it

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, no... this can't be changed because right after the variable there is a word, so it would consider this part of the variable name, which then could not be found... I'll have to leave it as it is, even though it looks strange

)
Failure(TriplestoreResponseException("Couldn't parse Turtle from triplestore", e, log))
}
}
}

Expand Down Expand Up @@ -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
)
)
}
}

Expand Down Expand Up @@ -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")
Expand All @@ -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 =>
Expand Down