Merge pull request #21 from hmrc/feature/fix-connector

Catch exceptions thrown in HTTP library
hmrc · Dec 4, 2017 · e53b03b · e53b03b
2 parents 6150fe6 + f185001
commit e53b03b
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 3 deletions.
diff --git a/app/uk/gov/hmrc/epayeapi/connectors/ConnectorBase.scala b/app/uk/gov/hmrc/epayeapi/connectors/ConnectorBase.scala
@@ -22,7 +22,7 @@ import play.api.libs.json.{JsError, JsSuccess, Reads}
 import uk.gov.hmrc.epayeapi.models.in
 import uk.gov.hmrc.epayeapi.models.in._
 import uk.gov.hmrc.epayeapi.syntax.json._
-import uk.gov.hmrc.play.http.{HeaderCarrier, HttpGet, HttpReads, HttpResponse}
+import uk.gov.hmrc.play.http._
 
 import scala.concurrent.{ExecutionContext, Future}
 
@@ -38,6 +38,17 @@ trait ConnectorBase {
     } yield reader(jsonReader).read("GET", url, response)
 
     result.recover({
+      case ex: HttpException =>
+        ex.responseCode match {
+          case Status.NOT_FOUND =>
+            ApiNotFound()
+          case statusCode: Int if statusCode < 500 =>
+            ApiError(statusCode, ex.message)
+          case statusCode: Int =>
+            Logger.error(s"Upstream returned unexpected response: status=$statusCode", ex)
+            ApiException(ex.getMessage)
+        }
+
       case ex: Exception =>
         Logger.error("HTTP request threw exception", ex)
         ApiException(ex.getMessage)

diff --git a/conf/application.conf b/conf/application.conf
@@ -159,7 +159,7 @@ Dev {
       }
       epaye {
         host = localhost
-        port = 8105
+        port = 8090
       }
       service-locator {
         host = localhost

diff --git a/test/uk/gov/hmrc/epayeapi/connectors/ConnectorSpec.scala b/test/uk/gov/hmrc/epayeapi/connectors/ConnectorSpec.scala
@@ -22,7 +22,8 @@ import org.scalatest.mock.MockitoSugar
 import play.api.http.Status
 import play.api.libs.json.{JsError, JsPath, Json}
 import uk.gov.hmrc.epayeapi.models.in._
-import uk.gov.hmrc.play.http.{HeaderCarrier, HttpGet, HttpResponse}
+import uk.gov.hmrc.play.http.hooks.HttpHook
+import uk.gov.hmrc.play.http._
 import uk.gov.hmrc.play.test.UnitSpec
 
 import scala.concurrent.ExecutionContext.Implicits.global
@@ -83,6 +84,27 @@ class ConnectorSpec extends UnitSpec with MockitoSugar with ScalaFutures {
       connector.getData.futureValue shouldEqual ApiNotFound[TestData]()
     }
 
+    "return ApiNotFound on 404 exceptions" in new Setup {
+      when(connector.http.GET(url))
+        .thenReturn {
+          failed {
+            new NotFoundException("Not found")
+          }
+        }
+
+      connector.getData.futureValue shouldEqual ApiNotFound[TestData]()
+    }
+    "return ApiError on unexpected responses with status < 500" in new Setup {
+      when(connector.http.GET(url))
+        .thenReturn {
+          failed {
+            new ForbiddenException("Forbidden")
+          }
+        }
+
+      connector.getData.futureValue shouldEqual ApiError[TestData](Status.FORBIDDEN, "Forbidden")
+    }
+
     "return ApiError on unexpected status codes from upstream" in new Setup {
       when(connector.http.GET(url))
         .thenReturn {