Gracefully handle HTML errors from Eureka server (#1265)

It is possible to get HTML responses from the server (e.g. Apache fails to talk to backend API). These would result in a failure to deserialize and a random exception logged.
Netflix · Dec 19, 2019 · c4d212b · c4d212b
1 parent 689b6fa
commit c4d212b
Show file tree

Hide file tree

Showing 3 changed files with 99 additions and 10 deletions.
diff --git a/eureka-client/build.gradle b/eureka-client/build.gradle
@@ -37,4 +37,7 @@ dependencies {
     testCompile "org.mockito:mockito-core:${mockitoVersion}"
     testCompile "org.mock-server:mockserver-netty:${mockserverVersion}"
     testCompile "com.netflix.governator:governator:${governatorVersion}"
+    testCompile "com.github.tomakehurst:wiremock-jre8:2.25.1"
+    testCompile "org.assertj:assertj-core:3.11.1"
+    testCompile "javax.servlet:javax.servlet-api:4.0.1"
 }
diff --git a/...in/java/com/netflix/discovery/shared/transport/jersey/AbstractJerseyEurekaHttpClient.java b/...in/java/com/netflix/discovery/shared/transport/jersey/AbstractJerseyEurekaHttpClient.java
@@ -1,14 +1,5 @@
 package com.netflix.discovery.shared.transport.jersey;
 
-import javax.ws.rs.core.MediaType;
-import javax.ws.rs.core.MultivaluedMap;
-import javax.ws.rs.core.Response.Status;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Map.Entry;
-
 import com.netflix.appinfo.InstanceInfo;
 import com.netflix.appinfo.InstanceInfo.InstanceStatus;
 import com.netflix.discovery.shared.Application;
@@ -24,6 +15,15 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.MultivaluedMap;
+import javax.ws.rs.core.Response.Status;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+
 import static com.netflix.discovery.shared.transport.EurekaHttpResponse.anEurekaHttpResponse;
 
 /**
@@ -32,6 +32,7 @@
 public abstract class AbstractJerseyEurekaHttpClient implements EurekaHttpClient {
 
     private static final Logger logger = LoggerFactory.getLogger(AbstractJerseyEurekaHttpClient.class);
+    protected static final String HTML = "html";
 
     protected final Client jerseyClient;
     protected final String serviceUrl;
@@ -101,7 +102,8 @@ public EurekaHttpResponse<InstanceInfo> sendHeartBeat(String appName, String id,
             addExtraHeaders(requestBuilder);
             response = requestBuilder.put(ClientResponse.class);
             EurekaHttpResponseBuilder<InstanceInfo> eurekaResponseBuilder = anEurekaHttpResponse(response.getStatus(), InstanceInfo.class).headers(headersOf(response));
-            if (response.hasEntity()) {
+            if (response.hasEntity() &&
+                    !HTML.equals(response.getType().getSubtype())) { //don't try and deserialize random html errors from the server
                 eurekaResponseBuilder.entity(response.getEntity(InstanceInfo.class));
             }
             return eurekaResponseBuilder.build();

diff --git a/...rc/test/java/com/netflix/discovery/shared/transport/jersey/UnexpectedContentTypeTest.java b/...rc/test/java/com/netflix/discovery/shared/transport/jersey/UnexpectedContentTypeTest.java
@@ -0,0 +1,84 @@
+package com.netflix.discovery.shared.transport.jersey;
+
+import com.github.tomakehurst.wiremock.junit.WireMockRule;
+import com.netflix.appinfo.InstanceInfo;
+import com.netflix.discovery.shared.resolver.DefaultEndpoint;
+import com.netflix.discovery.shared.transport.EurekaHttpResponse;
+import com.netflix.discovery.shared.transport.TransportClientFactory;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+import java.util.UUID;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
+import static com.github.tomakehurst.wiremock.client.WireMock.put;
+import static com.github.tomakehurst.wiremock.client.WireMock.putRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo;
+import static com.github.tomakehurst.wiremock.client.WireMock.verify;
+import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * @author rbolles on 12/19/19.
+ */
+public class UnexpectedContentTypeTest {
+
+    protected static final String CLIENT_APP_NAME = "unexpectedContentTypeTest";
+    private JerseyApplicationClient jerseyHttpClient;
+
+    @Rule
+    public WireMockRule wireMockRule = new WireMockRule(wireMockConfig().dynamicPort().dynamicHttpsPort()); // No-args constructor defaults to port 8080
+
+    @Before
+    public void setUp() throws Exception {
+        TransportClientFactory clientFactory = JerseyEurekaHttpClientFactory.newBuilder()
+                .withClientName(CLIENT_APP_NAME)
+                .build();
+
+        String uri = String.format("http://localhost:%s/v2/", wireMockRule.port());
+
+        jerseyHttpClient = (JerseyApplicationClient) clientFactory.newClient(new DefaultEndpoint(uri));
+    }
+
+    @Test
+    public void testSendHeartBeatReceivesUnexpectedHtmlResponse() {
+        long lastDirtyTimestamp = System.currentTimeMillis();
+        String uuid = UUID.randomUUID().toString();
+
+        stubFor(put(urlPathEqualTo("/v2/apps/" + CLIENT_APP_NAME + "/" + uuid))
+                .withQueryParam("status", equalTo("UP"))
+                .withQueryParam("lastDirtyTimestamp", equalTo(lastDirtyTimestamp + ""))
+                .willReturn(aResponse()
+                        .withStatus(502)
+                        .withHeader("Content-Type", "text/HTML")
+                        .withBody("<html><body>Something went wrong in Apacache</body></html>")));
+
+        InstanceInfo instanceInfo = mock(InstanceInfo.class);
+        when(instanceInfo.getStatus()).thenReturn(InstanceInfo.InstanceStatus.UP);
+        when(instanceInfo.getLastDirtyTimestamp()).thenReturn(lastDirtyTimestamp);
+
+        EurekaHttpResponse<InstanceInfo> response = jerseyHttpClient.sendHeartBeat(CLIENT_APP_NAME, uuid, instanceInfo, null);
+
+        verify(putRequestedFor(urlPathEqualTo("/v2/apps/" + CLIENT_APP_NAME + "/" + uuid))
+                .withQueryParam("status", equalTo("UP"))
+                .withQueryParam("lastDirtyTimestamp", equalTo(lastDirtyTimestamp + ""))
+        );
+
+        assertThat(response.getStatusCode()).as("status code").isEqualTo(502);
+        assertThat(response.getEntity()).as("instance info").isNull();
+    }
+
+    @After
+    public void tearDown() throws Exception {
+        if (jerseyHttpClient != null) {
+            jerseyHttpClient.shutdown();
+        }
+    }
+
+}