-
Notifications
You must be signed in to change notification settings - Fork 8.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HADOOP-17837: Add unresolved endpoint value to UnknownHostException #3272
Conversation
🎊 +1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, it's merged already. I had some suggestions
@@ -111,6 +111,7 @@ public void testInvalidAddress() throws Throwable { | |||
fail("Should not have connected"); | |||
} catch (UnknownHostException uhe) { | |||
LOG.info("Got exception: ", uhe); | |||
assertEquals("invalid-test-host:0", uhe.getMessage()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use GenericTestUtils.assertExceptionContains()
; this rethrows the exception if there's no match, and allows for extra text in the messge
@@ -589,7 +589,7 @@ public static void connect(Socket socket, | |||
} catch (SocketTimeoutException ste) { | |||
throw new ConnectTimeoutException(ste.getMessage()); | |||
} catch (UnresolvedAddressException uae) { | |||
throw new UnknownHostException(uae.getMessage()); | |||
throw new UnknownHostException(endpoint.toString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we still need that message from the original string so build it from uhe.getMessage() + " " + endpoint
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for taking a look. I considered this, but a UHE cannot have a message. It only as has no args constructor so it cannot be created with a message. There's also no setMessage method as far as I can tell, so can't be added after. https://docs.oracle.com/javase/8/docs/api/java/net/UnknownHostException.html
Per https://issues.apache.org/jira/browse/HADOOP-17837, adds `endpoint.toString() to the UnknownHostException. UnresolvedAddressException cannot have a message, so the current behavior is a UHE with no message as well. Adding the endpoint to the exception will make it easier to debug when these arise.