Skip to content

Commit

Permalink
Improve error messages when using proxy server.
Browse files Browse the repository at this point in the history
If using a proxy server and the AWS exception does not
contain a request ID, then that's a potential sign that the
request was rejected at the proxy server, because the
request ID is assigned at the AWS server side.  This patch
helps users troubleshoot this by including the proxy host in
the exception message.
  • Loading branch information
cnauroth authored and millems committed Apr 20, 2020
1 parent 6c82752 commit c9bc078
Show file tree
Hide file tree
Showing 9 changed files with 243 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ public enum ErrorType {
*/
private byte[] rawResponse;

/**
* Track proxy host if configured, in case error response came from proxy instead of AWS.
*/
private String proxyHost;

/**
* Constructs a new AmazonServiceException with the specified message.
*
Expand Down Expand Up @@ -264,7 +269,12 @@ public String getMessage() {
+ " (Service: " + getServiceName()
+ "; Status Code: " + getStatusCode()
+ "; Error Code: " + getErrorCode()
+ "; Request ID: " + getRequestId() + ")";
+ "; Request ID: " + getRequestId()
// If using a proxy host and no AWS request ID, then the error response may have come from the proxy
+ ((getRequestId() == null && getProxyHost() != null)
? "; Proxy: " + getProxyHost()
: "")
+ ")";
}

/**
Expand Down Expand Up @@ -312,4 +322,23 @@ public Map<String, String> getHttpHeaders() {
public void setHttpHeaders(Map<String, String> httpHeaders) {
this.httpHeaders = httpHeaders;
}

/**
* Returns proxy host if configured.
* If using a proxy, then it's possible that the error response came from the proxy instead of AWS.
*
* @return proxy host if configured or {@code null}
*/
public String getProxyHost() {
return proxyHost;
}

/**
* Sets proxy host.
*
* @param proxyHost the proxy host to set
*/
public void setProxyHost(String proxyHost) {
this.proxyHost = proxyHost;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ public <T> Response<T> execute(Request<?> request,
return requestExecutionBuilder()
.request(request)
.requestConfig(requestConfig)
.errorResponseHandler(new AwsErrorResponseHandler(errorResponseHandler, executionContext.getAwsRequestMetrics()))
.errorResponseHandler(new AwsErrorResponseHandler(errorResponseHandler, executionContext.getAwsRequestMetrics(), config))
.executionContext(executionContext)
.execute(adaptedRespHandler);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package com.amazonaws.http;

import com.amazonaws.AmazonServiceException;
import com.amazonaws.ClientConfiguration;
import com.amazonaws.annotation.SdkInternalApi;
import com.amazonaws.util.AWSRequestMetrics;
import java.util.Arrays;
Expand All @@ -28,19 +29,23 @@ class AwsErrorResponseHandler implements HttpResponseHandler<AmazonServiceExcept

private final HttpResponseHandler<AmazonServiceException> delegate;
private final AWSRequestMetrics awsRequestMetrics;
private final ClientConfiguration clientConfiguration;


AwsErrorResponseHandler(HttpResponseHandler<AmazonServiceException> errorResponseHandler,
AWSRequestMetrics awsRequestMetrics) {
AWSRequestMetrics awsRequestMetrics,
ClientConfiguration clientConfiguration) {
this.delegate = errorResponseHandler;
this.awsRequestMetrics = awsRequestMetrics;
this.clientConfiguration = clientConfiguration;
}

@Override
public AmazonServiceException handle(HttpResponse response) throws Exception {
final AmazonServiceException ase = handleAse(response);
ase.setStatusCode(response.getStatusCode());
ase.setServiceName(response.getRequest().getServiceName());
ase.setProxyHost(clientConfiguration.getProxyHost());
awsRequestMetrics.addPropertyWith(AWSRequestMetrics.Field.AWSRequestID, ase.getRequestId())
.addPropertyWith(AWSRequestMetrics.Field.AWSErrorCode, ase.getErrorCode())
.addPropertyWith(AWSRequestMetrics.Field.StatusCode, ase.getStatusCode());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/*
* Copyright 2010-2017 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/
package com.amazonaws.http;

import static org.junit.Assert.*;
import static org.mockito.Mockito.*;

import org.apache.http.HttpStatus;

import org.junit.Test;

import com.amazonaws.AmazonServiceException;
import com.amazonaws.ClientConfiguration;
import com.amazonaws.DefaultRequest;
import com.amazonaws.util.AWSRequestMetrics;
import com.amazonaws.util.StringInputStream;

public class AwsErrorResponseHandlerTest {

private static final String ERROR_MESSAGE = "someErrorMessage";
private static final String PROXY_HOST = "someHost";
private static final String REQUEST_ID = "1234";
private static final String SERVICE_NAME = "someService";

@Test
public void handle_withProxy_withRequestId() throws Exception {
HttpResponseHandler<AmazonServiceException> delegate = mock(HttpResponseHandler.class);
AmazonServiceException ase = new AmazonServiceException(ERROR_MESSAGE);
ase.setRequestId(REQUEST_ID);
when(delegate.handle(any(HttpResponse.class))).thenReturn(ase);
AWSRequestMetrics awsRequestMetrics = mock(AWSRequestMetrics.class);
ClientConfiguration conf = new ClientConfiguration();
conf.setProxyHost(PROXY_HOST);
AwsErrorResponseHandler responseHandler = new AwsErrorResponseHandler(delegate, awsRequestMetrics, conf);
HttpResponse httpResponse = new HttpResponse(new DefaultRequest<String>(SERVICE_NAME), null);
httpResponse.setStatusCode(HttpStatus.SC_FORBIDDEN);
AmazonServiceException e = responseHandler.handle(httpResponse);
assertNotNull(e);
assertEquals(PROXY_HOST, e.getProxyHost());
assertNotNull(e.getMessage());
assertFalse(e.getMessage().contains(PROXY_HOST));
}

@Test
public void handle_withProxy_withoutRequestId() throws Exception {
HttpResponseHandler<AmazonServiceException> delegate = mock(HttpResponseHandler.class);
AmazonServiceException ase = new AmazonServiceException(ERROR_MESSAGE);
when(delegate.handle(any(HttpResponse.class))).thenReturn(ase);
AWSRequestMetrics awsRequestMetrics = mock(AWSRequestMetrics.class);
ClientConfiguration conf = new ClientConfiguration();
conf.setProxyHost(PROXY_HOST);
AwsErrorResponseHandler responseHandler = new AwsErrorResponseHandler(delegate, awsRequestMetrics, conf);
HttpResponse httpResponse = new HttpResponse(new DefaultRequest<String>(SERVICE_NAME), null);
httpResponse.setStatusCode(HttpStatus.SC_FORBIDDEN);
AmazonServiceException e = responseHandler.handle(httpResponse);
assertNotNull(e);
assertEquals(PROXY_HOST, e.getProxyHost());
assertNotNull(e.getMessage());
assertTrue(e.getMessage().contains(PROXY_HOST));
}

@Test
public void handle_withoutProxy() throws Exception {
HttpResponseHandler<AmazonServiceException> delegate = mock(HttpResponseHandler.class);
AmazonServiceException ase = new AmazonServiceException(ERROR_MESSAGE);
when(delegate.handle(any(HttpResponse.class))).thenReturn(ase);
AWSRequestMetrics awsRequestMetrics = mock(AWSRequestMetrics.class);
ClientConfiguration conf = new ClientConfiguration();
AwsErrorResponseHandler responseHandler = new AwsErrorResponseHandler(delegate, awsRequestMetrics, conf);
HttpResponse httpResponse = new HttpResponse(new DefaultRequest<String>(SERVICE_NAME), null);
httpResponse.setStatusCode(HttpStatus.SC_FORBIDDEN);
AmazonServiceException e = responseHandler.handle(httpResponse);
assertNotNull(e);
assertNull(e.getProxyHost());
assertNotNull(e.getMessage());
assertFalse(e.getMessage().contains(PROXY_HOST));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ public class AmazonS3Client extends AmazonWebServiceClient implements AmazonS3 {
protected final AWSCredentialsProvider awsCredentialsProvider;

/** Responsible for handling error responses from all S3 service calls. */
protected final S3ErrorResponseHandler errorResponseHandler = new S3ErrorResponseHandler();
protected final S3ErrorResponseHandler errorResponseHandler;

/** Shared response handler for operations with no response. */
private final S3XmlResponseHandler<Void> voidResponseHandler = new S3XmlResponseHandler<Void>(null);
Expand Down Expand Up @@ -627,6 +627,7 @@ public AmazonS3Client(AWSCredentialsProvider credentialsProvider,
super(clientConfiguration, requestMetricCollector, true);
this.awsCredentialsProvider = credentialsProvider;
this.skipMd5CheckStrategy = skipMd5CheckStrategy;
this.errorResponseHandler = new S3ErrorResponseHandler(clientConfiguration);
init();
}

Expand Down Expand Up @@ -693,6 +694,8 @@ public AmazonS3Client(ClientConfiguration clientConfiguration) {
this.awsCredentialsProvider = s3ClientParams.getClientParams().getCredentialsProvider();
this.skipMd5CheckStrategy = SkipMd5CheckStrategy.INSTANCE;
setS3ClientOptions(s3ClientParams.getS3ClientOptions());
this.errorResponseHandler = new S3ErrorResponseHandler(
s3ClientParams.getClientParams().getClientConfiguration());
init();
}

Expand Down Expand Up @@ -2057,6 +2060,7 @@ public CopyObjectResult copyObject(CopyObjectRequest copyObjectRequest)
ase.setExtendedRequestId(hostId);
ase.setServiceName(request.getServiceName());
ase.setStatusCode(200);
ase.setProxyHost(clientConfiguration.getProxyHost());

throw ase;
}
Expand Down Expand Up @@ -2197,6 +2201,7 @@ public CopyPartResult copyPart(CopyPartRequest copyPartRequest) {
ase.setExtendedRequestId(hostId);
ase.setServiceName(request.getServiceName());
ase.setStatusCode(200);
ase.setProxyHost(clientConfiguration.getProxyHost());

throw ase;
}
Expand Down Expand Up @@ -2284,6 +2289,7 @@ public DeleteObjectsResult deleteObjects(DeleteObjectsRequest deleteObjectsReque
ex.setRequestId(headers.get(Headers.REQUEST_ID));
ex.setExtendedRequestId(headers.get(Headers.EXTENDED_REQUEST_ID));
ex.setCloudFrontId(headers.get(Headers.CLOUD_FRONT_ID));
ex.setProxyHost(clientConfiguration.getProxyHost());

throw ex;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ public class AmazonS3ExceptionBuilder {
*/
private String errorResponseXml;

/**
* Track proxy host if configured, in case error response came from proxy instead of AWS.
*/
private String proxyHost;

/**
* Returns the AWS request ID that uniquely identifies the service request
* the caller made.
Expand Down Expand Up @@ -235,6 +240,15 @@ public void setErrorResponseXml(String errorResponseXml) {
this.errorResponseXml = errorResponseXml;
}

/**
* Sets proxy host.
*
* @param proxyHost the proxy host to set
*/
public void setProxyHost(String proxyHost) {
this.proxyHost = proxyHost;
}

/**
* Creates a new AmazonS3Exception object with the values set.
*/
Expand All @@ -249,6 +263,7 @@ public AmazonS3Exception build() {
s3Exception.setCloudFrontId(cloudFrontId);
s3Exception.setAdditionalDetails(additionalDetails);
s3Exception.setErrorType(errorTypeOf(statusCode));
s3Exception.setProxyHost(proxyHost);
return s3Exception;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package com.amazonaws.services.s3.internal;

import com.amazonaws.AmazonServiceException;
import com.amazonaws.ClientConfiguration;
import com.amazonaws.http.HttpMethodName;
import com.amazonaws.http.HttpResponse;
import com.amazonaws.http.HttpResponseHandler;
Expand Down Expand Up @@ -58,6 +59,12 @@ private enum S3ErrorTags {
Error, Message, Code, RequestId, HostId
};

private final ClientConfiguration clientConfiguration;

public S3ErrorResponseHandler(ClientConfiguration clientConfiguration) {
this.clientConfiguration = clientConfiguration;
}

@Override
public AmazonServiceException handle(HttpResponse httpResponse)
throws XMLStreamException {
Expand Down Expand Up @@ -153,6 +160,7 @@ private AmazonServiceException createException(HttpResponse httpResponse) throws
}
continue;
case XMLStreamConstants.END_DOCUMENT:
exceptionBuilder.setProxyHost(clientConfiguration.getProxyHost());
return exceptionBuilder.build();
}
}
Expand Down Expand Up @@ -180,6 +188,7 @@ private AmazonS3Exception createExceptionFromHeaders(
.setErrorCode(statusCode + " " + errorResponse.getStatusText());
exceptionBuilder.addAdditionalDetail(Headers.S3_BUCKET_REGION,
errorResponse.getHeaders().get(Headers.S3_BUCKET_REGION));
exceptionBuilder.setProxyHost(clientConfiguration.getProxyHost());
return exceptionBuilder.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,12 @@ public String getMessage() {
+ "; Status Code: " + getStatusCode()
+ "; Error Code: " + getErrorCode()
+ "; Request ID: " + getRequestId()
+ "; S3 Extended Request ID: " + getExtendedRequestId() + ")";
+ "; S3 Extended Request ID: " + getExtendedRequestId()
// If using a proxy host and no AWS request ID, then the error response may have come from the proxy
+ ((getRequestId() == null && getProxyHost() != null)
? "; Proxy: " + getProxyHost()
: "")
+ ")";
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/*
* Copyright 2010-2017 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/
package com.amazonaws.services.s3.internal;

import static org.junit.Assert.*;

import org.apache.http.HttpStatus;

import org.junit.Test;

import com.amazonaws.AmazonServiceException;
import com.amazonaws.ClientConfiguration;
import com.amazonaws.DefaultRequest;
import com.amazonaws.http.HttpResponse;
import com.amazonaws.util.StringInputStream;

public class S3ErrorResponseHandlerTest {

private static final String PROXY_HOST = "someHost";
private static final String SERVICE_NAME = "someService";

@Test
public void handle_withProxy_withRequestId() throws Exception {
ClientConfiguration conf = new ClientConfiguration();
conf.setProxyHost(PROXY_HOST);
S3ErrorResponseHandler responseHandler = new S3ErrorResponseHandler(conf);
HttpResponse httpResponse = new HttpResponse(new DefaultRequest<String>(SERVICE_NAME), null);
httpResponse.setStatusCode(HttpStatus.SC_FORBIDDEN);
httpResponse.setContent(new StringInputStream(
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
+ "<Error>"
+ " <RequestId>1234</RequestId>"
+ "</Error>"));
AmazonServiceException e = responseHandler.handle(httpResponse);
assertNotNull(e);
assertEquals(PROXY_HOST, e.getProxyHost());
assertNotNull(e.getMessage());
assertFalse(e.getMessage().contains(PROXY_HOST));
}

@Test
public void handle_withProxy_withoutRequestId() throws Exception {
ClientConfiguration conf = new ClientConfiguration();
conf.setProxyHost(PROXY_HOST);
S3ErrorResponseHandler responseHandler = new S3ErrorResponseHandler(conf);
HttpResponse httpResponse = new HttpResponse(new DefaultRequest<String>(SERVICE_NAME), null);
httpResponse.setStatusCode(HttpStatus.SC_FORBIDDEN);
AmazonServiceException e = responseHandler.handle(httpResponse);
assertNotNull(e);
assertEquals(PROXY_HOST, e.getProxyHost());
assertNotNull(e.getMessage());
assertTrue(e.getMessage().contains(PROXY_HOST));
}

@Test
public void handle_withoutProxy() throws Exception {
ClientConfiguration conf = new ClientConfiguration();
S3ErrorResponseHandler responseHandler = new S3ErrorResponseHandler(conf);
HttpResponse httpResponse = new HttpResponse(new DefaultRequest<String>(SERVICE_NAME), null);
httpResponse.setStatusCode(HttpStatus.SC_FORBIDDEN);
AmazonServiceException e = responseHandler.handle(httpResponse);
assertNotNull(e);
assertNull(e.getProxyHost());
assertNotNull(e.getMessage());
assertFalse(e.getMessage().contains(PROXY_HOST));
}
}

0 comments on commit c9bc078

Please sign in to comment.