Skip to content

Commit c055a62

Browse files
tavomacieltavomaciel
committed
Fix error handling
Signed-off-by: Gustavo Maciel <gmaciel@atlassian.com>
1 parent 3289537 commit c055a62

File tree

2 files changed

+144
-95
lines changed

2 files changed

+144
-95
lines changed

‎java/integTest/src/test/java/glide/ConnectionTests.java‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -924,4 +924,58 @@ public void test_address_resolver_cluster_client() {
924924

925925
client.close();
926926
}
927+
928+
@Test
929+
@SneakyThrows
930+
public void test_address_resolver_cluster_client_throws_exception() {
931+
// Get the actual server address from test configuration
932+
String[] actualHostParts = CLUSTER_HOSTS[0].split(":");
933+
String actualHost = actualHostParts[0];
934+
int actualPort = Integer.parseInt(actualHostParts[1]);
935+
936+
AddressResolver resolver = (host, port) -> { throw new RuntimeException("test-exception"); };
937+
938+
var client =
939+
GlideClusterClient.createClient(
940+
GlideClusterClientConfiguration.builder()
941+
.address(NodeAddress.builder().host(actualHost).port(actualPort).build())
942+
.addressResolver(resolver)
943+
.build())
944+
.get();
945+
946+
// Connection still goes through, as the fallback is to use the original address if resolver throws an exception
947+
assertEquals("PONG", client.ping().get());
948+
assertEquals(
949+
"OK", client.set("cluster_resolver_test_key", "cluster_resolver_test_value").get());
950+
assertEquals("cluster_resolver_test_value", client.get("cluster_resolver_test_key").get());
951+
952+
client.close();
953+
}
954+
955+
@Test
956+
@SneakyThrows
957+
public void test_address_resolver_cluster_client_returns_null() {
958+
// Get the actual server address from test configuration
959+
String[] actualHostParts = CLUSTER_HOSTS[0].split(":");
960+
String actualHost = actualHostParts[0];
961+
int actualPort = Integer.parseInt(actualHostParts[1]);
962+
963+
AddressResolver resolver = (host, port) -> null;
964+
965+
var client =
966+
GlideClusterClient.createClient(
967+
GlideClusterClientConfiguration.builder()
968+
.address(NodeAddress.builder().host(actualHost).port(actualPort).build())
969+
.addressResolver(resolver)
970+
.build())
971+
.get();
972+
973+
// Connection still goes through, as the fallback is to use the original address if resolver returns null
974+
assertEquals("PONG", client.ping().get());
975+
assertEquals(
976+
"OK", client.set("cluster_resolver_test_key", "cluster_resolver_test_value").get());
977+
assertEquals("cluster_resolver_test_value", client.get("cluster_resolver_test_key").get());
978+
979+
client.close();
980+
}
927981
}

‎java/src/address_resolver.rs‎

Lines changed: 90 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::fmt::Display;
2+
use std::str;
13
use std::sync::Arc;
24

35
use jni::objects::{GlobalRef, JObject, JString};
@@ -35,109 +37,102 @@ impl std::fmt::Debug for JavaAddressResolver {
3537
}
3638
}
3739

38-
impl redis::AddressResolver for JavaAddressResolver {
39-
fn resolve(&self, host: &str, port: u16) -> (String, u16) {
40-
// Try to attach to JVM and call the Java resolver
41-
let mut env = match self.jvm.attach_current_thread_as_daemon() {
42-
Ok(env) => env,
43-
Err(err) => {
44-
error!("Failed to attach to JVM. Falling back to original host and port. {err:?}");
45-
return (host.to_string(), port);
40+
#[derive(Debug)]
41+
enum AddressResolverError {
42+
FailedToAttachError(jni::errors::Error),
43+
FailedToCreateHostString(jni::errors::Error),
44+
FailedToCallMethod(jni::errors::Error),
45+
InvalidResult(jni::errors::Error),
46+
InvalidString(str::Utf8Error),
47+
}
48+
49+
impl Display for AddressResolverError {
50+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
51+
match self {
52+
AddressResolverError::FailedToAttachError(err) => {
53+
write!(f, "Failed to attach to JVM thread: {err}")
54+
}
55+
AddressResolverError::FailedToCreateHostString(err) => {
56+
write!(f, "Failed to create Java string for host: {err}")
57+
}
58+
AddressResolverError::FailedToCallMethod(err) => {
59+
write!(f, "Failed to call method on Java resolver: {err}")
4660
}
47-
};
48-
let host_jstring = match env.new_string(host) {
49-
Ok(host_jstring) => host_jstring,
50-
Err(err) => {
51-
error!("Failed to create Java string for host: {err:?}");
52-
return (host.to_string(), port);
61+
AddressResolverError::InvalidResult(err) => {
62+
write!(f, "Invalid result from Java resolver: {err}")
5363
}
54-
};
55-
// Call the resolver's resolve method: ResolvedAddress resolve(String host, int port)
56-
let result = match env.call_method(
57-
self.resolver_global.as_obj(),
58-
"resolve",
59-
"(Ljava/lang/String;I)Lglide/api/models/configuration/ResolvedAddress;",
60-
&[
61-
jni::objects::JValue::Object(&host_jstring),
62-
jni::objects::JValue::Int(port as i32),
63-
],
64-
) {
65-
Ok(result) => result,
66-
Err(err) => {
67-
error!(
68-
"Failed to call resolve method on the JVM. Falling back to original host and port. {err:?}"
69-
);
70-
return (host.to_string(), port);
64+
AddressResolverError::InvalidString(err) => {
65+
write!(f, "Failed to convert Java string to Rust string: {err}")
7166
}
72-
};
73-
let Ok(resolved_address) = result.l() else {
74-
error!(
75-
"JavaAddressResolver did not return an object. Falling back to original host and port."
76-
);
77-
return (host.to_string(), port);
78-
};
67+
}
68+
}
69+
}
70+
71+
fn map_call_method_err(err: jni::errors::Error, env: &JNIEnv) -> AddressResolverError {
72+
if let Ok(true) = env.exception_check() {
73+
let _ = env.exception_describe(); // Log the exception details
74+
let _ = env.exception_clear();
75+
}
76+
AddressResolverError::FailedToCallMethod(err)
77+
}
78+
impl JavaAddressResolver {
79+
fn try_resolve(&self, host: &str, port: u16) -> Result<(String, u16), AddressResolverError> {
80+
// Prepare to call
81+
let mut env = self
82+
.jvm
83+
.attach_current_thread_as_daemon()
84+
.map_err(AddressResolverError::FailedToAttachError)?;
85+
let host_jstring = env
86+
.new_string(host)
87+
.map_err(AddressResolverError::FailedToCreateHostString)?;
88+
89+
// Call the resolver
90+
let result = env
91+
.call_method(
92+
self.resolver_global.as_obj(),
93+
"resolve",
94+
"(Ljava/lang/String;I)Lglide/api/models/configuration/ResolvedAddress;",
95+
&[
96+
jni::objects::JValue::Object(&host_jstring),
97+
jni::objects::JValue::Int(port as i32),
98+
],
99+
)
100+
.map_err(|err| map_call_method_err(err, &env))?;
101+
let resolved_address = result.l().map_err(AddressResolverError::InvalidResult)?;
79102
if resolved_address.is_null() {
80-
return (host.to_string(), port);
103+
return Ok((host.to_string(), port));
81104
}
82105

83106
// Call succeeded with non-null value. Let's extract the values now.
84-
let resolved_host_obj = match env.call_method(
85-
&resolved_address,
86-
"getHost",
87-
"()Ljava/lang/String;",
88-
&[],
89-
) {
90-
Ok(resolved_host_obj) => resolved_host_obj,
91-
Err(err) => {
92-
error!(
93-
"Failed to call getHost on ResolvedAddress. Falling back to original host and port. {err:?}"
94-
);
95-
return (host.to_string(), port);
96-
}
97-
};
98-
let resolved_host_jobj = match resolved_host_obj.l() {
99-
Ok(resolved_host_jobj) => resolved_host_jobj,
100-
Err(err) => {
101-
error!(
102-
"getHost did not return an object. Falling back to original host and port. {err:?}"
103-
);
104-
return (host.to_string(), port);
105-
}
106-
};
107-
if resolved_host_jobj.is_null() {
108-
error!("getHost returned null. Falling back to original host and port.");
109-
return (host.to_string(), port);
110-
}
107+
let resolved_host_obj = env
108+
.call_method(&resolved_address, "getHost", "()Ljava/lang/String;", &[])
109+
.map_err(|err| map_call_method_err(err, &env))?;
110+
let resolved_host_jobj = resolved_host_obj
111+
.l()
112+
.map_err(AddressResolverError::InvalidResult)?;
111113
let resolved_host_jstr: JString = resolved_host_jobj.into();
112-
let resolved_host_str = match env.get_string(&resolved_host_jstr) {
113-
Ok(resolved_host_str) => resolved_host_str,
114-
Err(err) => {
115-
error!(
116-
"Failed to convert resolved host to Rust string. Falling back to original host and port. {err:?}"
117-
);
118-
return (host.to_string(), port);
119-
}
120-
};
121-
let resolved_host_string = resolved_host_str.to_str().unwrap_or(host).to_string();
114+
let resolved_host_str = env
115+
.get_string(&resolved_host_jstr)
116+
.map_err(AddressResolverError::InvalidResult)?;
117+
let resolved_host_string = resolved_host_str
118+
.to_str()
119+
.map_err(AddressResolverError::InvalidString)?
120+
.to_string();
121+
let resolved_port_val = env
122+
.call_method(&resolved_address, "getPort", "()I", &[])
123+
.map_err(|err| map_call_method_err(err, &env))?;
124+
let resolved_port = resolved_port_val
125+
.i()
126+
.map_err(AddressResolverError::InvalidResult)?;
127+
Ok((resolved_host_string, resolved_port as u16))
128+
}
129+
}
122130

123-
let resolved_port_val = match env.call_method(&resolved_address, "getPort", "()I", &[]) {
124-
Ok(resolved_port_val) => resolved_port_val,
125-
Err(err) => {
126-
error!(
127-
"Failed to call getPort on ResolvedAddress. Falling back to original host and port. {err:?}"
128-
);
129-
return (host.to_string(), port);
130-
}
131-
};
132-
let resolved_port = match resolved_port_val.i() {
133-
Ok(resolved_port) => resolved_port,
134-
Err(err) => {
135-
error!(
136-
"getPort did not return an integer. Falling back to original host and port. {err:?}"
137-
);
138-
return (host.to_string(), port);
139-
}
140-
};
141-
(resolved_host_string, resolved_port as u16)
131+
impl redis::AddressResolver for JavaAddressResolver {
132+
fn resolve(&self, host: &str, port: u16) -> (String, u16) {
133+
self.try_resolve(host, port).unwrap_or_else(|err| {
134+
error!("Failed to resolve address on the JVM. {err}");
135+
(host.to_string(), port)
136+
})
142137
}
143138
}

0 commit comments

Comments
 (0)