-
-
Notifications
You must be signed in to change notification settings - Fork 758
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
better handling of invocationtargetexception #968
better handling of invocationtargetexception #968
Conversation
Throwable cause = e.getCause(); | ||
throw new SessionNotCreatedException( | ||
format("Unable to create new remote session. desired capabilities = %s" | ||
+ "and reason is " + cause.getMessage(), desired)); |
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.
why not to use format specifiers for both arguments?
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.
Also, it might be handy to pass the original exception as cause
constructor argument, so it would be easier to debug it.
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.
Throwable cause = e.getCause();
throw new SessionNotCreatedException(
format("Unable to create new remote session. desired capabilities = %s"
+ "and reason is %s", desired, cause != null ? cause.getMessage() : null,
e.getTargetException()));
@mykola-mokhnach Do you mean it this way?
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.
almost. It's just instead of setting reason to null if cause is null you set it to the original exception value
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.
also SessionNotCreatedException has the second constructor where one can provide the cause:
SessionNotCreatedException(String msg, Throwable cause)
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.
Yeah i used the same constructor above.
throw new WebDriverException(format("It is impossible to create a new session " | ||
+ "because 'createSession' which takes %s, %s and %s was not found " | ||
+ "or it is not accessible", | ||
HttpClient.class.getSimpleName(), | ||
InputStream.class.getSimpleName(), | ||
long.class.getSimpleName()), e); | ||
} catch (InvocationTargetException e) { | ||
Throwable cause = e.getCause(); |
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.
can it be that getCause returns null?
throw new SessionNotCreatedException( | ||
format("Unable to create new remote session. desired capabilities = %s" | ||
+ "and reason is " + cause.getMessage(), desired)); | ||
format("Unable to create new remote session. desired capabilities = %s", desired), e); |
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.
desired capabilities = %s
-> Desired capabilities: %s
Change list
Better handling of invocationtargetexception thrown during creating session
Thanks @iddol.
Types of changes
What types of changes are you proposing/introducing to Java client?
Put an
x
in the boxes that apply@mykola-mokhnach FYI