fix: Log the error response bodies from the Pact Broker #1830

Author: rholshausen

core/pactbroker/src/main/kotlin/au/com/dius/pact/core/pactbroker/HalClient.kt

@@ -210,7 +210,7 @@ open class HalClient @JvmOverloads constructor(
         if (handler != null) {
           handler(it.code, it)
         } else if (it.code >= 300) {
-          logger.error { "PUT JSON request failed with status ${it.code} ${it.reasonPhrase}" }
+          logger.error { "POST JSON request failed with status ${it.code} ${it.reasonPhrase}" }
           Result.Err(RequestFailedException(it.code, if (it.entity != null) EntityUtils.toString(it.entity) else null))
         } else {
           true
@@ -343,14 +343,38 @@ open class HalClient @JvmOverloads constructor(
       when (response.code) {
         404 -> Result.Err(NotFoundHalResponse("No HAL document found at path '$path'"))
         else -> {
-          val body = if (response.entity != null) EntityUtils.toString(response.entity) else null
+          val body = handleResponseBody(response, path)
           Result.Err(RequestFailedException(response.code, body,
-            "Request to path '$path' failed with response ${response.code}"))
+            "Request to path '$path' failed with HTTP response ${response.code}"))
         }
       }
     }
   }
 
+  private fun handleResponseBody(response: ClassicHttpResponse, path: String): String? {
+    var body: String? = null
+    if (response.entity != null) {
+      body = EntityUtils.toString(response.entity)
+      val contentType = ContentType.parseLenient(response.entity.contentType)
+      if (isJsonResponse(contentType)) {
+        val json = handleWith<JsonValue> { JsonParser.parseString(body) }
+        when (json) {
+          is Result.Ok -> {
+            logger.error { "Request to path '$path' failed with HTTP response ${response.code}" }
+            logger.error { "JSON Response:\n${json.value.prettyPrint()}" }
+          }
+
+          is Result.Err -> {
+            logger.error { "Request to path '$path' failed with HTTP response ${response.code}: $body" }
+          }
+        }
+      } else {
+        logger.error { "Request to path '$path' failed with HTTP response ${response.code}: $body" }
+      }
+    }
+    return body
+  }
+
   private fun fetchLink(link: String, options: Map<String, Any>): JsonValue.Object {
     val href = hrefForLink(link, options)
     return this.fetch(href, false).unwrap()

core/pactbroker/src/main/kotlin/au/com/dius/pact/core/pactbroker/PactBrokerClient.kt

@@ -473,6 +473,10 @@ open class PactBrokerClient(
     }
   }
 
+  @Deprecated(
+    "use version that takes a list of ConsumerVersionSelectors",
+    replaceWith = ReplaceWith("fetchConsumersWithSelectorsV2")
+  )
   override fun fetchConsumersWithSelectors(
     providerName: String,
     selectors: List<ConsumerVersionSelector>,

core/pactbroker/src/test/groovy/au/com/dius/pact/core/pactbroker/HalClientSpec.groovy

@@ -289,7 +289,7 @@ class HalClientSpec extends Specification {
 
   @Unroll
   @SuppressWarnings('UnnecessaryGetter')
-  def 'post URL returns #success if the response is #status'() {
+  def 'post to URL returns #success if the response is #status'() {
     given:
     def mockClient = Mock(CloseableHttpClient)
     client.httpClient = mockClient
@@ -308,7 +308,7 @@ class HalClientSpec extends Specification {
     'failure' | 400    | Result.Err
   }
 
-  def 'post URL returns a failure result if an exception is thrown'() {
+  def 'post to URL returns a failure result if an exception is thrown'() {
     given:
     def mockClient = Mock(CloseableHttpClient)
     client.httpClient = mockClient
@@ -322,7 +322,7 @@ class HalClientSpec extends Specification {
   }
 
   @SuppressWarnings('UnnecessaryGetter')
-  def 'post URL delegates to a handler if one is supplied'() {
+  def 'post to URL delegates to a handler if one is supplied'() {
     given:
     def mockClient = Mock(CloseableHttpClient)
     client.httpClient = mockClient
@@ -523,4 +523,23 @@ class HalClientSpec extends Specification {
       handler.handleResponse(mockResponse)
     }
   }
+
+  @Issue('1830')
+  def 'post to URL handles any error responses'() {
+    given:
+    def mockClient = Mock(CloseableHttpClient)
+    client.httpClient = mockClient
+    def mockResponse = Mock(ClassicHttpResponse) {
+      getCode() >> 400
+      getEntity() >> new StringEntity('{"error": ["it went bang!"]}', ContentType.APPLICATION_JSON)
+    }
+    client.pathInfo = JsonParser.INSTANCE.parseString('{"_links":{"path":{"href":"http://localhost:8080/"}}}')
+
+    when:
+    def result = client.postJson('path', [:], '"body"')
+
+    then:
+    1 * mockClient.execute(_, _, _) >> { req, c, handler -> handler.handleResponse(mockResponse) }
+    result instanceof Result.Err
+  }
 }

provider/src/main/kotlin/au/com/dius/pact/provider/junitsupport/loader/PactBrokerLoader.kt

@@ -12,6 +12,7 @@ import au.com.dius.pact.core.pactbroker.ConsumerVersionSelectors
 import au.com.dius.pact.core.pactbroker.IPactBrokerClient
 import au.com.dius.pact.core.pactbroker.PactBrokerClient
 import au.com.dius.pact.core.pactbroker.PactBrokerClientConfig
+import au.com.dius.pact.core.pactbroker.RequestFailedException
 import au.com.dius.pact.core.support.Result
 import au.com.dius.pact.core.support.Utils.permutations
 import au.com.dius.pact.core.support.expressions.DataType
@@ -249,7 +250,24 @@ open class PactBrokerLoader(
               providerBranch, pending, wipSinceDate)
       var consumers = when (result) {
         is Result.Ok -> result.value
-        is Result.Err -> throw result.error
+        is Result.Err -> {
+          when (val exception = result.error) {
+            is RequestFailedException -> {
+              logger.error(exception) {
+                if (exception.body.isNotEmpty()) {
+                  "Failed to load Pacts from the Pact broker: ${exception.message} (HTTP Status ${exception.status})" +
+                    "\nResponse: ${exception.body}"
+                } else {
+                  "Failed to load Pacts from the Pact broker: ${exception.message} (HTTP Status ${exception.status})"
+                }
+              }
+            }
+            else -> {
+              logger.error(exception) { "Failed to load Pacts from the Pact broker " }
+            }
+          }
+          throw result.error
+        }
       }
 
       if (failIfNoPactsFound && consumers.isEmpty()) {

provider/src/test/groovy/au/com/dius/pact/provider/junitsupport/loader/PactBrokerLoaderSpec.groovy

@@ -8,6 +8,7 @@ import au.com.dius.pact.core.pactbroker.IPactBrokerClient
 import au.com.dius.pact.core.pactbroker.InvalidHalResponse
 import au.com.dius.pact.core.pactbroker.InvalidNavigationRequest
 import au.com.dius.pact.core.pactbroker.PactBrokerResult
+import au.com.dius.pact.core.pactbroker.RequestFailedException
 import au.com.dius.pact.core.support.expressions.DataType
 import au.com.dius.pact.core.support.expressions.ExpressionParser
 import au.com.dius.pact.core.support.expressions.SystemPropertyResolver
@@ -1368,6 +1369,17 @@ class PactBrokerLoaderSpec extends Specification {
     thrown(InvalidNavigationRequest)
   }
 
+  @Issue('#1830')
+  def 'Handles error responses from the broker'() {
+    when:
+    pactBrokerLoader().load('test')
+
+    then:
+    1 * brokerClient.fetchConsumersWithSelectorsV2('test', [], [], '', false, '') >>
+      new Result.Err(new RequestFailedException(400, '{"error": "selectors are invalid"}', 'Request to broker failed'))
+    thrown(RequestFailedException)
+  }
+
   void 'Does not enable insecure TLS when not set in PactBroker annotation and not using the fallback system property'() {
     given:
     pactBrokerLoader = {