Skip to content

Commit 2866da5

Browse files
KaviarasuSakthivadivelKaviarasuSakthivadivel
committed
Improve authentication error logging and retry handling
Enhanced error logging and retry logic for authentication failures to provide better visibility into transient server errors and improve resilience against flaky authentication endpoints. Changes: - Enhanced AuthorizationException.getMessage() to include HTTP status code and error description in a structured format - Added AuthorizationException.isRetriable() method to identify transient errors that should be retried: * All 5xx server errors (500, 502, 503, 504, etc.) * 400 errors with retry hints ("retry your request", "unknown_error") * 429 rate limit errors - Updated FormCommand to handle 5xx errors as AuthorizationException instead of IOException for consistent retry handling - Modified retry policy in DataCloudTokenProvider to retry retriable AuthorizationExceptions while failing fast on permanent auth errors - Added comprehensive retry logging: * WARN logs on each retry attempt with attempt number and error details * ERROR logs when all retries are exhausted - Improved error logging in getWithRetry() to extract and log full error details including HTTP codes and error descriptions This improves debugging of authentication issues and makes the driver more resilient to transient server-side failures commonly seen in test environments.
1 parent b8c5eb9 commit 2866da5

File tree

7 files changed

+455
-8
lines changed

7 files changed

+455
-8
lines changed

‎integration-test/src/test/java/com/salesforce/datacloud/jdbc/integration/ShadedJarIntegrationTest.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,13 @@ void testDriverLoading() throws Exception {
6868
/**
6969
* Test 2: Verify connection and query execution (tests gRPC NameResolver and end-to-end functionality)
7070
* This is the critical test that would have caught the service file regression
71+
*
72+
* Note: This test requires credentials to be provided via system properties:
73+
* - test.connection.url (optional, defaults to test2.pc-rnd.salesforce.com)
74+
* - test.connection.userName
75+
* - test.connection.password
76+
* - test.connection.clientId
77+
* - test.connection.clientSecret
7178
*/
7279
@Test
7380
@Order(2)

‎jdbc-http/src/main/java/com/salesforce/datacloud/jdbc/auth/DataCloudTokenProvider.java‎

Lines changed: 70 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -200,18 +200,84 @@ private <T extends AuthenticationResponseWithError> T getWithRetry(CheckedSuppli
200200
try {
201201
return Failsafe.with(this.exponentialBackOffPolicy).get(response);
202202
} catch (FailsafeException ex) {
203-
if (ex.getCause() != null) {
204-
throw new SQLException(ex.getCause().getMessage(), "28000", ex);
203+
Throwable cause = ex.getCause();
204+
if (cause instanceof AuthorizationException) {
205+
AuthorizationException authEx = (AuthorizationException) cause;
206+
String errorMessage = authEx.getMessage();
207+
log.error(
208+
"Authorization failed - Error Code: {}, Error Description: {}, Message: {}",
209+
authEx.getErrorCode(),
210+
authEx.getErrorDescription(),
211+
errorMessage);
212+
throw new SQLException(errorMessage, "28000", authEx);
213+
} else if (cause != null) {
214+
String causeMessage = cause.getMessage();
215+
if (causeMessage == null || causeMessage.isEmpty()) {
216+
causeMessage = cause.getClass().getSimpleName() + " occurred during authentication";
217+
}
218+
log.error("Authentication failed: {}", causeMessage, cause);
219+
throw new SQLException(causeMessage, "28000", cause);
220+
} else {
221+
String exMessage = ex.getMessage();
222+
if (exMessage == null || exMessage.isEmpty()) {
223+
exMessage = "Authentication failed with unknown error";
224+
}
225+
log.error("Authentication failed: {}", exMessage, ex);
226+
throw new SQLException(exMessage, "28000", ex);
205227
}
206-
throw new SQLException(ex.getMessage(), "28000", ex);
207228
}
208229
}
209230

210231
private static RetryPolicy<AuthenticationResponseWithError> buildExponentialBackoffRetryPolicy(int maxRetries) {
211232
return RetryPolicy.<AuthenticationResponseWithError>builder()
212233
.withMaxRetries(maxRetries)
213234
.withBackoff(1, 30, ChronoUnit.SECONDS)
214-
.handleIf(e -> !(e instanceof AuthorizationException))
235+
.handleIf(e -> {
236+
// Retry on AuthorizationException only if it's retriable (transient errors)
237+
if (e instanceof AuthorizationException) {
238+
return ((AuthorizationException) e).isRetriable();
239+
}
240+
// Retry on other exceptions (IOExceptions, etc.)
241+
return true;
242+
})
243+
.onRetry(event -> {
244+
Throwable lastException = event.getLastException();
245+
if (lastException != null) {
246+
if (lastException instanceof AuthorizationException) {
247+
AuthorizationException authEx = (AuthorizationException) lastException;
248+
log.warn(
249+
"Authentication retry attempt {}/{} for HTTP {} error: {}",
250+
event.getAttemptCount(),
251+
maxRetries + 1,
252+
authEx.getErrorCode(),
253+
authEx.getErrorDescription());
254+
} else {
255+
log.warn(
256+
"Authentication retry attempt {}/{} for error: {}",
257+
event.getAttemptCount(),
258+
maxRetries + 1,
259+
lastException.getMessage());
260+
}
261+
}
262+
})
263+
.onRetriesExceeded(event -> {
264+
Throwable lastException = event.getException();
265+
if (lastException != null) {
266+
if (lastException instanceof AuthorizationException) {
267+
AuthorizationException authEx = (AuthorizationException) lastException;
268+
log.error(
269+
"Authentication failed after {} retry attempts. HTTP {} error: {}",
270+
maxRetries,
271+
authEx.getErrorCode(),
272+
authEx.getErrorDescription());
273+
} else {
274+
log.error(
275+
"Authentication failed after {} retry attempts. Error: {}",
276+
maxRetries,
277+
lastException.getMessage());
278+
}
279+
}
280+
})
215281
.build();
216282
}
217283
}

‎jdbc-http/src/main/java/com/salesforce/datacloud/jdbc/auth/errors/AuthorizationException.java‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,71 @@ public class AuthorizationException extends Exception {
1313
private final String message;
1414
private final String errorCode;
1515
private final String errorDescription;
16+
17+
@Override
18+
public String getMessage() {
19+
return buildDetailedMessage();
20+
}
21+
22+
private String buildDetailedMessage() {
23+
StringBuilder sb = new StringBuilder();
24+
if (message != null && !message.isEmpty()) {
25+
sb.append(message);
26+
} else {
27+
sb.append("Authorization failed");
28+
}
29+
30+
if (errorCode != null && !errorCode.isEmpty()) {
31+
sb.append(" (HTTP ").append(errorCode).append(")");
32+
}
33+
34+
if (errorDescription != null && !errorDescription.isEmpty()) {
35+
sb.append(": ").append(errorDescription);
36+
}
37+
38+
return sb.toString();
39+
}
40+
41+
/**
42+
* Determines if this exception represents a transient error that should be retried.
43+
* Retries are appropriate for:
44+
* - 5xx server errors (500, 502, 503, 504)
45+
* - 400 errors with retry hints in the error description (e.g., "retry your request", "unknown_error")
46+
*
47+
* @return true if this exception should be retried, false otherwise
48+
*/
49+
public boolean isRetriable() {
50+
if (errorCode == null || errorCode.isEmpty()) {
51+
return false;
52+
}
53+
54+
try {
55+
int httpCode = Integer.parseInt(errorCode);
56+
57+
// Retry on 5xx server errors (transient server issues)
58+
if (httpCode >= 500 && httpCode < 600) {
59+
return true;
60+
}
61+
62+
// Retry on 429 (Too Many Requests) - always retriable
63+
if (httpCode == 429) {
64+
return true;
65+
}
66+
67+
// Retry on 400 errors that indicate transient issues
68+
if (httpCode == 400) {
69+
String description = errorDescription != null ? errorDescription.toLowerCase() : "";
70+
// Check for retry hints in the error description
71+
return description.contains("retry")
72+
|| description.contains("unknown_error")
73+
|| description.contains("temporary")
74+
|| description.contains("rate limit")
75+
|| description.contains("throttle");
76+
}
77+
78+
return false;
79+
} catch (NumberFormatException e) {
80+
return false;
81+
}
82+
}
1683
}

‎jdbc-http/src/main/java/com/salesforce/datacloud/jdbc/http/FormCommand.java‎

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import lombok.NonNull;
1818
import lombok.Singular;
1919
import lombok.Value;
20+
import lombok.extern.slf4j.Slf4j;
2021
import lombok.val;
2122
import okhttp3.FormBody;
2223
import okhttp3.Headers;
@@ -25,6 +26,7 @@
2526
import okhttp3.Request;
2627
import okhttp3.Response;
2728

29+
@Slf4j
2830
@Value
2931
@Builder(builderClassName = "Builder")
3032
public class FormCommand {
@@ -76,12 +78,43 @@ private static <T> T executeRequest(@NonNull OkHttpClient client, Request reques
7678
throws SQLException, AuthorizationException {
7779
try (Response response = client.newCall(request).execute()) {
7880
if (!response.isSuccessful()) {
79-
if (response.code() >= 400 && response.code() < 500) {
80-
throw AuthorizationException.builder()
81+
int statusCode = response.code();
82+
String responseBody = "";
83+
try {
84+
val responseBodyObj = response.body();
85+
if (responseBodyObj != null) {
86+
responseBody = responseBodyObj.string();
87+
}
88+
} catch (IOException e) {
89+
// If we can't read the body, continue with empty string
90+
}
91+
92+
// Handle 4xx and 5xx errors as AuthorizationException for consistent retry handling
93+
if (statusCode >= 400 && statusCode < 600) {
94+
AuthorizationException authEx = AuthorizationException.builder()
8195
.message(response.message())
82-
.errorCode(String.valueOf(response.code()))
83-
.errorDescription(response.body().string())
96+
.errorCode(String.valueOf(statusCode))
97+
.errorDescription(responseBody)
8498
.build();
99+
100+
// Log the error details for debugging
101+
if (authEx.isRetriable()) {
102+
log.warn(
103+
"HTTP {} error from {} (retriable): {} - Response body: {}",
104+
statusCode,
105+
request.url(),
106+
response.message(),
107+
responseBody);
108+
} else {
109+
log.error(
110+
"HTTP {} error from {}: {} - Response body: {}",
111+
statusCode,
112+
request.url(),
113+
response.message(),
114+
responseBody);
115+
}
116+
117+
throw authEx;
85118
}
86119
throw new IOException("Unexpected code " + response);
87120
}

‎jdbc-http/src/test/java/com/salesforce/datacloud/jdbc/auth/DataCloudTokenProviderTest.java‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,72 @@ void retryPolicyDoesntRetryOnAuthorizationException() {
104104
}
105105
}
106106

107+
@SneakyThrows
108+
@Test
109+
void retryPolicyRetriesOnRetriableAuthorizationException() {
110+
val properties = propertiesForPassword("un", "pw");
111+
properties.setProperty(HttpClientProperties.HTTP_MAX_RETRIES, "2");
112+
val expectedTriesCount = 3; // initial + 2 retries
113+
try (val server = new MockWebServer()) {
114+
server.start();
115+
// Simulate HTTP 500 errors (retriable) that eventually succeed
116+
for (int i = 0; i < expectedTriesCount - 1; i++) {
117+
server.enqueue(new MockResponse().setResponseCode(500).setBody("Internal Server Error"));
118+
}
119+
// Last request succeeds
120+
val mapper = new ObjectMapper();
121+
val oAuthTokenResponse = new OAuthTokenResponse();
122+
oAuthTokenResponse.setToken(FAKE_TOKEN);
123+
oAuthTokenResponse.setInstanceUrl(server.url("").toString());
124+
server.enqueue(new MockResponse()
125+
.setBody(mapper.writeValueAsString(oAuthTokenResponse))
126+
.setResponseCode(200));
127+
128+
val loginUrl = server.url("").uri();
129+
HttpClientProperties clientProperties = HttpClientProperties.ofDestructive(properties);
130+
SalesforceAuthProperties authProperties = SalesforceAuthProperties.ofDestructive(loginUrl, properties);
131+
val token =
132+
DataCloudTokenProvider.of(clientProperties, authProperties).getOAuthToken();
133+
assertThat(token.getToken()).isEqualTo(FAKE_TOKEN);
134+
assertThat(server.getRequestCount()).isEqualTo(expectedTriesCount);
135+
server.shutdown();
136+
}
137+
}
138+
139+
@SneakyThrows
140+
@Test
141+
void retryPolicyRetriesOn400WithRetryHint() {
142+
val properties = propertiesForPassword("un", "pw");
143+
properties.setProperty(HttpClientProperties.HTTP_MAX_RETRIES, "2");
144+
val expectedTriesCount = 3; // initial + 2 retries
145+
try (val server = new MockWebServer()) {
146+
server.start();
147+
// Simulate HTTP 400 with retry hint (retriable)
148+
for (int i = 0; i < expectedTriesCount - 1; i++) {
149+
server.enqueue(new MockResponse()
150+
.setResponseCode(400)
151+
.setBody("{\"error\":\"unknown_error\",\"error_description\":\"retry your request\"}"));
152+
}
153+
// Last request succeeds
154+
val mapper = new ObjectMapper();
155+
val oAuthTokenResponse = new OAuthTokenResponse();
156+
oAuthTokenResponse.setToken(FAKE_TOKEN);
157+
oAuthTokenResponse.setInstanceUrl(server.url("").toString());
158+
server.enqueue(new MockResponse()
159+
.setBody(mapper.writeValueAsString(oAuthTokenResponse))
160+
.setResponseCode(200));
161+
162+
val loginUrl = server.url("").uri();
163+
HttpClientProperties clientProperties = HttpClientProperties.ofDestructive(properties);
164+
SalesforceAuthProperties authProperties = SalesforceAuthProperties.ofDestructive(loginUrl, properties);
165+
val token =
166+
DataCloudTokenProvider.of(clientProperties, authProperties).getOAuthToken();
167+
assertThat(token.getToken()).isEqualTo(FAKE_TOKEN);
168+
assertThat(server.getRequestCount()).isEqualTo(expectedTriesCount);
169+
server.shutdown();
170+
}
171+
}
172+
107173
@SneakyThrows
108174
@Test
109175
void oauthTokenRetrieved() {

0 commit comments

Comments
 (0)