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.

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

[JSON-API] Fix logging of internal server errors #12822

Merged
merged 1 commit into from
Feb 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@ class Endpoints(
import util.ErrorOps._

private def responseToRoute(res: Future[HttpResponse]): Route = _ => res map Complete
private def toRoute[T: MkHttpResponse](res: => T): Route =
private def toRoute[T: MkHttpResponse](res: => T)(implicit
lc: LoggingContextOf[InstanceUUID with RequestID]
): Route =
responseToRoute(httpResponse(res))

private def mkRequestLogMsg(request: HttpRequest, remoteAddress: RemoteAddress) =
Expand Down Expand Up @@ -383,19 +385,24 @@ class Endpoints(
.fromFunction((_: E \/ A).leftMap(E.run))
.recover(logException("Source") andThen Error.fromThrowable andThen (-\/(_)))

private def httpResponse[T](output: T)(implicit T: MkHttpResponse[T]): Future[HttpResponse] =
private def httpResponse[T](output: T)(implicit
T: MkHttpResponse[T],
lc: LoggingContextOf[InstanceUUID with RequestID],
): Future[HttpResponse] =
T.run(output)
.recover(Error.fromThrowable andThen (httpResponseError(_)))

private implicit def sourceStreamSearchResults[A: JsonWriter]
: MkHttpResponse[ET[domain.SyncResponse[Source[Error \/ A, NotUsed]]]] =
private implicit def sourceStreamSearchResults[A: JsonWriter](implicit
lc: LoggingContextOf[InstanceUUID with RequestID]
): MkHttpResponse[ET[domain.SyncResponse[Source[Error \/ A, NotUsed]]]] =
MkHttpResponse { output =>
implicitly[MkHttpResponse[Future[Error \/ SearchResult[Error \/ JsValue]]]]
.run(output.map(_ map (_ map (_ map ((_: A).toJson)))).run)
}

private implicit def searchResults
: MkHttpResponse[Future[Error \/ SearchResult[Error \/ JsValue]]] =
private implicit def searchResults(implicit
lc: LoggingContextOf[InstanceUUID with RequestID]
): MkHttpResponse[Future[Error \/ SearchResult[Error \/ JsValue]]] =
MkHttpResponse { output =>
output.map(_.fold(httpResponseError, searchHttpResponse))
}
Expand Down Expand Up @@ -426,7 +433,8 @@ class Endpoints(
}

private implicit def fullySync[A: JsonWriter](implicit
metrics: Metrics
metrics: Metrics,
lc: LoggingContextOf[InstanceUUID with RequestID],
): MkHttpResponse[ET[domain.SyncResponse[A]]] = MkHttpResponse { result =>
Timed.future(
metrics.daml.HttpJsonApi.responseCreationTimer,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import akka.http.scaladsl.server.{RequestContext, Route}
import akka.util.ByteString
import com.daml.http.domain.{JwtPayload, JwtPayloadLedgerIdOnly, JwtWritePayload}
import com.daml.http.json.SprayJson
import com.daml.http.util.Logging.{InstanceUUID, RequestID}
import com.daml.http.util.Logging.{InstanceUUID, RequestID, extendWithRequestIdLogCtx}
import util.GrpcHttpErrorCodes._
import com.daml.jwt.domain.{DecodedJwt, Jwt}
import com.daml.ledger.api.auth.{AuthServiceJWTCodec, CustomDamlJWTPayload, StandardJWTPayload}
Expand Down Expand Up @@ -290,27 +290,35 @@ object EndpointsCompanion {
}
}

lazy val notFound: Route = (ctx: RequestContext) =>
def notFound(implicit lc: LoggingContextOf[InstanceUUID]): Route = (ctx: RequestContext) =>
ctx.request match {
case HttpRequest(method, uri, _, _, _) =>
Future.successful(
Complete(httpResponseError(NotFound(s"${method: HttpMethod}, uri: ${uri: Uri}")))
extendWithRequestIdLogCtx(implicit lc =>
Future.successful(
Complete(httpResponseError(NotFound(s"${method: HttpMethod}, uri: ${uri: Uri}")))
)
)
}

private[http] def httpResponseError(error: Error): HttpResponse = {
private[http] def httpResponseError(
error: Error
)(implicit lc: LoggingContextOf[InstanceUUID with RequestID]): HttpResponse = {
import com.daml.http.json.JsonProtocol._
val resp = errorResponse(error)
httpResponse(resp.status, SprayJson.encodeUnsafe(resp))
}
private[this] val logger = ContextualizedLogger.get(getClass)

private[http] def errorResponse(error: Error): domain.ErrorResponse = {
private[http] def errorResponse(
error: Error
)(implicit lc: LoggingContextOf[InstanceUUID with RequestID]): domain.ErrorResponse = {
val (status, errorMsg): (StatusCode, String) = error match {
case InvalidUserInput(e) => StatusCodes.BadRequest -> e
case ParticipantServerError(grpcStatus, d) =>
grpcStatus.asAkkaHttpForJsonApi -> s"$grpcStatus${d.cata((": " + _), "")}"
case ServerError(_) => StatusCodes.InternalServerError -> "HTTP JSON API Server Error"
case ServerError(reason) =>
logger.error(s"Internal server error occured: $reason")
StatusCodes.InternalServerError -> "HTTP JSON API Server Error"
case Unauthorized(e) => StatusCodes.Unauthorized -> e
case NotFound(e) => StatusCodes.NotFound -> e
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ import Liskov.<~<
import com.daml.http.domain.TemplateId.toLedgerApiValue
import com.daml.http.domain.TemplateId.{OptionalPkg, RequiredPkg}
import com.daml.http.util.FlowUtil.allowOnlyFirstInput
import com.daml.http.util.Logging.InstanceUUID
import com.daml.http.util.Logging.{InstanceUUID, RequestID, extendWithRequestIdLogCtx}
import com.daml.lf.crypto.Hash
import com.daml.logging.{ContextualizedLogger, LoggingContextOf}
import com.daml.metrics.Metrics
Expand Down Expand Up @@ -697,7 +697,9 @@ class WebSocketService(
}.valueOr(e => Source.single(-\/(e))): Source[Error \/ Message, NotUsed],
)
.takeWhile(_.isRight, inclusive = true) // stop after emitting 1st error
.map(_.fold(e => wsErrorMessage(e), identity): Message)
.map(
_.fold(e => extendWithRequestIdLogCtx(implicit lc1 => wsErrorMessage(e)), identity): Message
)
}

private def parseJson(x: Message): Future[InvalidUserInput \/ JsValue] = x match {
Expand Down Expand Up @@ -931,7 +933,9 @@ class WebSocketService(
.collect { case (_, Some(x)) => x }
}

private[http] def wsErrorMessage(error: Error): TextMessage.Strict =
private[http] def wsErrorMessage(error: Error)(implicit
lc: LoggingContextOf[InstanceUUID with RequestID]
): TextMessage.Strict =
wsMessage(SprayJson.encodeUnsafe(errorResponse(error)))

private[http] def wsMessage(jsVal: JsValue): TextMessage.Strict =
Expand Down