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

feat: Handle Error in Background Jobs - Improve the way we store errors and defects in DB #1218

Merged
merged 3 commits into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
5 changes: 3 additions & 2 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ inThisBuild(
"-unchecked",
"-Dquill.macro.log=false", // disable quill macro logs
"-Wunused:all",
"-Wconf:any:warning" // TODO: change unused imports to errors, Wconf configuration string is different from scala 2, figure out how!
"-Wconf:any:warning", // TODO: change unused imports to errors, Wconf configuration string is different from scala 2, figure out how!
// TODO "-feature",
// TODO "-Xfatal-warnings",
// TODO "-Yexplicit-nulls",
Expand Down Expand Up @@ -502,7 +502,8 @@ lazy val models = project
), // TODO try to remove this from this module
// libraryDependencies += D.didScala
)
.settings(libraryDependencies += D.nimbusJwt) //FIXME just for the DidAgent
.settings(libraryDependencies += D.nimbusJwt) // FIXME just for the DidAgent
.dependsOn(shared)

/* TODO move code from agentDidcommx to here
models implementation for didcommx () */
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,40 @@
package org.hyperledger.identus.agent.server.jobs

import org.hyperledger.identus.mercury.HttpResponse
import org.hyperledger.identus.shared.models._

sealed trait BackgroundJobError
sealed trait BackgroundJobError(
override val statusCode: org.hyperledger.identus.shared.models.StatusCode,
override val userFacingMessage: String
) extends Failure {
override val namespace: String = "BackgroundJobError"
}

object BackgroundJobError {
final case class InvalidState(cause: String) extends BackgroundJobError

final case class ErrorSendingMessage(cause: String) extends BackgroundJobError
case object NotImplemented extends BackgroundJobError
final case class ErrorResponseReceivedFromPeerAgent(response: HttpResponse) extends BackgroundJobError {
final case class InvalidState(cause: String)
extends BackgroundJobError(
statusCode = StatusCode.InternalServerError,
userFacingMessage = s"Invalid State: cause='$cause'"
)

final case class ErrorSendingMessage(cause: String)
extends BackgroundJobError(
statusCode = StatusCode.BadRequest,
userFacingMessage = s"ErrorSendingMessage"
)

case object NotImplemented
extends BackgroundJobError(
statusCode = StatusCode.UnexpectedNotImplemented,
userFacingMessage = s"NotImplemented"
)
final case class ErrorResponseReceivedFromPeerAgent(response: HttpResponse)
extends BackgroundJobError(
statusCode = StatusCode.BadRequest,
userFacingMessage = s"DIDComm sending error: [${response.status}] - ${response.bodyAsString}"
) {

override def toString: String = s"DIDComm sending error: [${response.status}] - ${response.bodyAsString}"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,13 @@ import org.hyperledger.identus.agent.walletapi.model.error.DIDSecretStorageError
import org.hyperledger.identus.agent.walletapi.model.error.DIDSecretStorageError.{KeyNotFoundError, WalletNotFoundError}
import org.hyperledger.identus.agent.walletapi.service.ManagedDIDService
import org.hyperledger.identus.agent.walletapi.storage.DIDNonSecretStorage
import org.hyperledger.identus.connect.core.model.error.ConnectionServiceError.InvalidStateForOperation
import org.hyperledger.identus.connect.core.model.error.ConnectionServiceError.RecordIdNotFound
import org.hyperledger.identus.connect.core.model.ConnectionRecord
import org.hyperledger.identus.connect.core.model.ConnectionRecord.*
import org.hyperledger.identus.connect.core.service.ConnectionService
import org.hyperledger.identus.mercury.*
import org.hyperledger.identus.mercury.model.error.SendMessageError
import org.hyperledger.identus.resolvers.DIDResolver
import org.hyperledger.identus.shared.models.WalletAccessContext
import org.hyperledger.identus.shared.utils.aspects.CustomMetricsAspect
Expand Down Expand Up @@ -182,11 +185,11 @@ object ConnectBackgroundJobs extends BackgroundJobsHelper {
s"Connect - Error processing record: ${record.id}",
Cause.fail(walletNotFound)
)
case ((walletAccessContext, e)) =>
case ((walletAccessContext, errorResponse)) =>
for {
connectService <- ZIO.service[ConnectionService]
_ <- connectService
.reportProcessingFailure(record.id, Some(e.toString))
.reportProcessingFailure(record.id, Some(errorResponse))
.provideSomeLayer(ZLayer.succeed(walletAccessContext))
} yield ()
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import org.hyperledger.identus.mercury.protocol.issuecredential.*
import org.hyperledger.identus.pollux.core.model.*
import org.hyperledger.identus.pollux.core.model.error.CredentialServiceError
import org.hyperledger.identus.pollux.core.service.CredentialService
import org.hyperledger.identus.shared.models.Failure
import org.hyperledger.identus.shared.utils.aspects.CustomMetricsAspect
import org.hyperledger.identus.shared.utils.DurationOps.toMetricsSeconds
import zio.*
Expand Down Expand Up @@ -618,11 +619,12 @@ object IssueBackgroundJobs extends BackgroundJobsHelper {
case walletNotFound: WalletNotFoundError => ZIO.unit
case CredentialServiceError.RecordNotFound(_, _) => ZIO.unit
case CredentialServiceError.UnsupportedDidFormat(_) => ZIO.unit
case ((walletAccessContext, e)) =>
case failure: Failure => ??? // FIXME
case ((walletAccessContext, failure)) =>
for {
credentialService <- ZIO.service[CredentialService]
_ <- credentialService
.reportProcessingFailure(record.id, Some(e.toString))
.reportProcessingFailure(record.id, Some(failure))
.provideSomeLayer(ZLayer.succeed(walletAccessContext))
} yield ()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,23 @@ import org.hyperledger.identus.pollux.sdjwt.{HolderPrivateKey, IssuerPublicKey,
import org.hyperledger.identus.pollux.vc.jwt.{DidResolver as JwtDidResolver, Issuer as JwtIssuer, JWT, JwtPresentation}
import org.hyperledger.identus.resolvers.DIDResolver
import org.hyperledger.identus.shared.http.*
import org.hyperledger.identus.shared.models.WalletAccessContext
import org.hyperledger.identus.shared.models.*
import org.hyperledger.identus.shared.utils.aspects.CustomMetricsAspect
import org.hyperledger.identus.shared.utils.DurationOps.toMetricsSeconds
import zio.*
import zio.json.*
import zio.json.ast.Json
import zio.metrics.*
import zio.prelude.Validation
import zio.prelude.ZValidation.*
import zio.prelude.ZValidation.{Failure => ZFailure, *}

import java.time.{Clock, Instant, ZoneId}

object PresentBackgroundJobs extends BackgroundJobsHelper {
type ERROR = DIDSecretStorageError | PresentationError | CredentialServiceError | BackgroundJobError |
CastorDIDResolutionError | GetManagedDIDError | TransportError

private type ERROR =
/*DIDSecretStorageError | PresentationError | CredentialServiceError | BackgroundJobError | TransportError | */
CastorDIDResolutionError | GetManagedDIDError | Failure

private type RESOURCES = COMMON_RESOURCES & CredentialService & JwtDidResolver & DIDService & AppConfig &
MESSAGING_RESOURCES
Expand Down Expand Up @@ -82,7 +84,7 @@ object PresentBackgroundJobs extends BackgroundJobsHelper {
aux(record)
.tapError({
(error: PresentationError | DIDSecretStorageError | BackgroundJobError | CredentialServiceError |
CastorDIDResolutionError | GetManagedDIDError | TransportError) =>
CastorDIDResolutionError | GetManagedDIDError | TransportError | Failure) =>
ZIO.logErrorCause(
s"Present Proof - Error processing record: ${record.id}",
Cause.fail(error)
Expand Down Expand Up @@ -533,7 +535,7 @@ object PresentBackgroundJobs extends BackgroundJobsHelper {
requestPresentation: RequestPresentation
): ZIO[
PresentationService & DIDNonSecretStorage,
PresentationError | DIDSecretStorageError | BackgroundJobError,
Failure,
Unit
] = {
maybeCredentialsToUseJson match {
Expand Down Expand Up @@ -645,7 +647,7 @@ object PresentBackgroundJobs extends BackgroundJobsHelper {

def handlePresentationGenerated(id: DidCommID, presentation: Presentation): ZIO[
JwtDidResolver & COMMON_RESOURCES & MESSAGING_RESOURCES,
PresentationError | DIDSecretStorageError | BackgroundJobError | TransportError,
Failure,
Unit
] = {

Expand Down Expand Up @@ -712,7 +714,7 @@ object PresentBackgroundJobs extends BackgroundJobsHelper {

def handleRequestPending(id: DidCommID, record: RequestPresentation): ZIO[
JwtDidResolver & COMMON_RESOURCES & MESSAGING_RESOURCES,
PresentationError | DIDSecretStorageError | BackgroundJobError | TransportError,
Failure,
Unit
] = {
val VerifierSendPresentationRequestMsgSuccess = counterMetric(
Expand Down Expand Up @@ -800,7 +802,7 @@ object PresentBackgroundJobs extends BackgroundJobsHelper {
credentialFormat: CredentialFormat
): ZIO[
AppConfig & JwtDidResolver & COMMON_RESOURCES & MESSAGING_RESOURCES,
PresentationError | DIDSecretStorageError | TransportError,
Failure,
Unit
] = {
val result =
Expand All @@ -814,7 +816,7 @@ object PresentBackgroundJobs extends BackgroundJobsHelper {

private def handleJWT(id: DidCommID, requestPresentation: RequestPresentation, presentation: Presentation): ZIO[
AppConfig & JwtDidResolver & COMMON_RESOURCES & MESSAGING_RESOURCES,
PresentationError | DIDSecretStorageError | TransportError,
Failure,
Unit
] = {
val clock = java.time.Clock.system(ZoneId.systemDefault)
Expand Down Expand Up @@ -887,8 +889,8 @@ object PresentBackgroundJobs extends BackgroundJobsHelper {
case any => ZIO.fail(PresentationReceivedError("Only Base64 Supported"))
}
_ <- credentialsClaimsValidationResult match
case l @ Failure(_, _) => ZIO.logError(s"CredentialsClaimsValidationResult: $l")
case l @ Success(_, _) => ZIO.logInfo(s"CredentialsClaimsValidationResult: $l")
case l @ ZFailure(_, _) => ZIO.logError(s"CredentialsClaimsValidationResult: $l")
case l @ Success(_, _) => ZIO.logInfo(s"CredentialsClaimsValidationResult: $l")
service <- ZIO.service[PresentationService]
presReceivedToProcessedAspect = CustomMetricsAspect.endRecordingTime(
s"${id}_present_proof_flow_verifier_presentation_received_to_verification_success_or_failure_ms_gauge",
Expand All @@ -899,7 +901,7 @@ object PresentBackgroundJobs extends BackgroundJobsHelper {
service
.markPresentationVerified(id)
.provideSomeLayer(ZLayer.succeed(walletAccessContext)) @@ presReceivedToProcessedAspect
case Failure(log, error) =>
case ZFailure(log, error) =>
for {
_ <- service
.markPresentationVerificationFailed(id)
Expand Down Expand Up @@ -928,7 +930,7 @@ object PresentBackgroundJobs extends BackgroundJobsHelper {

private def handleSDJWT(id: DidCommID, presentation: Presentation): ZIO[
JwtDidResolver & COMMON_RESOURCES & MESSAGING_RESOURCES,
PresentationError | DIDSecretStorageError | TransportError,
Failure,
Unit
] = {
for {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,49 +3,39 @@ package org.hyperledger.identus.didcomm.controller
import org.hyperledger.identus.mercury.model.DidId
import org.hyperledger.identus.shared.models.{Failure, StatusCode}

sealed trait DIDCommControllerError(
val statusCode: StatusCode,
val userFacingMessage: String
) extends Failure {
override val namespace = "DIDCommControllerError"
sealed trait DIDCommControllerError extends Failure {
override def namespace = "DIDCommControllerError"
Copy link
Contributor

Choose a reason for hiding this comment

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

Any particular reason why you changed the previous construct and the below case class declaration? If not, I propose to stay consistent with the rest of the codebase.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To make final case class RecipientNotFoundError() an object

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's keep it as a case class, then. I don't think it is worth the trouble. Who knows, maybe we will need to some kind of message ID as a parameter in the future.

}

object DIDCommControllerError {
final case class InvalidContentType(maybeContentType: Option[String])
extends DIDCommControllerError(
StatusCode.BadRequest,
maybeContentType match
case Some(value) => s"The 'content-type' request header value is invalid: $value"
case None => s"The 'content-type' request header is undefined"
)

final case class RecipientNotFoundError()
extends DIDCommControllerError(
StatusCode.UnprocessableContent,
"Recipient not found in the DIDComm Message"
)

final case class UnexpectedError(override val statusCode: StatusCode)
extends DIDCommControllerError(
statusCode,
"An unexpected error occurred while processing your request"
)

final case class RequestBodyParsingError(cause: String)
extends DIDCommControllerError(
StatusCode.BadRequest,
s"Unable to parse the request body as a valid DIDComm message: $cause"
)

final case class PeerDIDNotFoundError(did: DidId)
extends DIDCommControllerError(
StatusCode.UnprocessableContent,
s"The Peer DID was not found in this agent: ${did.value}"
)

final case class PeerDIDKeyNotFoundError(did: DidId, keyId: String)
extends DIDCommControllerError(
StatusCode.UnprocessableContent,
s"The Peer DID does not contain the required key: DID=$did, keyId=$keyId"
)
final case class InvalidContentType(maybeContentType: Option[String]) extends DIDCommControllerError {
override def statusCode: StatusCode = StatusCode.BadRequest
override def userFacingMessage: String = maybeContentType match
case Some(value) => s"The 'content-type' request header value is invalid: $value"
case None => s"The 'content-type' request header is undefined"
}

object RecipientNotFoundError extends DIDCommControllerError {
override def statusCode: StatusCode = StatusCode.UnprocessableContent
override def userFacingMessage: String = "Recipient not found in the DIDComm Message"
}

final case class UnexpectedError(statusCode: StatusCode) extends DIDCommControllerError {
override def userFacingMessage: String = "An unexpected error occurred while processing your request"
}

final case class RequestBodyParsingError(cause: String) extends DIDCommControllerError {
override def statusCode: StatusCode = StatusCode.BadRequest
override def userFacingMessage: String = s"Unable to parse the request body as a valid DIDComm message: $cause"
}

final case class PeerDIDNotFoundError(did: DidId) extends DIDCommControllerError {
override def statusCode: StatusCode = StatusCode.UnprocessableContent
override def userFacingMessage: String = s"The Peer DID was not found in this agent: ${did.value}"
}

final case class PeerDIDKeyNotFoundError(did: DidId, keyId: String) extends DIDCommControllerError {
override def statusCode: StatusCode = StatusCode.UnprocessableContent
override def userFacingMessage: String = s"The Peer DID does not contain the required key: DID=$did, keyId=$keyId"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class DIDCommControllerImpl(
for {
recipientDid <- ZIO
.fromOption(msg.recipients.headOption.map(_.header.kid.split("#")(0)))
.mapError(_ => RecipientNotFoundError())
.mapError(_ => RecipientNotFoundError)
_ <- ZIO.logInfo(s"Extracted recipient Did => $recipientDid")
didId = DidId(recipientDid)
maybePeerDIDRecord <- didNonSecretStorage.getPeerDIDRecord(didId).orDie
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,9 @@ case class CredentialIssuerControllerImpl(
ZIO.unit,
ZIO.fail(OIDCCredentialIssuerService.Errors.InvalidProof("Invalid proof"))
)
.mapError { case InvalidProof(message) =>
badRequestInvalidProof(jwt, message)
.mapError {
case InvalidProof(message) => badRequestInvalidProof(jwt, message)
case DIDResolutionError(message) => badRequestInvalidProof(jwt, message)
}
nonce <- getNonceFromJwt(JWT(jwt))
.mapError(throwable => badRequestInvalidProof(jwt, throwable.getMessage))
Expand All @@ -180,15 +181,17 @@ case class CredentialIssuerControllerImpl(
sessionWithSubjectDid <- credentialIssuerService
.updateIssuanceSession(session.withSubjectDid(subjectDid))
.mapError(ue =>
serverError(Some(s"Unexpected error while updating issuance session with subject DID: ${ue.message}"))
serverError(
Some(s"Unexpected error while updating issuance session with subject DID: ${ue.userFacingMessage}")
)
)
credentialDefinition <- ZIO
.fromOption(maybeCredentialDefinition)
.mapError(_ => badRequestUnsupportedCredentialType("No credential definition provided"))
validatedCredentialDefinition <- credentialIssuerService
.validateCredentialDefinition(credentialDefinition)
.mapError(ue =>
serverError(Some(s"Unexpected error while validating credential definition: ${ue.message}"))
serverError(Some(s"Unexpected error while validating credential definition: ${ue.userFacingMessage}"))
)
credential <- credentialIssuerService
.issueJwtCredential(
Expand All @@ -198,9 +201,11 @@ case class CredentialIssuerControllerImpl(
maybeCredentialIdentifier,
validatedCredentialDefinition
)
.mapError(ue => serverError(Some(s"Unexpected error while issuing credential: ${ue.message}")))
.mapError(ue => serverError(Some(s"Unexpected error while issuing credential: ${ue.userFacingMessage}")))
} yield ImmediateCredentialResponse(credential.value)
case None => ZIO.fail(badRequestInvalidProof(jwt = "empty", details = "No proof provided"))
// case Some(CwtProof(_, _)) => ??? // TODO
// case Some(LdpProof(_, _)) => ??? // TODO
}
}

Expand All @@ -223,7 +228,9 @@ case class CredentialIssuerControllerImpl(
)
.map(offer => CredentialOfferResponse(offer.offerUri))
.mapError(ue =>
internalServerError(detail = Some(s"Unexpected error while creating credential offer: ${ue.message}"))
internalServerError(detail =
Some(s"Unexpected error while creating credential offer: ${ue.userFacingMessage}")
)
)
} yield resp
}
Expand All @@ -236,7 +243,7 @@ case class CredentialIssuerControllerImpl(
.getIssuanceSessionByIssuerState(request.issuerState)
.map(session => NonceResponse(session.nonce))
.mapError(ue =>
internalServerError(detail = Some(s"Unexpected error while creating credential offer: ${ue.message}"))
internalServerError(detail = Some(s"Unexpected error while creating credential offer: ${ue.userFacingMessage}"))
)
}

Expand Down
Loading
Loading