Skip to content

Commit 0d42ad1

Browse files
KaviarasuSakthivadivelKaviarasuSakthivadivel
authored
feat: Improve authentication error logging and retry handling (#140)
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.) * 429 rate limit errors - Modified retry policy in DataCloudTokenProvider to retry retriable AuthorizationExceptions while failing fast on permanent auth errors This improves debugging of authentication issues and makes the driver more resilient to transient server-side failures commonly seen in test environments.
1 parent a5399bb commit 0d42ad1

7 files changed

Lines changed: 539 additions & 8 deletions

File tree

‎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: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,58 @@ 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+
*
44+
* Retriable errors: 5xx server errors and 429 Too Many Requests.
45+
* Non-retriable: 4xx client errors indicate permanent issues (invalid credentials, malformed request).
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+
if (httpCode >= 500 && httpCode < 600) {
58+
return true;
59+
}
60+
61+
if (httpCode == 429) {
62+
return true;
63+
}
64+
65+
return false;
66+
} catch (NumberFormatException e) {
67+
return false;
68+
}
69+
}
1670
}

‎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: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,38 @@ 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+
107139
@SneakyThrows
108140
@Test
109141
void oauthTokenRetrieved() {
@@ -433,6 +465,66 @@ void testPasswordBasedDataCloudTokenFlow() {
433465
}
434466
}
435467

468+
@SneakyThrows
469+
@Test
470+
void retryPolicyFailsAfterMaxRetriesWithAuthorizationException() {
471+
val properties = propertiesForPassword("un", "pw");
472+
properties.setProperty(HttpClientProperties.HTTP_MAX_RETRIES, "2");
473+
val expectedTriesCount = 3; // initial + 2 retries
474+
try (val server = new MockWebServer()) {
475+
server.start();
476+
for (int i = 0; i < expectedTriesCount; i++) {
477+
server.enqueue(new MockResponse().setResponseCode(503).setBody("Service Unavailable"));
478+
}
479+
val loginUrl = server.url("").uri();
480+
HttpClientProperties clientProperties = HttpClientProperties.ofDestructive(properties);
481+
SalesforceAuthProperties authProperties = SalesforceAuthProperties.ofDestructive(loginUrl, properties);
482+
val ex = assertThrows(SQLException.class, () -> DataCloudTokenProvider.of(clientProperties, authProperties)
483+
.getOAuthToken());
484+
assertThat(ex.getCause()).isInstanceOf(AuthorizationException.class);
485+
assertThat(server.getRequestCount()).isEqualTo(expectedTriesCount);
486+
server.shutdown();
487+
}
488+
}
489+
490+
@SneakyThrows
491+
@Test
492+
void getWithRetryHandlesExceptionWithEmptyMessage() {
493+
val properties = propertiesForPassword("un", "pw");
494+
properties.setProperty(HttpClientProperties.HTTP_MAX_RETRIES, "0");
495+
try (val server = new MockWebServer()) {
496+
server.start();
497+
// Return malformed JSON to trigger Jackson parsing exception with potentially empty message
498+
server.enqueue(new MockResponse().setBody("{invalid json}").setResponseCode(200));
499+
val loginUrl = server.url("").uri();
500+
HttpClientProperties clientProperties = HttpClientProperties.ofDestructive(properties);
501+
SalesforceAuthProperties authProperties = SalesforceAuthProperties.ofDestructive(loginUrl, properties);
502+
val ex = assertThrows(SQLException.class, () -> DataCloudTokenProvider.of(clientProperties, authProperties)
503+
.getOAuthToken());
504+
assertThat(ex.getMessage()).isNotEmpty();
505+
assertThat(ex.getCause()).isNotNull();
506+
server.shutdown();
507+
}
508+
}
509+
510+
@SneakyThrows
511+
@Test
512+
void getWithRetryHandlesIOExceptionDuringRequest() {
513+
val properties = propertiesForPassword("un", "pw");
514+
properties.setProperty(HttpClientProperties.HTTP_MAX_RETRIES, "0");
515+
try (val server = new MockWebServer()) {
516+
server.start();
517+
server.enqueue(new MockResponse().setSocketPolicy(SocketPolicy.DISCONNECT_DURING_RESPONSE_BODY));
518+
val loginUrl = server.url("").uri();
519+
HttpClientProperties clientProperties = HttpClientProperties.ofDestructive(properties);
520+
SalesforceAuthProperties authProperties = SalesforceAuthProperties.ofDestructive(loginUrl, properties);
521+
val ex = assertThrows(SQLException.class, () -> DataCloudTokenProvider.of(clientProperties, authProperties)
522+
.getOAuthToken());
523+
assertThat(ex.getMessage()).isNotEmpty();
524+
server.shutdown();
525+
}
526+
}
527+
436528
private static void assertAuthorizationException(Throwable actual, CharSequence... messages) {
437529
AssertionsForClassTypes.assertThat(actual)
438530
.isInstanceOf(SQLException.class)

0 commit comments

Comments
 (0)