Skip to content

Conversation

@shahzebs
Copy link
Contributor

@shahzebs shahzebs commented Jan 9, 2026

When using CloseableHttpResponse and execute() with a response handler, the http connection was closed after the response handler terminated. Returning the http response was not feasible because the http connection was closed and thus so was the InputStream with the content of the response.

Using executeOpen() keeps the connection open and makes it possible to return the response object for later use. The InputStream remains open and unconsumed. The caller, howevever, is now responsible for closing the InputStream and by extension also the Http connection. This is easily done as pointed out in the examples with a try-with-resources.

When using CloseableHttpResponse and execute() with a response handler,
the http connection was closed after the response handler terminated.
Returning the http response was not feasible because the http connection
was closed and thus so was the InputStream with the content of the
response.

Using executeOpen() keeps the connection open and makes it possible to
return the response object for later use. The InputStream remains open
and unconsumed. The caller, howevever, is now responsible for closing
the InputStream and by extension also the Http connection. This is
easily done as pointed out in the examples with a try-with-resources.
@shahzebs shahzebs requested a review from eivinhb January 9, 2026 14:45
@shahzebs
Copy link
Contributor Author

shahzebs commented Jan 9, 2026

Should fix #209

@shahzebs shahzebs requested review from a team and runeflobakk January 12, 2026 12:57
@runeflobakk
Copy link
Member

Synes det er vanskelig å mene noe om dette er safe eller ikke. Jeg har ikke nok oversikt over hele API-flaten (som er ganske stor) denne klienten tilbyr nå. Det jeg legger merke til er at det nå virker som alle requests nå bruker executeOpen? Så lenge vi har kontroll på at vi alltid lukker ting internt der biblioteket selv konsumerer responsen, så er det vel i prinsippet ok.

Jeg tror jeg selv ville foretrukket å konsekvent bruke .executeOpen(..) kun de stedene vi skal laste ned en datastrøm (f.eks. et dokument eller noe), da de ulike execute(..)-metoden er laget nettopp for å unngå å gjøre tabber med å glemme IO-håndtering/lukking.

Bare nevner også at å endre returtypen til en metode er en ikke-binærkompatibel endring, sånn med tanke på versjonering. Dersom noe kode et eller annet sted er kompilert mot tidligere variant av en metode, så vil det kræsje hvis denne metoden får en ny versjon av biblioteket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants