Add S3 API execution trace to MinioException (minio#1136)

This is useful to debug issues further and to find root cause.

README.md

@@ -84,6 +84,7 @@ public class FileUploader {
               + "object 'asiaphotos-2015.zip' to bucket 'asiatrip'.");
     } catch (MinioException e) {
       System.out.println("Error occurred: " + e);
+      System.out.println("HTTP trace: " + e.httpTrace());
     }
   }
 }

api/src/main/java/io/minio/ComposeSource.java

@@ -93,7 +93,8 @@ public void buildHeaders(long objectSize, String etag) {
   public long objectSize() throws InternalException {
     if (this.objectSize == null) {
       throw new InternalException(
-          "buildHeaders(long objectSize, String etag) must be called prior to this method invocation");
+          "buildHeaders(long objectSize, String etag) must be called prior to this method invocation",
+          null);
     }
 
     return this.objectSize;
@@ -102,7 +103,8 @@ public long objectSize() throws InternalException {
   public Multimap<String, String> headers() throws InternalException {
     if (this.headers == null) {
       throw new InternalException(
-          "buildHeaders(long objectSize, String etag) must be called prior to this method invocation");
+          "buildHeaders(long objectSize, String etag) must be called prior to this method invocation",
+          null);
     }
 
     return this.headers;

api/src/main/java/io/minio/Digest.java

@@ -57,7 +57,8 @@ public static String sha256Hash(Object data, int len)
     } else {
       throw new InternalException(
           "Unknown data source to calculate SHA-256 hash. This should not happen, "
-              + "please report this issue at https://github.com/minio/minio-java/issues");
+              + "please report this issue at https://github.com/minio/minio-java/issues",
+          null);
     }
 
     return BaseEncoding.base16().encode(sha256Digest.digest()).toLowerCase(Locale.US);
@@ -82,7 +83,8 @@ public static String[] sha256Md5Hashes(Object data, int len)
     } else {
       throw new InternalException(
           "Unknown data source to calculate SHA-256 hash. This should not happen, "
-              + "please report this issue at https://github.com/minio/minio-java/issues");
+              + "please report this issue at https://github.com/minio/minio-java/issues",
+          null);
     }
 
     return new String[] {
@@ -108,7 +110,8 @@ public static String md5Hash(Object data, int len)
     } else {
       throw new InternalException(
           "Unknown data source to calculate MD5 hash. This should not happen, "
-              + "please report this issue at https://github.com/minio/minio-java/issues");
+              + "please report this issue at https://github.com/minio/minio-java/issues",
+          null);
     }
 
     return BaseEncoding.base64().encode(md5Digest.digest());

api/src/main/java/io/minio/MinioClient.java

@@ -101,6 +101,7 @@
 import java.time.ZonedDateTime;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedList;
@@ -189,8 +190,8 @@ public class MinioClient {
   private static final byte[] EMPTY_BODY = new byte[] {};
   // default network I/O timeout is 5 minutes
   private static final long DEFAULT_CONNECTION_TIMEOUT = 5;
-  // maximum allowed bucket policy size is 12KiB
-  private static final int MAX_BUCKET_POLICY_SIZE = 12 * 1024;
+  // maximum allowed bucket policy size is 20KiB
+  private static final int MAX_BUCKET_POLICY_SIZE = 20 * 1024;
   // default expiration for a presigned URL is 7 days in seconds
   private static final int DEFAULT_EXPIRY_TIME = 7 * 24 * 3600;
   private static final String DEFAULT_USER_AGENT =
@@ -229,6 +230,15 @@ public class MinioClient {
     standardHeaders.add("range");
   }
 
+  private static final Set<String> TRACE_QUERY_PARAMS = new HashSet<>();
+
+  static {
+    TRACE_QUERY_PARAMS.add("retention");
+    TRACE_QUERY_PARAMS.add("legal-hold");
+    TRACE_QUERY_PARAMS.add("tagging");
+    TRACE_QUERY_PARAMS.add(UPLOAD_ID);
+  }
+
   private final Map<String, String> regionCache = new ConcurrentHashMap<>();
   private String userAgent = DEFAULT_USER_AGENT;
   private PrintWriter traceStream;
@@ -536,6 +546,28 @@ protected Request createRequest(
     return requestBuilder.build();
   }
 
+  private StringBuilder newTraceBuilder(Request request, String body) {
+    StringBuilder traceBuilder = new StringBuilder();
+    traceBuilder.append("---------START-HTTP---------\n");
+    String encodedPath = request.url().encodedPath();
+    String encodedQuery = request.url().encodedQuery();
+    if (encodedQuery != null) {
+      encodedPath += "?" + encodedQuery;
+    }
+    traceBuilder.append(request.method()).append(" ").append(encodedPath).append(" HTTP/1.1\n");
+    traceBuilder.append(
+        request
+            .headers()
+            .toString()
+            .replaceAll("Signature=([0-9a-f]+)", "Signature=*REDACTED*")
+            .replaceAll("Credential=([^/]+)", "Credential=*REDACTED*"));
+    if (body != null) {
+      traceBuilder.append("\n").append(body);
+    }
+
+    return traceBuilder;
+  }
+
   /** Execute HTTP request for given args and parameters. */
   protected Response execute(
       Method method,
@@ -618,24 +650,12 @@ protected Response execute(
               request.header("x-amz-content-sha256"));
     }
 
-    if (this.traceStream != null) {
-      this.traceStream.println("---------START-HTTP---------");
-      String encodedPath = request.url().encodedPath();
-      String encodedQuery = request.url().encodedQuery();
-      if (encodedQuery != null) {
-        encodedPath += "?" + encodedQuery;
-      }
-      this.traceStream.println(request.method() + " " + encodedPath + " HTTP/1.1");
-      this.traceStream.println(
-          request
-              .headers()
-              .toString()
-              .replaceAll("Signature=([0-9a-f]+)", "Signature=*REDACTED*")
-              .replaceAll("Credential=([^/]+)", "Credential=*REDACTED*"));
-      if (traceRequestBody) {
-        this.traceStream.println(new String((byte[]) body, StandardCharsets.UTF_8));
-      }
-    }
+    StringBuilder traceBuilder =
+        newTraceBuilder(
+            request, traceRequestBody ? new String((byte[]) body, StandardCharsets.UTF_8) : null);
+    PrintWriter traceStream = this.traceStream;
+    if (traceStream != null) traceStream.println(traceBuilder.toString());
+    traceBuilder.append("\n");
 
     OkHttpClient httpClient = this.httpClient;
     if (method == Method.PUT || method == Method.POST) {
@@ -645,15 +665,27 @@ protected Response execute(
     }
 
     Response response = httpClient.newCall(request).execute();
-    if (this.traceStream != null) {
-      this.traceStream.println(
-          response.protocol().toString().toUpperCase(Locale.US) + " " + response.code());
-      this.traceStream.println(response.headers());
-    }
+    String trace =
+        response.protocol().toString().toUpperCase(Locale.US)
+            + " "
+            + response.code()
+            + "\n"
+            + response.headers();
+    traceBuilder.append(trace).append("\n");
+    if (traceStream != null) traceStream.println(trace);
 
     if (response.isSuccessful()) {
-      if (this.traceStream != null) {
-        this.traceStream.println(END_HTTP);
+      if (traceStream != null) {
+        // Trace response body only if the request is not GetObject/ListenBucketNotification S3 API.
+        Set<String> keys = queryParamMap.keySet();
+        if ((method != Method.GET
+                || objectName == null
+                || !Collections.disjoint(keys, TRACE_QUERY_PARAMS))
+            && !(keys.contains("events") && (keys.contains("prefix") || keys.contains("suffix")))) {
+          ResponseBody responseBody = response.peekBody(1024 * 1024);
+          traceStream.println(responseBody.string());
+        }
+        traceStream.println(END_HTTP);
       }
       return response;
     }
@@ -663,36 +695,32 @@ protected Response execute(
       errorXml = responseBody.string();
     }
 
-    if (this.traceStream != null && !("".equals(errorXml) && method.equals(Method.HEAD))) {
-      this.traceStream.println(errorXml);
+    if (!("".equals(errorXml) && method.equals(Method.HEAD))) {
+      traceBuilder.append(errorXml).append("\n");
+      if (traceStream != null) traceStream.println(errorXml);
     }
 
+    traceBuilder.append(END_HTTP).append("\n");
+    if (traceStream != null) traceStream.println(END_HTTP);
+
     // Error in case of Non-XML response from server for non-HEAD requests.
     String contentType = response.headers().get("content-type");
     if (!method.equals(Method.HEAD)
         && (contentType == null
             || !Arrays.asList(contentType.split(";")).contains("application/xml"))) {
-      if (this.traceStream != null) {
-        this.traceStream.println(END_HTTP);
-      }
       throw new InvalidResponseException(
           response.code(),
           contentType,
-          errorXml.substring(0, errorXml.length() > 1024 ? 1024 : errorXml.length()));
+          errorXml.substring(0, errorXml.length() > 1024 ? 1024 : errorXml.length()),
+          traceBuilder.toString());
     }
 
     ErrorResponse errorResponse = null;
     if (!"".equals(errorXml)) {
       errorResponse = Xml.unmarshal(ErrorResponse.class, errorXml);
     } else if (!method.equals(Method.HEAD)) {
-      if (this.traceStream != null) {
-        this.traceStream.println(END_HTTP);
-      }
-      throw new InvalidResponseException(response.code(), contentType, errorXml);
-    }
-
-    if (this.traceStream != null) {
-      this.traceStream.println(END_HTTP);
+      throw new InvalidResponseException(
+          response.code(), contentType, errorXml, traceBuilder.toString());
     }
 
     if (errorResponse == null) {
@@ -746,14 +774,16 @@ protected Response execute(
           break;
         default:
           if (response.code() >= 500) {
-            throw new ServerException("server failed with HTTP status code " + response.code());
+            throw new ServerException(
+                "server failed with HTTP status code " + response.code(), traceBuilder.toString());
           }
 
           throw new InternalException(
               "unhandled HTTP code "
                   + response.code()
                   + ".  Please report this issue at "
-                  + "https://github.com/minio/minio-java/issues");
+                  + "https://github.com/minio/minio-java/issues",
+              traceBuilder.toString());
       }
 
       errorResponse =
@@ -772,7 +802,7 @@ protected Response execute(
       regionCache.remove(bucketName);
     }
 
-    throw new ErrorResponseException(errorResponse, response);
+    throw new ErrorResponseException(errorResponse, response, traceBuilder.toString());
   }
 
   /** Returns region of given bucket either from region cache or set in constructor. */
@@ -867,7 +897,8 @@ private Response executeHead(
               errorResponse.resource(),
               errorResponse.requestId(),
               errorResponse.hostId()),
-          e.response());
+          e.response(),
+          e.httpTrace());
     }
   }
 
@@ -3777,7 +3808,8 @@ private long getAvailableSize(Object data, long expectedReadSize)
     if (!(data instanceof BufferedInputStream)) {
       throw new InternalException(
           "data must be BufferedInputStream. This should not happen.  "
-              + "Please report to https://github.com/minio/minio-java/issues/");
+              + "Please report to https://github.com/minio/minio-java/issues/",
+          null);
     }
 
     BufferedInputStream stream = (BufferedInputStream) data;
@@ -4137,7 +4169,7 @@ protected ObjectWriteResponse completeMultipartUpload(
         try {
           if (Xml.validate(ErrorResponse.class, bodyContent)) {
             ErrorResponse errorResponse = Xml.unmarshal(ErrorResponse.class, bodyContent);
-            throw new ErrorResponseException(errorResponse, response);
+            throw new ErrorResponseException(errorResponse, response, null);
           }
         } catch (XmlParserException e) {
           // As it is not <Error> message, fall-back to parse CompleteMultipartUploadOutput XML.

api/src/main/java/io/minio/SelectResponseStream.java

@@ -160,7 +160,7 @@ private boolean populate() throws EOFException, IOException, InternalException,
       return true;
     }
 
-    throw new InternalException("unknown event-type '" + headerMap.get(":event-type") + "'");
+    throw new InternalException("unknown event-type '" + headerMap.get(":event-type") + "'", null);
   }
 
   /** read single byte from payload. */

api/src/main/java/io/minio/errors/BucketPolicyTooLargeException.java

@@ -16,23 +16,10 @@
 
 package io.minio.errors;
 
-/** Thrown to indicate that given bucket name is not valid. */
+/** Thrown to indicate that received bucket policy is larger than 20KiB size. */
 @SuppressWarnings({"WeakerAccess", "unused"})
 public class BucketPolicyTooLargeException extends MinioException {
-
-  private final String bucketName;
-
-  /**
-   * Constructs a new BucketPolicyTooLargeException with bucket name caused the error and error
-   * message.
-   */
   public BucketPolicyTooLargeException(String bucketName) {
-    super();
-    this.bucketName = bucketName;
-  }
-
-  @Override
-  public String toString() {
-    return String.format("Bucket policy is too large for bucket %s", this.bucketName);
+    super("Bucket policy is larger than 20KiB size for bucket " + bucketName);
   }
 }

api/src/main/java/io/minio/errors/ErrorResponseException.java

@@ -32,8 +32,8 @@ public class ErrorResponseException extends MinioException {
   private final Response response;
 
   /** Constructs a new ErrorResponseException with error response and HTTP response object. */
-  public ErrorResponseException(ErrorResponse errorResponse, Response response) {
-    super(errorResponse.message());
+  public ErrorResponseException(ErrorResponse errorResponse, Response response, String httpTrace) {
+    super(errorResponse.message(), httpTrace);
     this.errorResponse = errorResponse;
     this.response = response;
   }

api/src/main/java/io/minio/errors/InternalException.java

@@ -21,7 +21,7 @@
  */
 public class InternalException extends MinioException {
   /** Constructs a new InternalException with given error message. */
-  public InternalException(String message) {
-    super(message);
+  public InternalException(String message, String httpTrace) {
+    super(message, httpTrace);
   }
 }

api/src/main/java/io/minio/errors/InvalidResponseException.java

@@ -18,13 +18,15 @@
 
 /** Thrown to indicate that non-xml response thrown from server. */
 public class InvalidResponseException extends MinioException {
-  public InvalidResponseException(int responseCode, String contentType, String body) {
+  public InvalidResponseException(
+      int responseCode, String contentType, String body, String httpTrace) {
     super(
         "Non-XML response from server. Response code: "
             + responseCode
             + ", Content-Type: "
             + contentType
             + ", body: "
-            + body);
+            + body,
+        httpTrace);
   }
 }

api/src/main/java/io/minio/errors/MinioException.java

@@ -18,6 +18,8 @@
 
 /** Base Exception class for all minio-java exceptions. */
 public class MinioException extends Exception {
+  String httpTrace = null;
+
   /** Constructs a new MinioException. */
   public MinioException() {
     super();
@@ -27,4 +29,14 @@ public MinioException() {
   public MinioException(String message) {
     super(message);
   }
+
+  /** Constructs a new MinioException with given error message. */
+  public MinioException(String message, String httpTrace) {
+    super(message);
+    this.httpTrace = httpTrace;
+  }
+
+  public String httpTrace() {
+    return this.httpTrace;
+  }
 }

api/src/main/java/io/minio/errors/ServerException.java

@@ -18,7 +18,7 @@
 
 /** Thrown to indicate that S3 service returning HTTP server error. */
 public class ServerException extends MinioException {
-  public ServerException(String message) {
-    super(message);
+  public ServerException(String message, String httpTrace) {
+    super(message, httpTrace);
   }
 }