Assign server response outside try block#17
Conversation
|
Hello @bluestealth, Thanks for the PR! Could you please take a moment to sign our CLA so we can review? |
| private Response executeApiCall(HttpRequestBase httpPost) throws IOException { | ||
| CloseableHttpResponse serverResponse = null; | ||
| Response response = new Response(); | ||
| CloseableHttpResponse serverResponse = httpClient.execute(httpPost); |
There was a problem hiding this comment.
I don't understand how this fixes sendgrid/sendgrid-java#165. It seems like the original code should be closing the connection at the end. Could you please elaborate on how your solution fixes the issue?
Some other questions:
- Did you test this? If so, could you describe them? I'd like to reproduce.
- By moving the
httpClient.executeout of the try/catch/finally, how will we handle any exceptions at that function call.
Thanks!
There was a problem hiding this comment.
You are correct that this doesn't fix the error, and could result in some exceptional states not being caught, but I can imagine some sequence of events where serverResponse.close() isn't called with the existing code. Give me a bit to think about that.
With this finalizer as long as the client nulls their reference to Class SendGrid or the class goes out of scope the port should be released. I have tested this on my own machine by nulling my reference, forcing garbage collection and then busy waiting to keep the process open.
@Override
public void finalize() throws Throwable {
try {
this.httpClient.close();
} catch(IOException e) {
throw new Throwable(e.getMessage());
} finally {
super.finalize();
}
}There was a problem hiding this comment.
Thanks for the follow up!
I'm not quite sure what is going on here, but I plan to investigate further.
|
|
||
| apply plugin: 'java' | ||
| apply plugin: 'fatjar' | ||
| apply plugin: "eu.appsatori.fatjar" |
There was a problem hiding this comment.
It looks like this plugin is no longer under development, per: https://github.com/musketyr/gradle-fatjar-plugin
Do you mind converting to https://github.com/johnrengelman/shadow?
|
Thanks again for your support! Please take a moment to fill out this form so that we send you some swag :) |
An attempt to fix Sendgrid client doesn't close TCP connection properly #165.