diff --git a/CHANGELOG.md b/CHANGELOG.md index ec2de9ef..9ebdbde9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ This project aims to adhere to [Semantic Versioning](http://semver.org/). leave the default Apache HttpClient [retry behavior](https://hc.apache.org/httpcomponents-client-4.5.x/tutorial/html/fundamentals.html#d5e316). - [Content type is set for file object in directory listing when it isn't available](https://github.com/joyent/java-manta/issues/341) - [Fixes validation guard clauses that are not validating anything](https://github.com/joyent/java-manta/issues/346) + - [MDC logging of the load balancer address now logs the proper address](https://github.com/joyent/java-manta/issues/266) ### Changed - Validation of paths passed to `MantaClient` is now more consistently strict. More useful errors should be thrown sooner for invalid paths, without any diff --git a/java-manta-client-unshaded/src/main/java/com/joyent/manta/client/MantaClient.java b/java-manta-client-unshaded/src/main/java/com/joyent/manta/client/MantaClient.java index adcdc529..0cc32a69 100644 --- a/java-manta-client-unshaded/src/main/java/com/joyent/manta/client/MantaClient.java +++ b/java-manta-client-unshaded/src/main/java/com/joyent/manta/client/MantaClient.java @@ -22,6 +22,7 @@ import com.joyent.manta.exception.MantaException; import com.joyent.manta.exception.MantaIOException; import com.joyent.manta.exception.MantaJobException; +import com.joyent.manta.exception.MantaNoHttpResponseException; import com.joyent.manta.exception.OnCloseAggregateException; import com.joyent.manta.http.ContentTypeLookup; import com.joyent.manta.http.EncryptionHttpHelper; @@ -1756,7 +1757,7 @@ public UUID createJob(final MantaJob job) throws IOException { try { return httpHelper.executeAndCloseRequest(post, jobIdFunction, "POST {} response [{}] {} ", path); - } catch (NoHttpResponseException e) { + } catch (NoHttpResponseException | MantaNoHttpResponseException e) { lastException = e; LOG.warn("Error posting createJob. Retrying.", e); } diff --git a/java-manta-client-unshaded/src/main/java/com/joyent/manta/client/MantaObjectOutputStream.java b/java-manta-client-unshaded/src/main/java/com/joyent/manta/client/MantaObjectOutputStream.java index 8bdc639a..f4f651bf 100644 --- a/java-manta-client-unshaded/src/main/java/com/joyent/manta/client/MantaObjectOutputStream.java +++ b/java-manta-client-unshaded/src/main/java/com/joyent/manta/client/MantaObjectOutputStream.java @@ -310,7 +310,10 @@ public synchronized void close() throws IOException { } catch (InterruptedException e) { // continue execution if interrupted } catch (ExecutionException e) { - MantaIOException mioe = new MantaIOException(e); + /* We wrap the cause because the stack trace for the + * ExecutionException offers nothing useful and is just a wrapper + * for exceptions that are thrown within a Future. */ + MantaIOException mioe = new MantaIOException(e.getCause()); if (this.objectResponse != null) { final String requestId = this.objectResponse.getHeaderAsString( diff --git a/java-manta-client-unshaded/src/main/java/com/joyent/manta/exception/MantaNoHttpResponseException.java b/java-manta-client-unshaded/src/main/java/com/joyent/manta/exception/MantaNoHttpResponseException.java new file mode 100644 index 00000000..3db6c847 --- /dev/null +++ b/java-manta-client-unshaded/src/main/java/com/joyent/manta/exception/MantaNoHttpResponseException.java @@ -0,0 +1,68 @@ +/* + * Copyright (c) 2017, Joyent, Inc. All rights reserved. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ +package com.joyent.manta.exception; + +/** + * Signals that the target server failed to respond with a valid HTTP response. + * + * This exception typically wraps {@link org.apache.http.NoHttpResponseException}. + * + * @author Elijah Zupancic + * @since 3.1.7 + */ +public class MantaNoHttpResponseException extends MantaIOException { + /** + * Constructs an instance with {@code null} + * as its error detail message. + */ + public MantaNoHttpResponseException() { + } + + /** + * Constructs an instance with the specified detail message. + * + * @param message The detail message (which is saved for later retrieval + * by the {@link #getMessage()} method) + */ + public MantaNoHttpResponseException(final String message) { + super(message); + } + + /** + * Constructs an instance with the specified detail message + * and cause. + * + *

Note that the detail message associated with {@code cause} is + * not automatically incorporated into this exception's detail + * message.

+ * + * @param message The detail message (which is saved for later retrieval + * by the {@link #getMessage()} method) + * @param cause The cause (which is saved for later retrieval by the + * {@link #getCause()} method). (A null value is permitted, + * and indicates that the cause is nonexistent or unknown.) + */ + public MantaNoHttpResponseException(final String message, final Throwable cause) { + super(message, cause); + } + + /** + * Constructs an instance with the specified cause and a + * detail message of {@code (cause==null ? null : cause.toString())} + * (which typically contains the class and detail message of {@code cause}). + * This constructor is useful for IO exceptions that are little more + * than wrappers for other throwables. + * + * @param cause The cause (which is saved for later retrieval by the + * {@link #getCause()} method). (A null value is permitted, + * and indicates that the cause is nonexistent or unknown.) + */ + public MantaNoHttpResponseException(final Throwable cause) { + super(cause); + } +} diff --git a/java-manta-client-unshaded/src/main/java/com/joyent/manta/http/MantaConnectionFactory.java b/java-manta-client-unshaded/src/main/java/com/joyent/manta/http/MantaConnectionFactory.java index a7fb8954..3be79225 100644 --- a/java-manta-client-unshaded/src/main/java/com/joyent/manta/http/MantaConnectionFactory.java +++ b/java-manta-client-unshaded/src/main/java/com/joyent/manta/http/MantaConnectionFactory.java @@ -48,6 +48,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import javax.management.DynamicMBean; import java.io.Closeable; import java.io.IOException; import java.net.InetSocketAddress; @@ -58,7 +59,6 @@ import java.util.Arrays; import java.util.Collection; import java.util.List; -import javax.management.DynamicMBean; /** * Factory class that creates instances of @@ -271,6 +271,7 @@ protected HttpClientBuilder createBuilder() { .setKeepAliveStrategy(new DefaultConnectionKeepAliveStrategy()) .setDefaultRequestConfig(requestConfig) .setConnectionManagerShared(false) + .setRequestExecutor(new MantaHttpRequestExecutor()) .setConnectionBackoffStrategy(new DefaultBackoffStrategy()); if (config.getRetries() > 0) { diff --git a/java-manta-client-unshaded/src/main/java/com/joyent/manta/http/MantaHttpRequestExecutor.java b/java-manta-client-unshaded/src/main/java/com/joyent/manta/http/MantaHttpRequestExecutor.java new file mode 100644 index 00000000..bedcce9e --- /dev/null +++ b/java-manta-client-unshaded/src/main/java/com/joyent/manta/http/MantaHttpRequestExecutor.java @@ -0,0 +1,128 @@ +/* + * Copyright (c) 2017, Joyent, Inc. All rights reserved. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ +package com.joyent.manta.http; + +import com.joyent.manta.exception.MantaIOException; +import com.joyent.manta.exception.MantaNoHttpResponseException; +import org.apache.commons.lang3.StringUtils; +import org.apache.http.HttpClientConnection; +import org.apache.http.HttpException; +import org.apache.http.HttpRequest; +import org.apache.http.HttpResponse; +import org.apache.http.NoHttpResponseException; +import org.apache.http.protocol.HttpContext; +import org.apache.http.protocol.HttpRequestExecutor; +import org.slf4j.MDC; + +import java.io.IOException; + +/** + * Extended implementation of {@link HttpRequestExecutor} with Manta specific + * extensions for logging and exception handling. + * + * @author Elijah Zupancic + * @since 3.1.7 + */ +public class MantaHttpRequestExecutor extends HttpRequestExecutor { + /** + * Creates new instance of HttpRequestExecutor. + * + * @param waitForContinue Maximum time in milliseconds to wait for a 100-continue response + */ + public MantaHttpRequestExecutor(final int waitForContinue) { + super(waitForContinue); + } + + /** + * Creates new instance of HttpRequestExecutor. + */ + public MantaHttpRequestExecutor() { + } + + /** + * Adds a context value for the Manta load balancer associated with the + * request to the MDC object with the key mantaLoadBalancerAddress + * and proxies the parent class + * {@link HttpRequestExecutor#doSendRequest(HttpRequest, HttpClientConnection, HttpContext)} + * method. + * + * {@inheritDoc} + */ + @Override + protected HttpResponse doSendRequest(final HttpRequest request, + final HttpClientConnection conn, + final HttpContext context) throws IOException, HttpException { + MDC.put("mantaLoadBalancerAddress", extractLoadBalancerAddress(conn)); + return super.doSendRequest(request, conn, context); + } + + /** + * Proxies the parent class + * {@link HttpRequestExecutor#doReceiveResponse(HttpRequest, HttpClientConnection, HttpContext)} + * method and catches {@link IOException} instances thrown. Those exceptions + * are then wrapped in a {@link MantaIOException} or + * {@link MantaNoHttpResponseException} instance in order to provide + * detailed information for debugging. + * + * {@inheritDoc} + */ + @Override + protected HttpResponse doReceiveResponse( + final HttpRequest request, + final HttpClientConnection conn, + final HttpContext context) throws HttpException, IOException { + HttpResponse response = null; + + try { + response = super.doReceiveResponse(request, conn, context); + + /* We catch all IOExceptions and wrap then in a MantaIOException because + * this allows us to capture key information like the request id and + * load balancer address directly in the exception message. */ + } catch (IOException e) { + final MantaIOException mioe; + + /* If the source exception is NoHttpResponseException we create + * a MantaNoHttpResponseException, so that we can act upon that + * exception type directly within Manta. */ + if (e instanceof NoHttpResponseException) { + mioe = new MantaNoHttpResponseException(e); + } else { + mioe = new MantaIOException(e); + } + + HttpHelper.annotateContextedException(mioe, request, response); + + if (request.getFirstHeader(MantaHttpHeaders.REQUEST_ID) != null) { + mioe.setContextValue("requestId", + request.getFirstHeader(MantaHttpHeaders.REQUEST_ID).getValue()); + } + + mioe.setContextValue("loadBalancerAddress", extractLoadBalancerAddress(conn)); + + throw mioe; + } + + return response; + } + + /** + * Extracts the remote load balancer IP address from the toString() method + * of a {@link HttpClientConnection}. + * + * @param conn connection to extract IP information from + * @return IP address string or null if connection is null + */ + private static String extractLoadBalancerAddress(final HttpClientConnection conn) { + if (conn == null) { + return null; + } + + return StringUtils.substringBetween(conn.toString(), "<->", ":"); + } +} diff --git a/java-manta-client-unshaded/src/main/java/com/joyent/manta/http/ShufflingDnsResolver.java b/java-manta-client-unshaded/src/main/java/com/joyent/manta/http/ShufflingDnsResolver.java index 36f79e14..b9457179 100644 --- a/java-manta-client-unshaded/src/main/java/com/joyent/manta/http/ShufflingDnsResolver.java +++ b/java-manta-client-unshaded/src/main/java/com/joyent/manta/http/ShufflingDnsResolver.java @@ -8,7 +8,6 @@ package com.joyent.manta.http; import org.apache.http.conn.DnsResolver; -import org.slf4j.MDC; import java.net.InetAddress; import java.net.UnknownHostException; @@ -28,8 +27,6 @@ public InetAddress[] resolve(final String host) throws UnknownHostException { final InetAddress[] addresses = InetAddress.getAllByName(host); shuffle(addresses); - MDC.put("mantaLoadBalancerAddress", addresses[0].getHostAddress()); - return addresses; } diff --git a/java-manta-client-unshaded/src/test/java/com/joyent/manta/client/MantaClientConnectionFailuresIT.java b/java-manta-client-unshaded/src/test/java/com/joyent/manta/client/MantaClientConnectionFailuresIT.java index 076886e6..50a09faf 100644 --- a/java-manta-client-unshaded/src/test/java/com/joyent/manta/client/MantaClientConnectionFailuresIT.java +++ b/java-manta-client-unshaded/src/test/java/com/joyent/manta/client/MantaClientConnectionFailuresIT.java @@ -12,6 +12,7 @@ import com.joyent.manta.config.ConfigContext; import com.joyent.manta.config.KeyPairFactory; import com.joyent.manta.config.TestConfigContext; +import com.joyent.manta.exception.MantaNoHttpResponseException; import com.joyent.manta.http.MantaConnectionFactory; import org.apache.http.HttpException; import org.apache.http.HttpResponse; @@ -125,7 +126,7 @@ public void canRetryOnNoHttpResponseException() throws IOException { boolean thrown = false; try { mantaClient.head(testPathPrefix); - } catch (NoHttpResponseException e) { + } catch (NoHttpResponseException | MantaNoHttpResponseException e) { thrown = true; } finally { mantaClient.closeWithWarning(); diff --git a/java-manta-it/src/test/resources/logback-test.xml b/java-manta-it/src/test/resources/logback-test.xml index 9de32dec..19775b37 100644 --- a/java-manta-it/src/test/resources/logback-test.xml +++ b/java-manta-it/src/test/resources/logback-test.xml @@ -3,7 +3,7 @@ - [%thread] %-5level %logger [%X{mantaRequestId}] - %msg%n + [%thread] %-5level %logger [%X{mantaLoadBalancerAddress} %X{mantaRequestId}] - %msg%n