Skip to content

Commit fa56d30

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 fa56d30

File tree

4 files changed

+176
-8
lines changed

4 files changed

+176
-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: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,66 @@ 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 specific 4xx errors that indicate transient issues
63+
if (httpCode == 400 || httpCode == 429) {
64+
String description = errorDescription != null ? errorDescription.toLowerCase() : "";
65+
// Check for retry hints in the error description
66+
return description.contains("retry")
67+
|| description.contains("unknown_error")
68+
|| description.contains("temporary")
69+
|| description.contains("rate limit")
70+
|| description.contains("throttle");
71+
}
72+
73+
return false;
74+
} catch (NumberFormatException e) {
75+
return false;
76+
}
77+
}
1678
}

‎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
}

0 commit comments

Comments
 (0)