Skip to content

Commit

Permalink
Fix logic in SamExceptionFactory to extract message
Browse files Browse the repository at this point in the history
  • Loading branch information
rtitle committed Aug 27, 2024
1 parent 6ac4257 commit e2055ad
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package org.broadinstitute.dsde.workbench.leonardo.dao.sam

import com.fasterxml.jackson.databind.ObjectMapper
import org.broadinstitute.dsde.workbench.client.sam.ApiException
import org.broadinstitute.dsde.workbench.client.sam.model.ErrorReport
import org.broadinstitute.dsde.workbench.leonardo.model.LeoException
import org.broadinstitute.dsde.workbench.model.TraceId

import scala.util.Try

/**
* Represents an exception interacting with Sam.
*/
Expand All @@ -16,13 +20,38 @@ class SamException private (message: String, code: Int, cause: Throwable, traceI
)

object SamException {
val objectMapper = new ObjectMapper

def create(messagePrefix: String, apiException: ApiException, traceId: TraceId): SamException =
new SamException(
// TODO: investigate whether we need to extract the ApiException message like in
// TCL SamExceptionFactory
s"$messagePrefix: ${apiException}",
extractMessage(messagePrefix, apiException),
apiException.getCode,
apiException.getCause,
traceId
)

/**
* Extracts a useful message from a Sam client ApiException.
*
* Logic borrowed from terra-common-lib SamExceptionFactory class. We can't use the class directly
* because it has Spring dependencies, which Leo excludes.
*/
private def extractMessage(messagePrefix: String, apiException: ApiException): String = {
// The generated ApiException class unfortunately formats getMessage(), and includes
// the entire response body. We want to extract the actual message from that.
val messagePattern = "^Message: ([\\S\\s]*)\\nHTTP response code:".r
val extractedMessage = messagePattern.findFirstMatchIn(apiException.getMessage).map(_.group(1)).filterNot(_.isBlank)

// If we could not extract the top-level message, try to deserialize the error report
// buried one level down and extract the message from that.
val finalMessage = extractedMessage.orElse {
val errorReport = Try(objectMapper.readValue(apiException.getResponseBody, classOf[ErrorReport]))
errorReport.map(_.getMessage).toOption
}

// Prepend the message prefix to the extracted message, if we have one.
s"$messagePrefix: ${finalMessage.getOrElse("")}"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package org.broadinstitute.dsde.workbench.leonardo.dao.sam

import akka.http.scaladsl.model.StatusCodes
import com.fasterxml.jackson.databind.ObjectMapper
import org.broadinstitute.dsde.workbench.client.sam.ApiException
import org.broadinstitute.dsde.workbench.client.sam.model.ErrorReport
import org.broadinstitute.dsde.workbench.leonardo.LeonardoTestSuite
import org.broadinstitute.dsde.workbench.model.TraceId
import org.scalatest.flatspec.AnyFlatSpec

import scala.jdk.CollectionConverters._

class SamExceptionSpec extends AnyFlatSpec with LeonardoTestSuite {
val objectMapper = new ObjectMapper

"SamException" should "create from an ApiException" in {
val cause = new RuntimeException("cause")
val errorReport = new ErrorReport().statusCode(400).message("error report message")
val apiException = new ApiException(
"test message",
cause,
400,
Map("responseHeader1" -> List("responseHeader1Value").asJava).asJava,
objectMapper.writeValueAsString(errorReport)
)
val samException = SamException.create("messagePrefix", apiException, TraceId("traceId"))

samException.message shouldBe "messagePrefix: test message"
samException.statusCode shouldBe StatusCodes.BadRequest
samException.cause shouldBe cause
}

it should "extract the message from the ErrorReport if the top-level message is blank" in {
val cause = new RuntimeException("cause")
val errorReport = new ErrorReport().statusCode(400).message("error report message")
val apiException = new ApiException(
"",
cause,
400,
Map("responseHeader1" -> List("responseHeader1Value").asJava).asJava,
objectMapper.writeValueAsString(errorReport)
)
val samException = SamException.create("messagePrefix", apiException, TraceId("traceId"))

samException.message shouldBe "messagePrefix: error report message"
samException.statusCode shouldBe StatusCodes.BadRequest
samException.cause shouldBe cause
}
}

0 comments on commit e2055ad

Please sign in to comment.