Skip to content

Commit 46b8204

Browse files
Copilotbrunoborges
andcommitted
Fix test to handle new IllegalStateException from closed sessions
Co-authored-by: brunoborges <129743+brunoborges@users.noreply.github.com>
1 parent c57e7a3 commit 46b8204

3 files changed

Lines changed: 37 additions & 23 deletions

File tree

src/main/java/com/github/copilot/sdk/CopilotSession.java

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ public final class CopilotSession implements AutoCloseable {
9797
private final AtomicReference<SessionHooks> hooksHandler = new AtomicReference<>();
9898
private volatile EventErrorHandler eventErrorHandler;
9999
private volatile EventErrorPolicy eventErrorPolicy = EventErrorPolicy.PROPAGATE_AND_LOG_ERRORS;
100-
100+
101101
/** Tracks whether this session instance has been terminated via close(). */
102102
private volatile boolean isTerminated = false;
103103

@@ -189,7 +189,8 @@ public String getWorkspacePath() {
189189
* @param handler
190190
* the error handler, or {@code null} to use only the default logging
191191
* behavior
192-
* @throws IllegalStateException if this session has been terminated
192+
* @throws IllegalStateException
193+
* if this session has been terminated
193194
* @see EventErrorHandler
194195
* @see #setEventErrorPolicy(EventErrorPolicy)
195196
* @since 1.0.8
@@ -229,7 +230,8 @@ public void setEventErrorHandler(EventErrorHandler handler) {
229230
* @param policy
230231
* the error policy (default is
231232
* {@link EventErrorPolicy#PROPAGATE_AND_LOG_ERRORS})
232-
* @throws IllegalStateException if this session has been terminated
233+
* @throws IllegalStateException
234+
* if this session has been terminated
233235
* @see EventErrorPolicy
234236
* @see #setEventErrorHandler(EventErrorHandler)
235237
* @since 1.0.8
@@ -251,7 +253,8 @@ public void setEventErrorPolicy(EventErrorPolicy policy) {
251253
* @param prompt
252254
* the message text to send
253255
* @return a future that resolves with the message ID assigned by the server
254-
* @throws IllegalStateException if this session has been terminated
256+
* @throws IllegalStateException
257+
* if this session has been terminated
255258
* @see #send(MessageOptions)
256259
*/
257260
public CompletableFuture<String> send(String prompt) {
@@ -269,7 +272,8 @@ public CompletableFuture<String> send(String prompt) {
269272
* the message text to send
270273
* @return a future that resolves with the final assistant message event, or
271274
* {@code null} if no assistant message was received
272-
* @throws IllegalStateException if this session has been terminated
275+
* @throws IllegalStateException
276+
* if this session has been terminated
273277
* @see #sendAndWait(MessageOptions)
274278
*/
275279
public CompletableFuture<AssistantMessageEvent> sendAndWait(String prompt) {
@@ -286,7 +290,8 @@ public CompletableFuture<AssistantMessageEvent> sendAndWait(String prompt) {
286290
* @param options
287291
* the message options containing the prompt and attachments
288292
* @return a future that resolves with the message ID assigned by the server
289-
* @throws IllegalStateException if this session has been terminated
293+
* @throws IllegalStateException
294+
* if this session has been terminated
290295
* @see #sendAndWait(MessageOptions)
291296
* @see #send(String)
292297
*/
@@ -317,7 +322,8 @@ public CompletableFuture<String> send(MessageOptions options) {
317322
* {@code null} if no assistant message was received. The future
318323
* completes exceptionally with a TimeoutException if the timeout
319324
* expires.
320-
* @throws IllegalStateException if this session has been terminated
325+
* @throws IllegalStateException
326+
* if this session has been terminated
321327
* @see #sendAndWait(MessageOptions)
322328
* @see #send(MessageOptions)
323329
*/
@@ -380,7 +386,8 @@ public CompletableFuture<AssistantMessageEvent> sendAndWait(MessageOptions optio
380386
* the message options containing the prompt and attachments
381387
* @return a future that resolves with the final assistant message event, or
382388
* {@code null} if no assistant message was received
383-
* @throws IllegalStateException if this session has been terminated
389+
* @throws IllegalStateException
390+
* if this session has been terminated
384391
* @see #sendAndWait(MessageOptions, long)
385392
*/
386393
public CompletableFuture<AssistantMessageEvent> sendAndWait(MessageOptions options) {
@@ -414,7 +421,8 @@ public CompletableFuture<AssistantMessageEvent> sendAndWait(MessageOptions optio
414421
* @param handler
415422
* a callback to be invoked when a session event occurs
416423
* @return a Closeable that, when closed, unsubscribes the handler
417-
* @throws IllegalStateException if this session has been terminated
424+
* @throws IllegalStateException
425+
* if this session has been terminated
418426
* @see #on(Class, Consumer)
419427
* @see AbstractSessionEvent
420428
* @see #setEventErrorPolicy(EventErrorPolicy)
@@ -466,7 +474,8 @@ public Closeable on(Consumer<AbstractSessionEvent> handler) {
466474
* @param handler
467475
* a callback invoked when events of this type occur
468476
* @return a Closeable that unsubscribes the handler when closed
469-
* @throws IllegalStateException if this session has been terminated
477+
* @throws IllegalStateException
478+
* if this session has been terminated
470479
* @see #on(Consumer)
471480
* @see AbstractSessionEvent
472481
*/
@@ -729,7 +738,8 @@ CompletableFuture<Object> handleHooksInvoke(String hookType, JsonNode input) {
729738
* assistant responses, tool invocations, and other session events.
730739
*
731740
* @return a future that resolves with a list of all session events
732-
* @throws IllegalStateException if this session has been terminated
741+
* @throws IllegalStateException
742+
* if this session has been terminated
733743
* @see AbstractSessionEvent
734744
*/
735745
public CompletableFuture<List<AbstractSessionEvent>> getMessages() {
@@ -760,17 +770,19 @@ public CompletableFuture<List<AbstractSessionEvent>> getMessages() {
760770
* continuing to generate a response.
761771
*
762772
* @return a future that completes when the abort is acknowledged
763-
* @throws IllegalStateException if this session has been terminated
773+
* @throws IllegalStateException
774+
* if this session has been terminated
764775
*/
765776
public CompletableFuture<Void> abort() {
766777
ensureNotTerminated();
767778
return rpc.invoke("session.abort", Map.of("sessionId", sessionId), Void.class);
768779
}
769-
780+
770781
/**
771782
* Verifies that this session has not yet been terminated.
772-
*
773-
* @throws IllegalStateException if close() has already been invoked
783+
*
784+
* @throws IllegalStateException
785+
* if close() has already been invoked
774786
*/
775787
private void ensureNotTerminated() {
776788
if (isTerminated) {

src/test/java/com/github/copilot/sdk/ClosedSessionGuardTest.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@
1717

1818
/**
1919
* Tests for closed-session guard functionality in CopilotSession.
20-
*
20+
*
2121
* <p>
2222
* Verifies that all public methods that interact with session state throw
23-
* IllegalStateException when invoked after close(), and that close() itself
24-
* is idempotent.
23+
* IllegalStateException when invoked after close(), and that close() itself is
24+
* idempotent.
2525
* </p>
2626
*/
2727
public class ClosedSessionGuardTest {
@@ -60,8 +60,8 @@ void testSendStringThrowsAfterTermination() throws Exception {
6060
}
6161

6262
/**
63-
* Verifies that send(MessageOptions) throws IllegalStateException after
64-
* session is terminated.
63+
* Verifies that send(MessageOptions) throws IllegalStateException after session
64+
* is terminated.
6565
*/
6666
@Test
6767
void testSendOptionsThrowsAfterTermination() throws Exception {

src/test/java/com/github/copilot/sdk/CopilotSessionTest.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,15 @@ void testShouldReceiveSessionEvents_createAndDestroy() throws Exception {
7979

8080
session.close();
8181

82-
// Session should no longer be accessible
82+
// Session should no longer be accessible - now throws IllegalStateException
8383
try {
8484
session.getMessages().get();
8585
fail("Expected exception for closed session");
8686
} catch (Exception e) {
87-
assertTrue(e.getMessage().toLowerCase().contains("not found")
88-
|| e.getCause().getMessage().toLowerCase().contains("not found"));
87+
// After our changes, we now get IllegalStateException directly
88+
assertTrue(e.getMessage().toLowerCase().contains("closed")
89+
|| (e.getCause() != null
90+
&& e.getCause().getMessage().toLowerCase().contains("not found")));
8991
}
9092
}
9193
}

0 commit comments

Comments
 (0)