diff --git a/src/main/scala/org/broadinstitute/dsde/firecloud/EntityService.scala b/src/main/scala/org/broadinstitute/dsde/firecloud/EntityService.scala index 299d826c9..5a41df1b2 100644 --- a/src/main/scala/org/broadinstitute/dsde/firecloud/EntityService.scala +++ b/src/main/scala/org/broadinstitute/dsde/firecloud/EntityService.scala @@ -11,7 +11,7 @@ import org.broadinstitute.dsde.firecloud.model.{ModelSchema, _} import org.broadinstitute.dsde.firecloud.service.PerRequest.{PerRequestMessage, RequestComplete} import org.broadinstitute.dsde.firecloud.service.TsvTypes.TsvType import org.broadinstitute.dsde.firecloud.service.{TSVFileSupport, TsvTypes} -import org.broadinstitute.dsde.firecloud.utils.TSVLoadFile +import org.broadinstitute.dsde.firecloud.utils.{StatusCodeUtils, TSVLoadFile} import org.broadinstitute.dsde.rawls.model._ import org.broadinstitute.dsde.workbench.model.google.{GcsBucketName, GcsObjectName} import org.databiosphere.workspacedata.client.ApiException @@ -59,6 +59,7 @@ class EntityService(rawlsDAO: RawlsDAO, modelSchema: ModelSchema )(implicit val executionContext: ExecutionContext) extends TSVFileSupport + with StatusCodeUtils with LazyLogging { val format = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ssZZ") @@ -379,7 +380,7 @@ class EntityService(rawlsDAO: RawlsDAO, } // if human-readable message extraction fails, just default to the ApiException message val errMsg = Try(extractMessage(apiEx.getResponseBody)).toOption.getOrElse(apiEx.getMessage) - new FireCloudExceptionWithErrorReport(ErrorReport(apiEx.getCode, errMsg)) + new FireCloudExceptionWithErrorReport(ErrorReport(statusCodeFrom(apiEx.getCode), errMsg)) } def getEntitiesWithType(workspaceNamespace: String, diff --git a/src/main/scala/org/broadinstitute/dsde/firecloud/model/ModelJsonProtocol.scala b/src/main/scala/org/broadinstitute/dsde/firecloud/model/ModelJsonProtocol.scala index 4db0cbaaf..2c3dc3813 100644 --- a/src/main/scala/org/broadinstitute/dsde/firecloud/model/ModelJsonProtocol.scala +++ b/src/main/scala/org/broadinstitute/dsde/firecloud/model/ModelJsonProtocol.scala @@ -13,6 +13,7 @@ import org.broadinstitute.dsde.firecloud.model.Project.ProjectRoles.ProjectRole import org.broadinstitute.dsde.firecloud.model.Project._ import org.broadinstitute.dsde.firecloud.model.SamResource.{AccessPolicyName, ResourceId, UserPolicy} import org.broadinstitute.dsde.firecloud.model.ShareLog.{Share, ShareType} +import org.broadinstitute.dsde.firecloud.utils.StatusCodeUtils import org.broadinstitute.dsde.rawls.model.UserModelJsonSupport._ import org.broadinstitute.dsde.rawls.model.WorkspaceACLJsonSupport.WorkspaceAccessLevelFormat import org.broadinstitute.dsde.rawls.model._ @@ -25,7 +26,7 @@ import spray.json._ import scala.util.{Failure, Success, Try} //noinspection TypeAnnotation,RedundantNewCaseClass -object ModelJsonProtocol extends WorkspaceJsonSupport with SprayJsonSupport { +object ModelJsonProtocol extends WorkspaceJsonSupport with SprayJsonSupport with StatusCodeUtils { import spray.json.DefaultJsonProtocol._ def optionalEntryIntReader(fieldName: String, data: Map[String, JsValue]): Option[Int] = @@ -55,7 +56,7 @@ object ModelJsonProtocol extends WorkspaceJsonSupport with SprayJsonSupport { override def write(code: StatusCode): JsValue = JsNumber(code.intValue) override def read(json: JsValue): StatusCode = json match { - case JsNumber(n) => n.intValue + case JsNumber(n) => statusCodeFrom(n.intValue) case _ => throw DeserializationException("unexpected json type") } } diff --git a/src/main/scala/org/broadinstitute/dsde/firecloud/utils/EnabledUserDirectives.scala b/src/main/scala/org/broadinstitute/dsde/firecloud/utils/EnabledUserDirectives.scala index 2061ef9d4..592b457b2 100644 --- a/src/main/scala/org/broadinstitute/dsde/firecloud/utils/EnabledUserDirectives.scala +++ b/src/main/scala/org/broadinstitute/dsde/firecloud/utils/EnabledUserDirectives.scala @@ -18,7 +18,7 @@ import org.broadinstitute.dsde.workbench.util.FutureSupport.toFutureTry import scala.concurrent.{ExecutionContext, Future, Promise} import scala.util.{Failure, Success} -trait EnabledUserDirectives extends LazyLogging with SprayJsonSupport { +trait EnabledUserDirectives extends LazyLogging with SprayJsonSupport with StatusCodeUtils { // Hardcode an ErrorReportSource to allow differentiating between enabled-user errors and other errors. implicit val errorReportSource: ErrorReportSource = ErrorReportSource("Orchestration-enabled-check") @@ -58,7 +58,7 @@ trait EnabledUserDirectives extends LazyLogging with SprayJsonSupport { s"ApiException exception checking enabled status for user ${userInfo.userEmail}: (${apiex.getMessage}) while calling $uri", apiex ) - val code = StatusCode.int2StatusCode(apiex.getCode) + val code = statusCodeFrom(apiex.getCode, Option(StatusCodes.InternalServerError)) if (code == StatusCodes.NotFound) { throwErrorReport(StatusCodes.Unauthorized, "User is not registered.") } else { @@ -99,7 +99,7 @@ trait EnabledUserDirectives extends LazyLogging with SprayJsonSupport { case Failure(_) => response } throw new FireCloudExceptionWithErrorReport( - ErrorReport(StatusCode.int2StatusCode(statusCode), + ErrorReport(statusCodeFrom(statusCode, Option(StatusCodes.InternalServerError)), s"Sam call to $functionName failed with error '$stringErrMsg'" ) ) diff --git a/src/main/scala/org/broadinstitute/dsde/firecloud/utils/StatusCodeUtils.scala b/src/main/scala/org/broadinstitute/dsde/firecloud/utils/StatusCodeUtils.scala new file mode 100644 index 000000000..41373ff55 --- /dev/null +++ b/src/main/scala/org/broadinstitute/dsde/firecloud/utils/StatusCodeUtils.scala @@ -0,0 +1,31 @@ +package org.broadinstitute.dsde.firecloud.utils + +import akka.http.scaladsl.model.{StatusCode, StatusCodes} + +import scala.util.Try + +trait StatusCodeUtils { + + /** + * Safely translates an integer to a StatusCode. This method avoids the RuntimeException + * thrown by StatusCode.int2StatusCode when supplied with an unknown code and returns + * a default status code instead. + * + * @param intCode the integer value to translate + * @param default the code to return if the integer value is unknown; + * defaults to Internal Server Error + * @return the final status code + */ + def statusCodeFrom(intCode: Int, default: Option[StatusCode] = None): StatusCode = + Try(StatusCode.int2StatusCode(intCode)).getOrElse( + default.getOrElse( + StatusCodes.custom(intCode, + reason = s"unknown status $intCode", + defaultMessage = s"unknown status $intCode", + isSuccess = false, + allowsEntity = true + ) + ) + ) + +} diff --git a/src/main/scala/org/broadinstitute/dsde/firecloud/webservice/UserApiService.scala b/src/main/scala/org/broadinstitute/dsde/firecloud/webservice/UserApiService.scala index 97d839946..f63e925fb 100644 --- a/src/main/scala/org/broadinstitute/dsde/firecloud/webservice/UserApiService.scala +++ b/src/main/scala/org/broadinstitute/dsde/firecloud/webservice/UserApiService.scala @@ -187,7 +187,10 @@ trait UserApiService } case x => // if we get any other error from Sam, pass that error on - respondWithErrorReport(x.intValue, "Unexpected response validating registration: " + x.toString, requestContext) + respondWithErrorReport(statusCodeFrom(x.intValue), + "Unexpected response validating registration: " + x.toString, + requestContext + ) } private def handleOkResponse(regInfo: RegistrationInfoV2, @@ -232,7 +235,7 @@ trait UserApiService } } case x => - respondWithErrorReport(x.intValue, + respondWithErrorReport(statusCodeFrom(x.intValue), "Unexpected response validating registration: " + x.toString, requestContext ) diff --git a/src/test/scala/org/broadinstitute/dsde/firecloud/utils/StatusCodeUtilsSpec.scala b/src/test/scala/org/broadinstitute/dsde/firecloud/utils/StatusCodeUtilsSpec.scala new file mode 100644 index 000000000..f56643a88 --- /dev/null +++ b/src/test/scala/org/broadinstitute/dsde/firecloud/utils/StatusCodeUtilsSpec.scala @@ -0,0 +1,41 @@ +package org.broadinstitute.dsde.firecloud.utils + +import akka.http.scaladsl.model.StatusCode +import akka.http.scaladsl.model.StatusCodes._ +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers.convertToAnyShouldWrapper + +class StatusCodeUtilsSpec extends AnyFlatSpec with StatusCodeUtils { + + behavior of "statusCodeFrom" + + val expectedCases: Map[Int, StatusCode] = Map( + 200 -> OK, + 404 -> NotFound, + 503 -> ServiceUnavailable + ) + + expectedCases.foreach { case (intCode, statusCode) => + it should s"handle known code $intCode" in { + statusCodeFrom(intCode) shouldBe statusCode + } + } + + val unknownCodes: List[Int] = List(-1, 0, 42, 222, 555) + + unknownCodes.foreach { intCode => + it should s"create a custom status code $intCode for unknown values" in { + val actual = statusCodeFrom(intCode) + actual.intValue() shouldBe intCode + actual.isSuccess() shouldBe false + actual.defaultMessage() shouldBe s"unknown status $intCode" + } + } + + unknownCodes.foreach { intCode => + it should s"default unknown code $intCode to the caller-supplied default" in { + statusCodeFrom(intCode, Option(ImATeapot)) shouldBe ImATeapot + } + } + +}